Bug 1265739 - DocumenetProperties should use LONG as return type. r?jimm draft
authorMakoto Kato <m_kato@ga2.so-net.ne.jp>
Mon, 22 Aug 2016 14:45:49 +0900 (2016-08-22)
changeset 403743 59b40ae02511705737a2702924bd798ae06aa833
parent 403581 f97a056ae6235de7855fd8aaa04fb1c8d183bd06
child 528991 21a23da012ddf310d4bf95e17532186fd05cf652
push id27002
push userm_kato@ga2.so-net.ne.jp
push dateMon, 22 Aug 2016 05:54:58 +0000 (2016-08-22)
reviewersjimm
bugs1265739, 1075194
milestone51.0a1
Bug 1265739 - DocumenetProperties should use LONG as return type. r?jimm When investigating bug 1075194, I found that we don't check return value of DocumentPropertiesW. So we sould use correct type and add log for this API. MozReview-Commit-ID: Ck3VwMq9OpQ
embedding/components/printingui/win/nsPrintDialogUtil.cpp
widget/windows/nsDeviceContextSpecWin.cpp
--- a/embedding/components/printingui/win/nsPrintDialogUtil.cpp
+++ b/embedding/components/printingui/win/nsPrintDialogUtil.cpp
@@ -476,58 +476,58 @@ CreateGlobalDevModeAndInit(const nsXPIDL
   if (!status) {
     return nsReturnRef<nsHGLOBAL>();
   }
 
   // Make sure hPrinter is closed on all paths
   nsAutoPrinter autoPrinter(hPrinter);
 
   // Get the buffer size
-  DWORD dwNeeded = ::DocumentPropertiesW(gParentWnd, hPrinter, printName, nullptr,
+  LONG needed = ::DocumentPropertiesW(gParentWnd, hPrinter, printName, nullptr,
                                          nullptr, 0);
-  if (dwNeeded == 0) {
+  if (needed < 0) {
     return nsReturnRef<nsHGLOBAL>();
   }
 
   // Allocate a buffer of the correct size.
   nsAutoDevMode newDevMode((LPDEVMODEW)::HeapAlloc(::GetProcessHeap(), HEAP_ZERO_MEMORY,
-                                                   dwNeeded));
+                                                   needed));
   if (!newDevMode) {
     return nsReturnRef<nsHGLOBAL>();
   }
 
-  nsHGLOBAL hDevMode = ::GlobalAlloc(GHND, dwNeeded);
+  nsHGLOBAL hDevMode = ::GlobalAlloc(GHND, needed);
   nsAutoGlobalMem globalDevMode(hDevMode);
   if (!hDevMode) {
     return nsReturnRef<nsHGLOBAL>();
   }
 
-  DWORD dwRet = ::DocumentPropertiesW(gParentWnd, hPrinter, printName, newDevMode,
+  LONG ret = ::DocumentPropertiesW(gParentWnd, hPrinter, printName, newDevMode,
                                       nullptr, DM_OUT_BUFFER);
-  if (dwRet != IDOK) {
+  if (ret != IDOK) {
     return nsReturnRef<nsHGLOBAL>();
   }
 
   // Lock memory and copy contents from DEVMODE (current printer)
   // to Global Memory DEVMODE
   LPDEVMODEW devMode = (DEVMODEW *)::GlobalLock(hDevMode);
   if (!devMode) {
     return nsReturnRef<nsHGLOBAL>();
   }
 
-  memcpy(devMode, newDevMode.get(), dwNeeded);
+  memcpy(devMode, newDevMode.get(), needed);
   // Initialize values from the PrintSettings
   nsCOMPtr<nsIPrintSettingsWin> psWin = do_QueryInterface(aPS);
   MOZ_ASSERT(psWin);
   psWin->CopyToNative(devMode);
 
   // Sets back the changes we made to the DevMode into the Printer Driver
-  dwRet = ::DocumentPropertiesW(gParentWnd, hPrinter, printName, devMode, devMode,
+  ret = ::DocumentPropertiesW(gParentWnd, hPrinter, printName, devMode, devMode,
                                 DM_IN_BUFFER | DM_OUT_BUFFER);
-  if (dwRet != IDOK) {
+  if (ret != IDOK) {
     ::GlobalUnlock(hDevMode);
     return nsReturnRef<nsHGLOBAL>();
   }
 
   ::GlobalUnlock(hDevMode);
 
   return globalDevMode.out();
 }
--- a/widget/windows/nsDeviceContextSpecWin.cpp
+++ b/widget/windows/nsDeviceContextSpecWin.cpp
@@ -8,46 +8,34 @@
 #include "mozilla/gfx/PrintTargetWindows.h"
 #include "mozilla/RefPtr.h"
 
 #include "nsDeviceContextSpecWin.h"
 #include "prmem.h"
 
 #include <winspool.h>
 
-#include <tchar.h>
-
 #include "nsIWidget.h"
 
 #include "nsTArray.h"
 #include "nsIPrintSettingsWin.h"
 
 #include "nsString.h"
-#include "nsCRT.h"
 #include "nsIServiceManager.h"
 #include "nsReadableUtils.h"
 #include "nsStringEnumerator.h"
 
 #include "gfxWindowsSurface.h"
 
 #include "nsIFileStreams.h"
 #include "nsIWindowWatcher.h"
 #include "nsIDOMWindow.h"
 #include "mozilla/Services.h"
 #include "nsWindowsHelpers.h"
 
-// For NS_CopyNativeToUnicode
-#include "nsNativeCharsetUtils.h"
-
-// File Picker
-#include "nsIFile.h"
-#include "nsIFilePicker.h"
-#include "nsIStringBundle.h"
-#define NS_ERROR_GFX_PRINTER_BUNDLE_URL "chrome://global/locale/printing.properties"
-
 #include "mozilla/gfx/Logging.h"
 
 #include "mozilla/Logging.h"
 PRLogModuleInfo * kWidgetPrintingLogMod = PR_NewLogModule("printing-widget");
 #define PR_PL(_p1)  MOZ_LOG(kWidgetPrintingLogMod, mozilla::LogLevel::Debug, _p1)
 
 using namespace mozilla;
 using namespace mozilla::gfx;
@@ -337,74 +325,78 @@ nsDeviceContextSpecWin::GetDataFromPrint
     rv = GlobalPrinters::GetInstance()->EnumeratePrinterList();
     if (NS_FAILED(rv)) {
       PR_PL(("***** nsDeviceContextSpecWin::GetDataFromPrinter - Couldn't enumerate printers!\n"));
       DISPLAY_LAST_ERROR
     }
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
-  HANDLE hPrinter = nullptr;
+  nsHPRINTER hPrinter = nullptr;
   wchar_t *name = (wchar_t*)aName; // Windows APIs use non-const name argument
 
   BOOL status = ::OpenPrinterW(name, &hPrinter, nullptr);
   if (status) {
+    nsAutoPrinter autoPrinter(hPrinter);
 
     LPDEVMODEW   pDevMode;
-    DWORD       dwNeeded, dwRet;
 
     // Allocate a buffer of the correct size.
-    dwNeeded = ::DocumentPropertiesW(nullptr, hPrinter, name, nullptr, nullptr, 0);
+    LONG needed = ::DocumentPropertiesW(nullptr, hPrinter, name, nullptr,
+                                        nullptr, 0);
+    if (needed < 0) {
+      PR_PL(("**** nsDeviceContextSpecWin::GetDataFromPrinter - Couldn't get size of DEVMODE using DocumentPropertiesW(pDeviceName = \"%s\"). GetLastEror() = %08x\n",
+             aName ? NS_ConvertUTF16toUTF8(aName).get() : "", GetLastError()));
+      return NS_ERROR_FAILURE;
+    }
 
-    pDevMode = (LPDEVMODEW)::HeapAlloc (::GetProcessHeap(), HEAP_ZERO_MEMORY, dwNeeded);
+    pDevMode = (LPDEVMODEW)::HeapAlloc(::GetProcessHeap(), HEAP_ZERO_MEMORY,
+                                       needed);
     if (!pDevMode) return NS_ERROR_FAILURE;
 
     // Get the default DevMode for the printer and modify it for our needs.
-    dwRet = DocumentPropertiesW(nullptr, hPrinter, name,
-                               pDevMode, nullptr, DM_OUT_BUFFER);
+    LONG ret = ::DocumentPropertiesW(nullptr, hPrinter, name,
+                                     pDevMode, nullptr, DM_OUT_BUFFER);
 
-    if (dwRet == IDOK && aPS) {
+    if (ret == IDOK && aPS) {
       nsCOMPtr<nsIPrintSettingsWin> psWin = do_QueryInterface(aPS);
       MOZ_ASSERT(psWin);
       psWin->CopyToNative(pDevMode);
       // Sets back the changes we made to the DevMode into the Printer Driver
-      dwRet = ::DocumentPropertiesW(nullptr, hPrinter, name,
-                                   pDevMode, pDevMode,
-                                   DM_IN_BUFFER | DM_OUT_BUFFER);
+      ret = ::DocumentPropertiesW(nullptr, hPrinter, name,
+                                  pDevMode, pDevMode,
+                                  DM_IN_BUFFER | DM_OUT_BUFFER);
 
       // We need to copy the final DEVMODE settings back to our print settings,
       // because they may have been set from invalid prefs.
-      if (dwRet == IDOK) {
+      if (ret == IDOK) {
         // We need to get information from the device as well.
         nsAutoHDC printerDC(::CreateICW(kDriverName, aName, nullptr, pDevMode));
         if (NS_WARN_IF(!printerDC)) {
           ::HeapFree(::GetProcessHeap(), 0, pDevMode);
-          ::ClosePrinter(hPrinter);
           return NS_ERROR_FAILURE;
         }
 
         psWin->CopyFromNative(printerDC, pDevMode);
       }
     }
 
-    if (dwRet != IDOK) {
+    if (ret != IDOK) {
       ::HeapFree(::GetProcessHeap(), 0, pDevMode);
-      ::ClosePrinter(hPrinter);
-      PR_PL(("***** nsDeviceContextSpecWin::GetDataFromPrinter - DocumentProperties call failed code: %d/0x%x\n", dwRet, dwRet));
+      PR_PL(("***** nsDeviceContextSpecWin::GetDataFromPrinter - DocumentProperties call failed code: %d/0x%x\n", ret, ret));
       DISPLAY_LAST_ERROR
       return NS_ERROR_FAILURE;
     }
 
     SetDevMode(pDevMode); // cache the pointer and takes responsibility for the memory
 
     SetDeviceName(aName);
 
     SetDriverName(kDriverName);
 
-    ::ClosePrinter(hPrinter);
     rv = NS_OK;
   } else {
     rv = NS_ERROR_GFX_PRINTER_NAME_NOT_FOUND;
     PR_PL(("***** nsDeviceContextSpecWin::GetDataFromPrinter - Couldn't open printer: [%s]\n", NS_ConvertUTF16toUTF8(aName).get()));
     DISPLAY_LAST_ERROR
   }
   return rv;
 }