Bug 1242456 - Create RAII helpers to manage HGLOBAL and HPRINTER in ShowNativePrintDialog and CreateGlobalDevModeAndInit. r=jimm
authorNgoc Thi Huynh <so61pi.re@gmail.com>
Tue, 05 Apr 2016 13:24:28 +0700
changeset 348796 e91dafc1450e6174d57f48d3633a4f351325d720
parent 348667 d6b8e3e0fb9b0b489031b34230d2f90b63550534
child 348797 a80b31406b47c4b8dc6154f89758a16f8faa5ce3
push id14928
push userbmo:mh+mozilla@glandium.org
push dateFri, 08 Apr 2016 05:15:03 +0000
reviewersjimm
bugs1242456
milestone48.0a1
Bug 1242456 - Create RAII helpers to manage HGLOBAL and HPRINTER in ShowNativePrintDialog and CreateGlobalDevModeAndInit. r=jimm MozReview-Commit-ID: 4IrEEgJMtKZ
embedding/components/printingui/win/nsPrintDialogUtil.cpp
embedding/components/printingui/win/nsPrintDialogUtil.h
xpcom/base/nsWindowsHelpers.h
--- a/embedding/components/printingui/win/nsPrintDialogUtil.cpp
+++ b/embedding/components/printingui/win/nsPrintDialogUtil.cpp
@@ -50,16 +50,18 @@ WIN_LIBS=                               
 #include "nsIStringBundle.h"
 
 // For NS_CopyUnicodeToNative
 #include "nsNativeCharsetUtils.h"
 
 // This is for extending the dialog
 #include <dlgs.h>
 
+#include "nsWindowsHelpers.h"
+
 // Default labels for the radio buttons
 static const char* kAsLaidOutOnScreenStr = "As &laid out on the screen";
 static const char* kTheSelectedFrameStr  = "The selected &frame";
 static const char* kEachFrameSeparately  = "&Each frame separately";
 
 
 //-----------------------------------------------
 // Global Data
@@ -457,89 +459,79 @@ static UINT CALLBACK PrintHookProc(HWND 
 //----------------------------------------------------------------------------------
 // Returns a Global Moveable Memory Handle to a DevMode
 // from the Printer by the name of aPrintName
 //
 // NOTE:
 //   This function assumes that aPrintName has already been converted from 
 //   unicode
 //
-HGLOBAL CreateGlobalDevModeAndInit(const nsXPIDLString& aPrintName, nsIPrintSettings* aPS)
+static nsReturnRef<HGLOBAL> CreateGlobalDevModeAndInit(const nsXPIDLString& aPrintName,
+                                                       nsIPrintSettings* aPS)
 {
-  HGLOBAL hGlobalDevMode = nullptr;
-
-  HANDLE hPrinter = nullptr;
+  HPRINTER hPrinter = nullptr;
   // const cast kludge for silly Win32 api's
   LPWSTR printName = const_cast<wchar_t*>(static_cast<const wchar_t*>(aPrintName.get()));
   BOOL status = ::OpenPrinterW(printName, &hPrinter, nullptr);
-  if (status) {
-
-    LPDEVMODEW  pNewDevMode;
-    DWORD       dwNeeded, dwRet;
-
-    // Get the buffer size
-    dwNeeded = ::DocumentPropertiesW(gParentWnd, hPrinter, printName, nullptr, nullptr, 0);
-    if (dwNeeded == 0) {
-      return nullptr;
-    }
-
-    // Allocate a buffer of the correct size.
-    pNewDevMode = (LPDEVMODEW)::HeapAlloc (::GetProcessHeap(), HEAP_ZERO_MEMORY, dwNeeded);
-    if (!pNewDevMode) return nullptr;
+  if (!status) {
+    return nullptr;
+  }
 
-    hGlobalDevMode = (HGLOBAL)::GlobalAlloc(GHND, dwNeeded);
-    if (!hGlobalDevMode) {
-      ::HeapFree(::GetProcessHeap(), 0, pNewDevMode);
-      return nullptr;
-    }
-
-    dwRet = ::DocumentPropertiesW(gParentWnd, hPrinter, printName, pNewDevMode, nullptr, DM_OUT_BUFFER);
-
-    if (dwRet != IDOK) {
-      ::HeapFree(::GetProcessHeap(), 0, pNewDevMode);
-      ::GlobalFree(hGlobalDevMode);
-      ::ClosePrinter(hPrinter);
-      return nullptr;
-    }
+  // Make sure hPrinter is closed on all paths
+  nsAutoPrinter autoPrinter(hPrinter);
 
-    // Lock memory and copy contents from DEVMODE (current printer)
-    // to Global Memory DEVMODE
-    LPDEVMODEW devMode = (DEVMODEW *)::GlobalLock(hGlobalDevMode);
-    if (devMode) {
-      memcpy(devMode, pNewDevMode, dwNeeded);
-      // Initialize values from the PrintSettings
-      nsCOMPtr<nsIPrintSettingsWin> psWin = do_QueryInterface(aPS);
-      MOZ_ASSERT(psWin);
-      psWin->CopyToNative(devMode);
+  // Get the buffer size
+  DWORD dwNeeded = ::DocumentPropertiesW(gParentWnd, hPrinter, printName, nullptr,
+                                         nullptr, 0);
+  if (dwNeeded == 0) {
+    return nullptr;
+  }
 
-      // Sets back the changes we made to the DevMode into the Printer Driver
-      dwRet = ::DocumentPropertiesW(gParentWnd, hPrinter, printName, devMode, devMode, DM_IN_BUFFER | DM_OUT_BUFFER);
-      if (dwRet != IDOK) {
-        ::GlobalUnlock(hGlobalDevMode);
-        ::GlobalFree(hGlobalDevMode);
-        ::HeapFree(::GetProcessHeap(), 0, pNewDevMode);
-        ::ClosePrinter(hPrinter);
-        return nullptr;
-      }
+  // Allocate a buffer of the correct size.
+  nsAutoDevMode newDevMode((LPDEVMODEW)::HeapAlloc(::GetProcessHeap(), HEAP_ZERO_MEMORY,
+                                                   dwNeeded));
+  if (!newDevMode) {
+    return nullptr;
+  }
 
-      ::GlobalUnlock(hGlobalDevMode);
-    } else {
-      ::GlobalFree(hGlobalDevMode);
-      hGlobalDevMode = nullptr;
-    }
-
-    ::HeapFree(::GetProcessHeap(), 0, pNewDevMode);
-
-    ::ClosePrinter(hPrinter);
-
-  } else {
+  nsAutoGlobalMem globalDevMode((HGLOBAL)::GlobalAlloc(GHND, dwNeeded));
+  if (!globalDevMode) {
     return nullptr;
   }
 
-  return hGlobalDevMode;
+  DWORD dwRet = ::DocumentPropertiesW(gParentWnd, hPrinter, printName, newDevMode,
+                                      nullptr, DM_OUT_BUFFER);
+  if (dwRet != IDOK) {
+    return nullptr;
+  }
+
+  // Lock memory and copy contents from DEVMODE (current printer)
+  // to Global Memory DEVMODE
+  LPDEVMODEW devMode = (DEVMODEW *)::GlobalLock(globalDevMode);
+  if (!devMode) {
+    return nullptr;
+  }
+
+  memcpy(devMode, newDevMode.get(), dwNeeded);
+  // 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,
+                                DM_IN_BUFFER | DM_OUT_BUFFER);
+  if (dwRet != IDOK) {
+    ::GlobalUnlock(globalDevMode);
+    return nullptr;
+  }
+
+  ::GlobalUnlock(globalDevMode);
+
+  return globalDevMode.out();
 }
 
 //------------------------------------------------------------------
 // helper
 static void GetDefaultPrinterNameFromGlobalPrinters(nsXPIDLString &printerName)
 {
   nsCOMPtr<nsIPrinterEnumerator> prtEnum = do_GetService("@mozilla.org/gfx/printerenumerator;1");
   if (prtEnum) {
@@ -571,74 +563,69 @@ static nsresult
 ShowNativePrintDialog(HWND              aHWnd,
                       nsIPrintSettings* aPrintSettings)
 {
   //NS_ENSURE_ARG_POINTER(aHWnd);
   NS_ENSURE_ARG_POINTER(aPrintSettings);
 
   gDialogWasExtended  = false;
 
-  HGLOBAL hGlobalDevMode = nullptr;
-  HGLOBAL hDevNames      = nullptr;
-
   // Get the Print Name to be used
   nsXPIDLString printerName;
   aPrintSettings->GetPrinterName(getter_Copies(printerName));
 
   // If there is no name then use the default printer
   if (printerName.IsEmpty()) {
     GetDefaultPrinterNameFromGlobalPrinters(printerName);
   } else {
     HANDLE hPrinter = nullptr;
-    if(!::OpenPrinterW(const_cast<wchar_t*>(static_cast<const wchar_t*>(printerName.get())), &hPrinter, nullptr)) {
+    if(!::OpenPrinterW(const_cast<wchar_t*>(static_cast<const wchar_t*>(printerName.get())),
+                       &hPrinter, nullptr)) {
       // If the last used printer is not found, we should use default printer.
       GetDefaultPrinterNameFromGlobalPrinters(printerName);
     } else {
       ::ClosePrinter(hPrinter);
     }
   }
 
   // Now create a DEVNAMES struct so the the dialog is initialized correctly.
 
   uint32_t len = printerName.Length();
-  hDevNames = (HGLOBAL)::GlobalAlloc(GHND, sizeof(wchar_t) * (len + 1) + 
-                                     sizeof(DEVNAMES));
-  if (!hDevNames) {
+  nsAutoGlobalMem autoDevNames((HGLOBAL)::GlobalAlloc(GHND, sizeof(wchar_t) * (len + 1) +
+                                                      sizeof(DEVNAMES)));
+  if (!autoDevNames) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
-  DEVNAMES* pDevNames = (DEVNAMES*)::GlobalLock(hDevNames);
+  DEVNAMES* pDevNames = (DEVNAMES*)::GlobalLock(autoDevNames);
   if (!pDevNames) {
-    ::GlobalFree(hDevNames);
     return NS_ERROR_FAILURE;
   }
   pDevNames->wDriverOffset = sizeof(DEVNAMES)/sizeof(wchar_t);
   pDevNames->wDeviceOffset = sizeof(DEVNAMES)/sizeof(wchar_t);
   pDevNames->wOutputOffset = sizeof(DEVNAMES)/sizeof(wchar_t)+len;
   pDevNames->wDefault      = 0;
 
   memcpy(pDevNames+1, printerName, (len + 1) * sizeof(wchar_t));
-  ::GlobalUnlock(hDevNames);
+  ::GlobalUnlock(autoDevNames);
 
   // Create a Moveable Memory Object that holds a new DevMode
   // from the Printer Name
   // The PRINTDLG.hDevMode requires that it be a moveable memory object
-  // NOTE: We only need to free hGlobalDevMode when the dialog is cancelled
-  // When the user prints, it comes back in the printdlg struct and 
-  // is used and cleaned up later
-  hGlobalDevMode = CreateGlobalDevModeAndInit(printerName, aPrintSettings);
+  // NOTE: autoDevMode is automatically freed when any error occurred
+  nsAutoGlobalMem autoDevMode(CreateGlobalDevModeAndInit(printerName, aPrintSettings));
 
   // Prepare to Display the Print Dialog
   PRINTDLGW  prntdlg;
   memset(&prntdlg, 0, sizeof(PRINTDLGW));
 
   prntdlg.lStructSize = sizeof(prntdlg);
   prntdlg.hwndOwner   = aHWnd;
-  prntdlg.hDevMode    = hGlobalDevMode;
-  prntdlg.hDevNames   = hDevNames;
+  prntdlg.hDevMode    = autoDevMode;
+  prntdlg.hDevNames   = autoDevNames;
   prntdlg.hDC         = nullptr;
   prntdlg.Flags       = PD_ALLPAGES | PD_RETURNIC | 
                         PD_USEDEVMODECOPIESANDCOLLATE | PD_COLLATE;
 
   // if there is a current selection then enable the "Selection" radio button
   int16_t howToEnableFrameUI = nsIPrintSettings::kFrameEnableNone;
   bool isOn;
   aPrintSettings->GetPrintOptions(nsIPrintSettings::kEnableSelectionRB, &isOn);
@@ -677,23 +664,21 @@ ShowNativePrintDialog(HWND              
 
   BOOL result = ::PrintDlgW(&prntdlg);
 
   if (TRUE == result) {
     // check to make sure we don't have any nullptr pointers
     NS_ENSURE_TRUE(aPrintSettings && prntdlg.hDevMode, NS_ERROR_FAILURE);
 
     if (prntdlg.hDevNames == nullptr) {
-      ::GlobalFree(hGlobalDevMode);
       return NS_ERROR_FAILURE;
     }
     // Lock the deviceNames and check for nullptr
     DEVNAMES *devnames = (DEVNAMES *)::GlobalLock(prntdlg.hDevNames);
     if (devnames == nullptr) {
-      ::GlobalFree(hGlobalDevMode);
       return NS_ERROR_FAILURE;
     }
 
     char16_t* device = &(((char16_t *)devnames)[devnames->wDeviceOffset]);
     char16_t* driver = &(((char16_t *)devnames)[devnames->wDriverOffset]);
 
     // Check to see if the "Print To File" control is checked
     // then take the name from devNames and set it in the PrintSettings
@@ -711,17 +696,16 @@ ShowNativePrintDialog(HWND              
     } else {
       // clear "print to file" info
       aPrintSettings->SetPrintToFile(false);
       aPrintSettings->SetToFileName(nullptr);
     }
 
     nsCOMPtr<nsIPrintSettingsWin> psWin(do_QueryInterface(aPrintSettings));
     if (!psWin) {
-      ::GlobalFree(hGlobalDevMode);
       return NS_ERROR_FAILURE;
     }
 
     // Setup local Data members
     psWin->SetDeviceName(device);
     psWin->SetDriverName(driver);
 
 #if defined(DEBUG_rods) || defined(DEBUG_dcone)
@@ -767,17 +751,16 @@ ShowNativePrintDialog(HWND              
       aPrintSettings->SetPrintFrameType(nsIPrintSettings::kNoFrames);
     }
     // Unlock DeviceNames
     ::GlobalUnlock(prntdlg.hDevNames);
 
     // Transfer the settings from the native data to the PrintSettings
     LPDEVMODEW devMode = (LPDEVMODEW)::GlobalLock(prntdlg.hDevMode);
     if (!devMode || !prntdlg.hDC) {
-      ::GlobalFree(hGlobalDevMode);
       return NS_ERROR_FAILURE;
     }
     psWin->SetDevMode(devMode); // copies DevMode
     psWin->CopyFromNative(prntdlg.hDC, devMode);
     ::GlobalUnlock(prntdlg.hDevMode);
     ::DeleteDC(prntdlg.hDC);
 
 #if defined(DEBUG_rods) || defined(DEBUG_dcone)
@@ -800,17 +783,16 @@ ShowNativePrintDialog(HWND              
     } else {
       printf("Printing from page no. %d to %d\n", fromPageNum, toPageNum);
     }
 #endif
     
   } else {
     ::SetFocus(aHWnd);
     aPrintSettings->SetIsCancelled(true);
-    if (hGlobalDevMode) ::GlobalFree(hGlobalDevMode);
     return NS_ERROR_ABORT;
   }
 
   return NS_OK;
 }
 
 //------------------------------------------------------------------
 static void 
--- a/embedding/components/printingui/win/nsPrintDialogUtil.h
+++ b/embedding/components/printingui/win/nsPrintDialogUtil.h
@@ -4,11 +4,9 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 #ifndef nsFlyOwnDialog_h___
 #define nsFlyOwnDialog_h___
 
 nsresult NativeShowPrintDialog(HWND                aHWnd,
                                nsIWebBrowserPrint* aWebBrowserPrint,
                                nsIPrintSettings*   aPrintSettings);
 
-HGLOBAL CreateGlobalDevModeAndInit(const nsXPIDLString& aPrintName, nsIPrintSettings* aPS);
-
 #endif /* nsFlyOwnDialog_h___ */
--- a/xpcom/base/nsWindowsHelpers.h
+++ b/xpcom/base/nsWindowsHelpers.h
@@ -207,25 +207,96 @@ public:
   static void Release(RawRef aDevMode)
   {
     if (aDevMode != Void()) {
       ::HeapFree(::GetProcessHeap(), 0, aDevMode);
     }
   }
 };
 
+
+template<>
+class nsAutoRefTraits<HGLOBAL>
+{
+public:
+  typedef HGLOBAL RawRef;
+  static RawRef Void()
+  {
+    return nullptr;
+  }
+
+  static void Release(RawRef hGlobal)
+  {
+    if (hGlobal != Void()) {
+      ::GlobalFree(hGlobal);
+    }
+  }
+};
+
+
+// because Printer's HANDLE uses ClosePrinter and we already have nsAutoRef<HANDLE>
+// which uses CloseHandle so we need to create a wrapper class for HANDLE to have
+// another specialization for nsAutoRefTraits
+class HPRINTER {
+public:
+  HPRINTER(HANDLE hPrinter) : m_hPrinter(hPrinter)
+  {
+  }
+
+  operator HANDLE() const
+  {
+    return m_hPrinter;
+  }
+
+  HANDLE* operator&()
+  {
+    return &m_hPrinter;
+  }
+
+private:
+    HANDLE m_hPrinter;
+};
+
+
+// winspool.h header has AddMonitor macro, it conflicts with AddMonitor member
+// function in TaskbarPreview.cpp and TaskbarTabPreview.cpp. Beside, we only
+// need ClosePrinter here for Release function, so having its prototype is enough
+extern "C" BOOL WINAPI ClosePrinter(HANDLE hPrinter);
+
+
+template<>
+class nsAutoRefTraits<HPRINTER>
+{
+public:
+  typedef HPRINTER RawRef;
+  static RawRef Void()
+  {
+    return nullptr;
+  }
+
+  static void Release(RawRef hPrinter)
+  {
+    if (hPrinter != Void()) {
+      ::ClosePrinter(hPrinter);
+    }
+  }
+};
+
+
 typedef nsAutoRef<HKEY> nsAutoRegKey;
 typedef nsAutoRef<HDC> nsAutoHDC;
 typedef nsAutoRef<HBRUSH> nsAutoBrush;
 typedef nsAutoRef<HRGN> nsAutoRegion;
 typedef nsAutoRef<HBITMAP> nsAutoBitmap;
 typedef nsAutoRef<SC_HANDLE> nsAutoServiceHandle;
 typedef nsAutoRef<HANDLE> nsAutoHandle;
 typedef nsAutoRef<HMODULE> nsModuleHandle;
 typedef nsAutoRef<DEVMODEW*> nsAutoDevMode;
+typedef nsAutoRef<HGLOBAL> nsAutoGlobalMem;
+typedef nsAutoRef<HPRINTER> nsAutoPrinter;
 
 namespace {
 
 // Construct a path "<system32>\<aModule>". return false if the output buffer
 // is too small.
 // Note: If the system path cannot be found, or doesn't fit in the output buffer
 // with the module name, we will just ignore the system path and output the
 // module name alone;