Bug 1242456 - Create RAIIs to manage HGLOBAL and printer HANDLE in ShowNativePrintDialog and CreateGlobalDevModeAndInit. r=jimm, r=bobowen
authorNgoc Thi Huynh <so61pi.re@gmail.com>
Tue, 05 Apr 2016 13:24:28 +0700
changeset 331465 72e36db09b3a4e61332af101d8eb7ca3f8efb329
parent 331464 a777bd20ea00c5e7f94864c9e2fa31bf4bf2c371
child 331466 fa0b7eb8c9075aa854650f164004ca0ba3035deb
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimm, bobowen
bugs1242456
milestone48.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1242456 - Create RAIIs to manage HGLOBAL and printer HANDLE in ShowNativePrintDialog and CreateGlobalDevModeAndInit. r=jimm, r=bobowen
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,81 @@ 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<nsHGLOBAL>
+CreateGlobalDevModeAndInit(const nsXPIDLString& aPrintName,
+                           nsIPrintSettings* aPS)
 {
-  HGLOBAL hGlobalDevMode = nullptr;
-
-  HANDLE hPrinter = nullptr;
+  nsHPRINTER 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 nsReturnRef<nsHGLOBAL>();
+  }
 
-    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 nsReturnRef<nsHGLOBAL>();
+  }
 
-      // 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 nsReturnRef<nsHGLOBAL>();
+  }
 
-      ::GlobalUnlock(hGlobalDevMode);
-    } else {
-      ::GlobalFree(hGlobalDevMode);
-      hGlobalDevMode = nullptr;
-    }
-
-    ::HeapFree(::GetProcessHeap(), 0, pNewDevMode);
-
-    ::ClosePrinter(hPrinter);
-
-  } else {
-    return nullptr;
+  nsHGLOBAL hDevMode = ::GlobalAlloc(GHND, dwNeeded);
+  nsAutoGlobalMem globalDevMode(hDevMode);
+  if (!hDevMode) {
+    return nsReturnRef<nsHGLOBAL>();
   }
 
-  return hGlobalDevMode;
+  DWORD dwRet = ::DocumentPropertiesW(gParentWnd, hPrinter, printName, newDevMode,
+                                      nullptr, DM_OUT_BUFFER);
+  if (dwRet != 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);
+  // 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(hDevMode);
+    return nsReturnRef<nsHGLOBAL>();
+  }
+
+  ::GlobalUnlock(hDevMode);
+
+  return globalDevMode.out();
 }
 
 //------------------------------------------------------------------
 // helper
 static void GetDefaultPrinterNameFromGlobalPrinters(nsXPIDLString &printerName)
 {
   nsCOMPtr<nsIPrinterEnumerator> prtEnum = do_GetService("@mozilla.org/gfx/printerenumerator;1");
   if (prtEnum) {
@@ -571,73 +565,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));
+  nsHGLOBAL hDevNames = ::GlobalAlloc(GHND, sizeof(wchar_t) * (len + 1)
+                                      + sizeof(DEVNAMES));
+  nsAutoGlobalMem autoDevNames(hDevNames);
   if (!hDevNames) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   DEVNAMES* pDevNames = (DEVNAMES*)::GlobalLock(hDevNames);
   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);
 
   // 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.hDevMode    = autoDevMode.get();
   prntdlg.hDevNames   = hDevNames;
   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;
@@ -677,23 +667,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 +699,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 +754,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 +786,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,112 @@ public:
   static void Release(RawRef aDevMode)
   {
     if (aDevMode != Void()) {
       ::HeapFree(::GetProcessHeap(), 0, aDevMode);
     }
   }
 };
 
+
+// HGLOBAL is just a typedef of HANDLE which nsSimpleRef has a specialization of,
+// that means having a nsAutoRefTraits specialization for HGLOBAL is useless.
+// Therefore we create a wrapper class for HGLOBAL to make nsAutoRefTraits and
+// nsAutoRef work as intention.
+class nsHGLOBAL {
+public:
+  nsHGLOBAL(HGLOBAL hGlobal) : m_hGlobal(hGlobal)
+  {
+  }
+
+  operator HGLOBAL() const
+  {
+    return m_hGlobal;
+  }
+
+private:
+  HGLOBAL m_hGlobal;
+};
+
+
+template<>
+class nsAutoRefTraits<nsHGLOBAL>
+{
+public:
+  typedef nsHGLOBAL RawRef;
+  static RawRef Void()
+  {
+    return nullptr;
+  }
+
+  static void Release(RawRef hGlobal)
+  {
+    ::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 nsHPRINTER {
+public:
+  nsHPRINTER(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<nsHPRINTER>
+{
+public:
+  typedef nsHPRINTER RawRef;
+  static RawRef Void()
+  {
+    return nullptr;
+  }
+
+  static void Release(RawRef hPrinter)
+  {
+    ::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<nsHGLOBAL> nsAutoGlobalMem;
+typedef nsAutoRef<nsHPRINTER> 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;