Improved resource management by using ScopedPrinterHandle.

BUG=none
TEST=none


Review URL: http://codereview.chromium.org/9569029

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

chrome/service/cloud_print/print_system_win.cc

@@ -33,6 +33,16 @@
 
 namespace {
 
+class PrinterChangeHandleTraits {
+ public:
+  static bool CloseHandle(HANDLE handle) {
+    return ::FindClosePrinterChangeNotification(handle) != FALSE;
+  }
+};
+
+typedef base::win::GenericScopedHandle<PrinterChangeHandleTraits>
+    ScopedPrinterChangeHandle;
+
 class DevMode {
  public:
   DevMode() : dm_(NULL) {}
@@ -122,9 +132,7 @@ namespace cloud_print {
 class PrintSystemWatcherWin : public base::win::ObjectWatcher::Delegate {
  public:
   PrintSystemWatcherWin()
-      : printer_(NULL),
-        printer_change_(NULL),
-        delegate_(NULL),
+      : delegate_(NULL),
         did_signal_(false) {
   }
   ~PrintSystemWatcherWin() {
@@ -151,11 +159,11 @@ class PrintSystemWatcherWin : public base::win::ObjectWatcher::Delegate {
       printer_name_to_use = const_cast<LPTSTR>(printer_name_wide.c_str());
     }
     bool ret = false;
-    OpenPrinter(printer_name_to_use, &printer_, NULL);
-    if (printer_) {
-      printer_change_ = FindFirstPrinterChangeNotification(
-          printer_, PRINTER_CHANGE_PRINTER|PRINTER_CHANGE_JOB, 0, NULL);
-      if (printer_change_) {
+    OpenPrinter(printer_name_to_use, printer_.Receive(), NULL);
+    if (printer_.IsValid()) {
+      printer_change_.Set(FindFirstPrinterChangeNotification(
+          printer_, PRINTER_CHANGE_PRINTER|PRINTER_CHANGE_JOB, 0, NULL));
+      if (printer_change_.IsValid()) {
         ret = watcher_.StartWatching(printer_change_, this);
       }
     }
@@ -166,14 +174,8 @@ class PrintSystemWatcherWin : public base::win::ObjectWatcher::Delegate {
   }
   bool Stop() {
     watcher_.StopWatching();
-    if (printer_) {
-      ClosePrinter(printer_);
-      printer_ = NULL;
-    }
-    if (printer_change_) {
-      FindClosePrinterChangeNotification(printer_change_);
-      printer_change_ = NULL;
-    }
+    printer_.Close();
+    printer_change_.Close();
     return true;
   }
 
@@ -203,7 +205,7 @@ class PrintSystemWatcherWin : public base::win::ObjectWatcher::Delegate {
 
   bool GetCurrentPrinterInfo(printing::PrinterBasicInfo* printer_info) {
     DCHECK(printer_info);
-    if (!printer_)
+    if (!printer_.IsValid())
       return false;
 
     DWORD bytes_needed = 0;
@@ -234,10 +236,11 @@ class PrintSystemWatcherWin : public base::win::ObjectWatcher::Delegate {
 
  private:
   base::win::ObjectWatcher watcher_;
-  HANDLE printer_;            // The printer being watched
-  HANDLE printer_change_;     // Returned by FindFirstPrinterChangeNotifier
-  Delegate* delegate_;        // Delegate to notify
-  bool did_signal_;           // DoneWaiting was called
+  printing::ScopedPrinterHandle printer_;  // The printer being watched
+  // Returned by FindFirstPrinterChangeNotifier.
+  ScopedPrinterChangeHandle printer_change_;
+  Delegate* delegate_;           // Delegate to notify
+  bool did_signal_;              // DoneWaiting was called
 };
 
 // This typedef is to workaround the issue with certain versions of
@@ -804,13 +807,13 @@ bool PrintSystemWin::GetJobDetails(const std::string& printer_name,
                                    PlatformJobId job_id,
                                    PrintJobDetails *job_details) {
   DCHECK(job_details);
-  HANDLE printer_handle = NULL;
+  printing::ScopedPrinterHandle printer_handle;
   std::wstring printer_name_wide = UTF8ToWide(printer_name);
-  OpenPrinter(const_cast<LPTSTR>(printer_name_wide.c_str()), &printer_handle,
-              NULL);
-  DCHECK(printer_handle);
+  OpenPrinter(const_cast<LPTSTR>(printer_name_wide.c_str()),
+              printer_handle.Receive(), NULL);
+  DCHECK(printer_handle.IsValid());
   bool ret = false;
-  if (printer_handle) {
+  if (printer_handle.IsValid()) {
     DWORD bytes_needed = 0;
     GetJob(printer_handle, job_id, 1, NULL, 0, &bytes_needed);
     DWORD last_error = GetLastError();
@@ -840,7 +843,6 @@ bool PrintSystemWin::GetJobDetails(const std::string& printer_name,
         ret = true;
       }
     }
-    ClosePrinter(printer_handle);
   }
   return ret;
 }

printing/backend/print_backend_win.cc

-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -131,12 +131,11 @@ bool PrintBackendWin::GetPrinterCapsAndDefaults(
       DCHECK(SUCCEEDED(hr));
       printer_info->caps_mime_type = "text/xml";
     }
-    // TODO(sanjeevr): Add ScopedPrinterHandle
-    HANDLE printer_handle = NULL;
-    OpenPrinter(const_cast<LPTSTR>(printer_name_wide.c_str()), &printer_handle,
-                NULL);
+    ScopedPrinterHandle printer_handle;
+    OpenPrinter(const_cast<LPTSTR>(printer_name_wide.c_str()),
+                printer_handle.Receive(), NULL);
     DCHECK(printer_handle);
-    if (printer_handle) {
+    if (printer_handle.IsValid()) {
       LONG devmode_size = DocumentProperties(
           NULL, printer_handle, const_cast<LPTSTR>(printer_name_wide.c_str()),
           NULL, NULL, 0);
@@ -166,7 +165,6 @@ bool PrintBackendWin::GetPrinterCapsAndDefaults(
           printer_info->defaults_mime_type = "text/xml";
         }
       }
-      ClosePrinter(printer_handle);
     }
     XPSModule::CloseProvider(provider);
   }
@@ -175,15 +173,10 @@ bool PrintBackendWin::GetPrinterCapsAndDefaults(
 
 bool PrintBackendWin::IsValidPrinter(const std::string& printer_name) {
   std::wstring printer_name_wide = UTF8ToWide(printer_name);
-  HANDLE printer_handle = NULL;
-  OpenPrinter(const_cast<LPTSTR>(printer_name_wide.c_str()), &printer_handle,
-              NULL);
-  bool ret = false;
-  if (printer_handle) {
-    ret = true;
-    ClosePrinter(printer_handle);
-  }
-  return ret;
+  ScopedPrinterHandle printer_handle;
+  OpenPrinter(const_cast<LPTSTR>(printer_name_wide.c_str()),
+              printer_handle.Receive(), NULL);
+  return printer_handle.IsValid();
 }
 
 scoped_refptr<PrintBackend> PrintBackend::CreateInstance(

printing/printing_context_win.cc

-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -14,6 +14,7 @@
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
+#include "printing/backend/win_helper.h"
 #include "printing/print_job_constants.h"
 #include "printing/print_settings_initializer_win.h"
 #include "printing/printed_document.h"
@@ -381,9 +382,9 @@ PrintingContext::Result PrintingContextWin::UpdatePrinterSettings(
     return OK;
   }
 
-  HANDLE printer;
+  ScopedPrinterHandle printer;
   LPWSTR device_name_wide = const_cast<wchar_t*>(device_name.c_str());
-  if (!OpenPrinter(device_name_wide, &printer, NULL))
+  if (!OpenPrinter(device_name_wide, printer.Receive(), NULL))
     return OnError();
 
   // Make printer changes local to Chrome.
@@ -403,7 +404,6 @@ PrintingContext::Result PrintingContextWin::UpdatePrinterSettings(
   }
   if (dev_mode == NULL) {
     buffer.reset();
-    ClosePrinter(printer);
     return OnError();
   }
 
@@ -433,19 +433,16 @@ PrintingContext::Result PrintingContextWin::UpdatePrinterSettings(
   // Update data using DocumentProperties.
   if (DocumentProperties(NULL, printer, device_name_wide, dev_mode, dev_mode,
                          DM_IN_BUFFER | DM_OUT_BUFFER) != IDOK) {
-    ClosePrinter(printer);
     return OnError();
   }
 
   // Set printer then refresh printer settings.
   if (!AllocateContext(device_name, dev_mode, &context_)) {
-    ClosePrinter(printer);
     return OnError();
   }
   PrintSettingsInitializerWin::InitPrintSettings(context_, *dev_mode,
                                                  ranges, device_name,
                                                  false, &settings_);
-  ClosePrinter(printer);
   return OK;
 }
 
@@ -456,10 +453,9 @@ PrintingContext::Result PrintingContextWin::InitWithSettings(
   settings_ = settings;
 
   // TODO(maruel): settings_.ToDEVMODE()
-  HANDLE printer;
+  ScopedPrinterHandle printer;
   if (!OpenPrinter(const_cast<wchar_t*>(settings_.device_name().c_str()),
-                   &printer,
-                   NULL))
+                   printer.Receive(), NULL))
     return FAILED;
 
   Result status = OK;
@@ -467,9 +463,6 @@ PrintingContext::Result PrintingContextWin::InitWithSettings(
   if (!GetPrinterSettings(printer, settings_.device_name()))
     status = FAILED;
 
-  // Close the printer after retrieving the context.
-  ClosePrinter(printer);
-
   if (status != OK)
     ResetSettings();
   return status;