Mark Windows print API calls as blocking

https://crrev.com/c/2135054 resolved a threading issue by moving print
operations to the browser UI thread on Windows. Prior to that CL, the
work being done was marked as blocking at the task level, rather than
at the individual API level. The blocking annotations were removed in
order to not have a ScopedBlockingCall immediately preceded by a
ScopedAllowBlocking.

This CL reintroduces the ScopedBlockingCall marks at the point where
potentially blocking APIs are actually called. Existing callers also
gain ScopedAllowBlocking as needed.

Bug: 1067666
Change-Id: Ifb96feb96f0e6ba38ac06f09f0aa9640c547dba2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2128690
Commit-Queue: Kevin Babbitt <kbabbitt@microsoft.com>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Raymes Khoury <raymes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756910}

base/threading/thread_restrictions.h

@@ -147,6 +147,7 @@ class CategorizedWorkerPool;
 class DesktopCaptureDevice;
 class InProcessUtilityThread;
 class NestedMessagePumpAndroid;
+class PepperPrintSettingsManagerImpl;
 class RenderProcessHostImpl;
 class RenderWidgetHostViewMac;
 class RTCVideoDecoder;
@@ -365,6 +366,7 @@ class BASE_EXPORT ScopedAllowBlocking {
   friend class android_webview::ScopedAllowInitGLBindings;
   friend class chromeos::MojoUtils;  // http://crbug.com/1055467
   friend class content::BrowserProcessSubThread;
+  friend class content::PepperPrintSettingsManagerImpl;
   friend class content::RenderProcessHostImpl;
   friend class content::RenderWidgetHostViewMac;  // http://crbug.com/121917
   friend class content::WebContentsViewMac;

chrome/browser/printing/print_job_worker.cc

@@ -222,8 +222,15 @@ void PrintJobWorker::UpdatePrintSettings(base::Value new_settings,
         print_backend->GetPrinterDriverInfo(printer_name));
   }
 
-  PrintingContext::Result result =
-      printing_context_->UpdatePrintSettings(std::move(new_settings));
+  PrintingContext::Result result;
+  {
+#if defined(OS_WIN)
+    // Blocking is needed here because Windows printer drivers are oftentimes
+    // not thread-safe and have to be accessed on the UI thread.
+    base::ScopedAllowBlocking allow_blocking;
+#endif
+    result = printing_context_->UpdatePrintSettings(std::move(new_settings));
+  }
   GetSettingsDone(std::move(callback), result);
 }
 

content/browser/renderer_host/pepper/pepper_print_settings_manager.cc

@@ -5,6 +5,7 @@
 #include "content/browser/renderer_host/pepper/pepper_print_settings_manager.h"
 
 #include "base/task/post_task.h"
+#include "build/build_config.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
@@ -12,6 +13,10 @@
 #include "ppapi/c/pp_errors.h"
 #include "printing/buildflags/buildflags.h"
 
+#if defined(OS_WIN)
+#include "base/threading/thread_restrictions.h"
+#endif
+
 #if BUILDFLAG(ENABLE_PRINT_PREVIEW)
 #include "printing/printing_context.h"  // nogncheck
 #include "printing/units.h"  // nogncheck
@@ -59,11 +64,23 @@ class PrintingContextDelegate : public printing::PrintingContext::Delegate {
   }
 };
 
-PepperPrintSettingsManager::Result ComputeDefaultPrintSettings() {
+#endif
+
+}  // namespace
+
+PepperPrintSettingsManager::Result
+PepperPrintSettingsManagerImpl::ComputeDefaultPrintSettings() {
+#if BUILDFLAG(ENABLE_PRINT_PREVIEW)
   // This function should run on the UI thread because |PrintingContext| methods
   // call into platform APIs.
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
+#if defined(OS_WIN)
+  // Blocking is needed here because Windows printer drivers are oftentimes
+  // not thread-safe and have to be accessed on the UI thread.
+  base::ScopedAllowBlocking allow_blocking;
+#endif
+
   PrintingContextDelegate delegate;
   std::unique_ptr<printing::PrintingContext> context(
       printing::PrintingContext::Create(&delegate));
@@ -101,15 +118,11 @@ PepperPrintSettingsManager::Result ComputeDefaultPrintSettings() {
   // so just make it the default.
   settings.format = PP_PRINTOUTPUTFORMAT_PDF;
   return PepperPrintSettingsManager::Result(settings, PP_OK);
-}
 #else
-PepperPrintSettingsManager::Result ComputeDefaultPrintSettings() {
   return PepperPrintSettingsManager::Result(PP_PrintSettings_Dev(),
                                             PP_ERROR_NOTSUPPORTED);
-}
 #endif
-
-}  // namespace
+}
 
 void PepperPrintSettingsManagerImpl::GetDefaultPrintSettings(
     PepperPrintSettingsManager::Callback callback) {

content/browser/renderer_host/pepper/pepper_print_settings_manager.h

@@ -42,6 +42,8 @@ class CONTENT_EXPORT PepperPrintSettingsManagerImpl
       PepperPrintSettingsManager::Callback callback) override;
 
  private:
+  static PepperPrintSettingsManager::Result ComputeDefaultPrintSettings();
+
   DISALLOW_COPY_AND_ASSIGN(PepperPrintSettingsManagerImpl);
 };
 

printing/backend/print_backend_win.cc

@@ -16,6 +16,7 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/threading/scoped_blocking_call.h"
 #include "base/win/scoped_bstr.h"
 #include "base/win/scoped_hglobal.h"
 #include "printing/backend/print_backend_consts.h"
@@ -222,6 +223,8 @@ std::string PrintBackendWin::GetDefaultPrinterName() {
   DWORD size = MAX_PATH;
   TCHAR default_printer_name[MAX_PATH];
   std::string ret;
+  base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
+                                                base::BlockingType::MAY_BLOCK);
   if (::GetDefaultPrinter(default_printer_name, &size))
     ret = base::WideToUTF8(default_printer_name);
   return ret;

printing/backend/win_helper.cc

@@ -20,6 +20,7 @@
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/threading/scoped_blocking_call.h"
 #include "base/win/windows_version.h"
 #include "printing/backend/print_backend.h"
 #include "printing/backend/print_backend_consts.h"
@@ -212,6 +213,8 @@ bool XPSModule::InitImpl() {
 HRESULT XPSModule::OpenProvider(const base::string16& printer_name,
                                 DWORD version,
                                 HPTPROVIDER* provider) {
+  base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
+                                                base::BlockingType::MAY_BLOCK);
   return g_open_provider_proc(printer_name.c_str(), version, provider);
 }
 
@@ -219,6 +222,8 @@ HRESULT XPSModule::GetPrintCapabilities(HPTPROVIDER provider,
                                         IStream* print_ticket,
                                         IStream* capabilities,
                                         BSTR* error_message) {
+  base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
+                                                base::BlockingType::MAY_BLOCK);
   return g_get_print_capabilities_proc(provider, print_ticket, capabilities,
                                        error_message);
 }
@@ -228,6 +233,8 @@ HRESULT XPSModule::ConvertDevModeToPrintTicket(HPTPROVIDER provider,
                                                PDEVMODE devmode,
                                                EPrintTicketScope scope,
                                                IStream* print_ticket) {
+  base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
+                                                base::BlockingType::MAY_BLOCK);
   return g_convert_devmode_to_print_ticket_proc(provider, devmode_size_in_bytes,
                                                 devmode, scope, print_ticket);
 }
@@ -240,6 +247,8 @@ HRESULT XPSModule::ConvertPrintTicketToDevMode(
     ULONG* devmode_byte_count,
     PDEVMODE* devmode,
     BSTR* error_message) {
+  base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
+                                                base::BlockingType::MAY_BLOCK);
   return g_convert_print_ticket_to_devmode_proc(
       provider, print_ticket, base_devmode_type, scope, devmode_byte_count,
       devmode, error_message);
@@ -251,15 +260,21 @@ HRESULT XPSModule::MergeAndValidatePrintTicket(HPTPROVIDER provider,
                                                EPrintTicketScope scope,
                                                IStream* result_ticket,
                                                BSTR* error_message) {
+  base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
+                                                base::BlockingType::MAY_BLOCK);
   return g_merge_and_validate_print_ticket_proc(
       provider, base_ticket, delta_ticket, scope, result_ticket, error_message);
 }
 
 HRESULT XPSModule::ReleaseMemory(PVOID buffer) {
+  base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
+                                                base::BlockingType::MAY_BLOCK);
   return g_release_memory_proc(buffer);
 }
 
 HRESULT XPSModule::CloseProvider(HPTPROVIDER provider) {
+  base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
+                                                base::BlockingType::MAY_BLOCK);
   return g_close_provider_proc(provider);
 }
 
@@ -489,14 +504,21 @@ std::unique_ptr<DEVMODE, base::FreeDeleter> CreateDevModeWithColor(
 }
 
 bool PrinterHasValidPaperSize(const wchar_t* name, const wchar_t* port) {
+  base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
+                                                base::BlockingType::MAY_BLOCK);
   return DeviceCapabilities(name, port, DC_PAPERSIZE, nullptr, nullptr) > 0;
 }
 
 std::unique_ptr<DEVMODE, base::FreeDeleter> CreateDevMode(HANDLE printer,
                                                           DEVMODE* in) {
   wchar_t* device_name_ptr = const_cast<wchar_t*>(L"");
-  LONG buffer_size = DocumentProperties(nullptr, printer, device_name_ptr,
-                                        nullptr, nullptr, 0);
+  LONG buffer_size;
+  {
+    base::ScopedBlockingCall scoped_blocking_call(
+        FROM_HERE, base::BlockingType::MAY_BLOCK);
+    buffer_size = DocumentProperties(nullptr, printer, device_name_ptr, nullptr,
+                                     nullptr, 0);
+  }
   if (buffer_size < static_cast<int>(sizeof(DEVMODE)))
     return nullptr;
 
@@ -521,9 +543,13 @@ std::unique_ptr<DEVMODE, base::FreeDeleter> CreateDevMode(HANDLE printer,
     return nullptr;
   }
 
-  if (DocumentProperties(nullptr, printer, device_name_ptr, out.get(), in,
-                         flags) != IDOK) {
-    return nullptr;
+  {
+    base::ScopedBlockingCall scoped_blocking_call(
+        FROM_HERE, base::BlockingType::MAY_BLOCK);
+    if (DocumentProperties(nullptr, printer, device_name_ptr, out.get(), in,
+                           flags) != IDOK) {
+      return nullptr;
+    }
   }
 
   int size = out->dmSize;
@@ -559,8 +585,14 @@ std::unique_ptr<DEVMODE, base::FreeDeleter> PromptDevMode(
   std::unique_ptr<DEVMODE, base::FreeDeleter> out(
       reinterpret_cast<DEVMODE*>(calloc(buffer_size, 1)));
   DWORD flags = (in ? (DM_IN_BUFFER) : 0) | DM_OUT_BUFFER | DM_IN_PROMPT;
-  LONG result = DocumentProperties(window, printer, printer_name_ptr, out.get(),
-                                   in, flags);
+  LONG result;
+  {
+    base::ScopedBlockingCall scoped_blocking_call(
+        FROM_HERE, base::BlockingType::MAY_BLOCK);
+    result = DocumentProperties(window, printer, printer_name_ptr, out.get(),
+                                in, flags);
+  }
+
   if (canceled)
     *canceled = (result == IDCANCEL);
   if (result != IDOK)