Don't close handles returned by ::OpenPrinter with error.

BUG=248061

Review URL: https://chromiumcodereview.appspot.com/15798023

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@205600 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 29 insertions, 20 deletions

diff --git a/printing/backend/print_backend_win.cc b/printing/backend/print_backend_win.cc
index 521207b..0d1958f 100644
--- a/printing/backend/print_backend_win.cc
+++ b/printing/backend/print_backend_win.cc
@@ -80,9 +80,9 @@ bool PrintBackendWin::EnumeratePrinters(PrinterList* printer_list) {
       reinterpret_cast<PRINTER_INFO_4*>(printer_info_buffer.get());
   for (DWORD index = 0; index < count_returned; index++) {
     ScopedPrinterHandle printer;
-    OpenPrinter(printer_info[index].pPrinterName, printer.Receive(), NULL);
     PrinterBasicInfo info;
-    if (InitBasicPrinterInfo(printer, &info)) {
+    if (printer.OpenPrinter(printer_info[index].pPrinterName) &&
+        InitBasicPrinterInfo(printer, &info)) {
       info.is_default = (info.printer_name == default_printer);
       printer_list->push_back(info);
     }
@@ -102,10 +102,7 @@ bool PrintBackendWin::GetPrinterSemanticCapsAndDefaults(
     const std::string& printer_name,
     PrinterSemanticCapsAndDefaults* printer_info) {
   ScopedPrinterHandle printer_handle;
-  OpenPrinter(const_cast<LPTSTR>(UTF8ToWide(printer_name).c_str()),
-              printer_handle.Receive(), NULL);
-  DCHECK(printer_handle);
-  if (!printer_handle.IsValid()) {
+  if (!printer_handle.OpenPrinter(UTF8ToWide(printer_name).c_str())) {
     LOG(WARNING) << "Failed to open printer, error = " << GetLastError();
     return false;
   }
@@ -199,10 +196,7 @@ bool PrintBackendWin::GetPrinterCapsAndDefaults(
       printer_info->caps_mime_type = "text/xml";
     }
     ScopedPrinterHandle printer_handle;
-    OpenPrinter(const_cast<LPTSTR>(printer_name_wide.c_str()),
-                printer_handle.Receive(), NULL);
-    DCHECK(printer_handle);
-    if (printer_handle.IsValid()) {
+    if (printer_handle.OpenPrinter(printer_name_wide.c_str())) {
       LONG devmode_size = DocumentProperties(
           NULL, printer_handle, const_cast<LPTSTR>(printer_name_wide.c_str()),
           NULL, NULL, 0);
@@ -242,8 +236,7 @@ bool PrintBackendWin::GetPrinterCapsAndDefaults(
 std::string PrintBackendWin::GetPrinterDriverInfo(
     const std::string& printer_name) {
   ScopedPrinterHandle printer;
-  if (!::OpenPrinter(const_cast<LPTSTR>(UTF8ToWide(printer_name).c_str()),
-                     printer.Receive(), NULL)) {
+  if (!printer.OpenPrinter(UTF8ToWide(printer_name).c_str())) {
     return std::string();
   }
   return GetDriverInfo(printer);
@@ -251,9 +244,7 @@ std::string PrintBackendWin::GetPrinterDriverInfo(
 
 bool PrintBackendWin::IsValidPrinter(const std::string& printer_name) {
   ScopedPrinterHandle printer_handle;
-  OpenPrinter(const_cast<LPTSTR>(UTF8ToWide(printer_name).c_str()),
-              printer_handle.Receive(), NULL);
-  return printer_handle.IsValid();
+  return printer_handle.OpenPrinter(UTF8ToWide(printer_name).c_str());
 }
 
 scoped_refptr<PrintBackend> PrintBackend::CreateInstance(
diff --git a/printing/backend/win_helper.h b/printing/backend/win_helper.h
index 0401433..d0f8b35 100644
--- a/printing/backend/win_helper.h
+++ b/printing/backend/win_helper.h
@@ -41,8 +41,26 @@ class PrinterHandleTraits {
   DISALLOW_IMPLICIT_CONSTRUCTORS(PrinterHandleTraits);
 };
 
-typedef base::win::GenericScopedHandle<
-    PrinterHandleTraits, base::win::VerifierTraits> ScopedPrinterHandle;
+class ScopedPrinterHandle
+    : public base::win::GenericScopedHandle<PrinterHandleTraits,
+                                            base::win::VerifierTraits> {
+ public:
+  bool OpenPrinter(const wchar_t* printer) {
+    // ::OpenPrinter may return error but assign some value into handle.
+    if (!::OpenPrinter(const_cast<LPTSTR>(printer), Receive(), NULL)) {
+      Take();
+    }
+    return IsValid();
+  }
+
+ private:
+  typedef base::win::GenericScopedHandle<PrinterHandleTraits,
+                                         base::win::VerifierTraits> Base;
+  // Hide Receive to avoid assigning handle when ::OpenPrinter returned error.
+  Base::Receiver Receive() {
+    return Base::Receive();
+  }
+};
 
 // Wrapper class to wrap the XPS APIs (PTxxx APIs). This is needed because these
 // APIs are not available by default on XP. We could delayload prntvpt.dll but
diff --git a/printing/printing_context_win.cc b/printing/printing_context_win.cc
index b5a57cf..46aa98f 100644
--- a/printing/printing_context_win.cc
+++ b/printing/printing_context_win.cc
@@ -371,7 +371,7 @@ PrintingContext::Result PrintingContextWin::UpdatePrinterSettings(
 
   ScopedPrinterHandle printer;
   LPWSTR device_name_wide = const_cast<wchar_t*>(device_name.c_str());
-  if (!OpenPrinter(device_name_wide, printer.Receive(), NULL))
+  if (!printer.OpenPrinter(device_name_wide))
     return OnError();
 
   // Make printer changes local to Chrome.
@@ -441,9 +441,9 @@ PrintingContext::Result PrintingContextWin::InitWithSettings(
 
   // TODO(maruel): settings_.ToDEVMODE()
   ScopedPrinterHandle printer;
-  if (!OpenPrinter(const_cast<wchar_t*>(settings_.device_name().c_str()),
-                   printer.Receive(), NULL))
+  if (!printer.OpenPrinter(settings_.device_name().c_str())) {
     return FAILED;
+  }
 
   Result status = OK;