Backed out changeset e91dafc1450e (bug 1242456) due to incorrect HGLOBAL handling.
authorBob Owen <bobowencode@gmail.com>
Fri, 08 Apr 2016 16:37:04 +0100
changeset 316244 3b202a7788c5a5b5ea788024cddb6754cd03d89d
parent 316243 77d7242cc3375e5f659b641372cee95ba411d536
child 316245 77c21ffcbd8183c7a4ea71dd281b38ad5fb755b8
push id9480
push userjlund@mozilla.com
push dateMon, 25 Apr 2016 17:12:58 +0000
treeherdermozilla-aurora@0d6a91c76a9e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1242456
milestone48.0a1
backs oute91dafc1450e6174d57f48d3633a4f351325d720
Backed out changeset e91dafc1450e (bug 1242456) due to incorrect HGLOBAL handling.
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,18 +50,16 @@ 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
@@ -459,79 +457,89 @@ 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
 //
-static nsReturnRef<HGLOBAL> CreateGlobalDevModeAndInit(const nsXPIDLString& aPrintName,
-                                                       nsIPrintSettings* aPS)
+HGLOBAL CreateGlobalDevModeAndInit(const nsXPIDLString& aPrintName, nsIPrintSettings* aPS)
 {
-  HPRINTER hPrinter = nullptr;
+  HGLOBAL hGlobalDevMode = nullptr;
+
+  HANDLE 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) {
-    return 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;
 
-  // Make sure hPrinter is closed on all paths
-  nsAutoPrinter autoPrinter(hPrinter);
+    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;
+    }
 
-  // Get the buffer size
-  DWORD dwNeeded = ::DocumentPropertiesW(gParentWnd, hPrinter, printName, nullptr,
-                                         nullptr, 0);
-  if (dwNeeded == 0) {
-    return nullptr;
-  }
+    // 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);
 
-  // Allocate a buffer of the correct size.
-  nsAutoDevMode newDevMode((LPDEVMODEW)::HeapAlloc(::GetProcessHeap(), HEAP_ZERO_MEMORY,
-                                                   dwNeeded));
-  if (!newDevMode) {
+      // 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;
+      }
+
+      ::GlobalUnlock(hGlobalDevMode);
+    } else {
+      ::GlobalFree(hGlobalDevMode);
+      hGlobalDevMode = nullptr;
+    }
+
+    ::HeapFree(::GetProcessHeap(), 0, pNewDevMode);
+
+    ::ClosePrinter(hPrinter);
+
+  } else {
     return nullptr;
   }
 
-  nsAutoGlobalMem globalDevMode((HGLOBAL)::GlobalAlloc(GHND, dwNeeded));
-  if (!globalDevMode) {
-    return nullptr;
-  }
-
-  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();
+  return hGlobalDevMode;
 }
 
 //------------------------------------------------------------------
 // helper
 static void GetDefaultPrinterNameFromGlobalPrinters(nsXPIDLString &printerName)
 {
   nsCOMPtr<nsIPrinterEnumerator> prtEnum = do_GetService("@mozilla.org/gfx/printerenumerator;1");
   if (prtEnum) {
@@ -563,69 +571,74 @@ 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();
-  nsAutoGlobalMem autoDevNames((HGLOBAL)::GlobalAlloc(GHND, sizeof(wchar_t) * (len + 1) +
-                                                      sizeof(DEVNAMES)));
-  if (!autoDevNames) {
+  hDevNames = (HGLOBAL)::GlobalAlloc(GHND, sizeof(wchar_t) * (len + 1) + 
+                                     sizeof(DEVNAMES));
+  if (!hDevNames) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
-  DEVNAMES* pDevNames = (DEVNAMES*)::GlobalLock(autoDevNames);
+  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(autoDevNames);
+  ::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: autoDevMode is automatically freed when any error occurred
-  nsAutoGlobalMem autoDevMode(CreateGlobalDevModeAndInit(printerName, aPrintSettings));
+  // 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);
 
   // Prepare to Display the Print Dialog
   PRINTDLGW  prntdlg;
   memset(&prntdlg, 0, sizeof(PRINTDLGW));
 
   prntdlg.lStructSize = sizeof(prntdlg);
   prntdlg.hwndOwner   = aHWnd;
-  prntdlg.hDevMode    = autoDevMode;
-  prntdlg.hDevNames   = autoDevNames;
+  prntdlg.hDevMode    = hGlobalDevMode;
+  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;
   aPrintSettings->GetPrintOptions(nsIPrintSettings::kEnableSelectionRB, &isOn);
@@ -664,21 +677,23 @@ 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
@@ -696,16 +711,17 @@ 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)
@@ -751,16 +767,17 @@ 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)
@@ -783,16 +800,17 @@ 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,9 +4,11 @@
  * 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,96 +207,25 @@ 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;