Commit 2e0b5c5c authored by Kevin Babbitt's avatar Kevin Babbitt Committed by Commit Bot

Move printer handler tasks to the UI thread on Windows

We observed an issue in Microsoft Edge where a certain printer driver
was being called on two different threads simultaneously and reacted
by making an unbalanced CoUninitialize() call on the browser UI thread.
If this happens enough times, the CoInitialize() count of the UI thread
will eventually drop to zero and cause problems when, for example,
out-of-process accessibility clients attempt to access objects that are
only safe to call on the UI thread - the COM marshaler will see that
the UI thread is in the implicit MTA and assume that these objects can
be called from any MTA thread. I haven't successfully reproduced the
same issue in Chrome, but the threading pattern for printer driver
calls is the same, so the potential for it to occur is there.

Existing code comments document that some Windows printer drivers are
only safe to call from the UI thread. Accordingly, this CL addresses
the issue by moving work done by the local printer handler from a
generic task runner thread to the browser UI thread on Windows.

Bug: 1065145
Change-Id: Ic20872cb33bfb6c94381059578c2055cb1a9a1eb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2135054
Commit-Queue: Kevin Babbitt <kbabbitt@microsoft.com>
Reviewed-by: default avatarLei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756356}
parent 5f603436
...@@ -206,6 +206,7 @@ class ScopedIPCSupport; ...@@ -206,6 +206,7 @@ class ScopedIPCSupport;
} }
} }
namespace printing { namespace printing {
class LocalPrinterHandlerDefault;
class PrintJobWorker; class PrintJobWorker;
class PrinterQuery; class PrinterQuery;
} }
...@@ -372,6 +373,7 @@ class BASE_EXPORT ScopedAllowBlocking { ...@@ -372,6 +373,7 @@ class BASE_EXPORT ScopedAllowBlocking {
friend class memory_instrumentation::OSMetrics; friend class memory_instrumentation::OSMetrics;
friend class module_installer::ScopedAllowModulePakLoad; friend class module_installer::ScopedAllowModulePakLoad;
friend class mojo::CoreLibraryInitializer; friend class mojo::CoreLibraryInitializer;
friend class printing::LocalPrinterHandlerDefault;
friend class printing::PrintJobWorker; friend class printing::PrintJobWorker;
friend class resource_coordinator::TabManagerDelegate; // crbug.com/778703 friend class resource_coordinator::TabManagerDelegate; // crbug.com/778703
friend class web::WebSubThread; friend class web::WebSubThread;
......
...@@ -11,45 +11,62 @@ ...@@ -11,45 +11,62 @@
#include "base/logging.h" #include "base/logging.h"
#include "base/memory/ref_counted.h" #include "base/memory/ref_counted.h"
#include "base/memory/ref_counted_memory.h" #include "base/memory/ref_counted_memory.h"
#include "base/task/post_task.h"
#include "base/task/task_traits.h" #include "base/task/task_traits.h"
#include "base/task/thread_pool.h" #include "base/task/thread_pool.h"
#include "base/threading/scoped_blocking_call.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/browser/browser_process.h" #include "chrome/browser/browser_process.h"
#include "chrome/browser/ui/webui/print_preview/print_preview_utils.h" #include "chrome/browser/ui/webui/print_preview/print_preview_utils.h"
#include "components/printing/browser/printer_capabilities.h" #include "components/printing/browser/printer_capabilities.h"
#include "content/public/browser/browser_task_traits.h"
#include "content/public/browser/browser_thread.h" #include "content/public/browser/browser_thread.h"
#include "printing/backend/print_backend.h"
#if defined(OS_MACOSX) #if defined(OS_MACOSX)
#include "components/printing/browser/features.h" #include "components/printing/browser/features.h"
#include "components/printing/browser/printer_capabilities_mac.h" #include "components/printing/browser/printer_capabilities_mac.h"
#endif #endif
#if defined(OS_WIN)
#include "base/threading/thread_restrictions.h"
#endif
namespace printing { namespace printing {
namespace { namespace {
scoped_refptr<base::TaskRunner> CreatePrinterHandlerTaskRunner() { scoped_refptr<base::TaskRunner> CreatePrinterHandlerTaskRunner() {
// USER_VISIBLE because the result is displayed in the print preview dialog. // USER_VISIBLE because the result is displayed in the print preview dialog.
#if !defined(OS_WIN)
static constexpr base::TaskTraits kTraits = { static constexpr base::TaskTraits kTraits = {
base::MayBlock(), base::TaskPriority::USER_VISIBLE}; base::MayBlock(), base::TaskPriority::USER_VISIBLE};
#endif
#if defined(USE_CUPS) #if defined(USE_CUPS)
// CUPS is thread safe. // CUPS is thread safe.
return base::ThreadPool::CreateTaskRunner(kTraits); return base::ThreadPool::CreateTaskRunner(kTraits);
#elif defined(OS_WIN) #elif defined(OS_WIN)
// Windows drivers are likely not thread-safe. // Windows drivers are likely not thread-safe and need to be accessed on the
return base::ThreadPool::CreateSingleThreadTaskRunner(kTraits); // UI thread.
return base::CreateSingleThreadTaskRunner({content::BrowserThread::UI,
base::MayBlock(),
base::TaskPriority::USER_VISIBLE});
#else #else
// Be conservative on unsupported platforms. // Be conservative on unsupported platforms.
return base::ThreadPool::CreateSingleThreadTaskRunner(kTraits); return base::ThreadPool::CreateSingleThreadTaskRunner(kTraits);
#endif #endif
} }
PrinterList EnumeratePrintersAsync(const std::string& locale) { } // namespace
base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
base::BlockingType::MAY_BLOCK); // static
PrinterList LocalPrinterHandlerDefault::EnumeratePrintersAsync(
const std::string& locale) {
#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
scoped_refptr<PrintBackend> print_backend( scoped_refptr<PrintBackend> print_backend(
PrintBackend::CreateInstance(nullptr, locale)); PrintBackend::CreateInstance(nullptr, locale));
...@@ -58,7 +75,9 @@ PrinterList EnumeratePrintersAsync(const std::string& locale) { ...@@ -58,7 +75,9 @@ PrinterList EnumeratePrintersAsync(const std::string& locale) {
return printer_list; return printer_list;
} }
base::Value FetchCapabilitiesAsync(const std::string& device_name, // static
base::Value LocalPrinterHandlerDefault::FetchCapabilitiesAsync(
const std::string& device_name,
const std::string& locale) { const std::string& locale) {
PrinterSemanticCapsAndDefaults::Papers user_defined_papers; PrinterSemanticCapsAndDefaults::Papers user_defined_papers;
#if defined(OS_MACOSX) #if defined(OS_MACOSX)
...@@ -66,8 +85,12 @@ base::Value FetchCapabilitiesAsync(const std::string& device_name, ...@@ -66,8 +85,12 @@ base::Value FetchCapabilitiesAsync(const std::string& device_name,
user_defined_papers = GetMacCustomPaperSizes(); user_defined_papers = GetMacCustomPaperSizes();
#endif #endif
base::ScopedBlockingCall scoped_blocking_call(FROM_HERE, #if defined(OS_WIN)
base::BlockingType::MAY_BLOCK); // 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
scoped_refptr<PrintBackend> print_backend( scoped_refptr<PrintBackend> print_backend(
PrintBackend::CreateInstance(nullptr, locale)); PrintBackend::CreateInstance(nullptr, locale));
...@@ -84,9 +107,15 @@ base::Value FetchCapabilitiesAsync(const std::string& device_name, ...@@ -84,9 +107,15 @@ base::Value FetchCapabilitiesAsync(const std::string& device_name,
/*has_secure_protocol=*/false, print_backend); /*has_secure_protocol=*/false, print_backend);
} }
std::string GetDefaultPrinterAsync(const std::string& locale) { // static
base::ScopedBlockingCall scoped_blocking_call(FROM_HERE, std::string LocalPrinterHandlerDefault::GetDefaultPrinterAsync(
base::BlockingType::MAY_BLOCK); const std::string& locale) {
#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
scoped_refptr<PrintBackend> print_backend( scoped_refptr<PrintBackend> print_backend(
PrintBackend::CreateInstance(nullptr, locale)); PrintBackend::CreateInstance(nullptr, locale));
...@@ -95,8 +124,6 @@ std::string GetDefaultPrinterAsync(const std::string& locale) { ...@@ -95,8 +124,6 @@ std::string GetDefaultPrinterAsync(const std::string& locale) {
return default_printer; return default_printer;
} }
} // namespace
LocalPrinterHandlerDefault::LocalPrinterHandlerDefault( LocalPrinterHandlerDefault::LocalPrinterHandlerDefault(
content::WebContents* preview_web_contents) content::WebContents* preview_web_contents)
: preview_web_contents_(preview_web_contents), : preview_web_contents_(preview_web_contents),
......
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "base/strings/string16.h" #include "base/strings/string16.h"
#include "base/values.h" #include "base/values.h"
#include "chrome/browser/ui/webui/print_preview/printer_handler.h" #include "chrome/browser/ui/webui/print_preview/printer_handler.h"
#include "printing/backend/print_backend.h"
namespace base { namespace base {
class TaskRunner; class TaskRunner;
...@@ -44,6 +45,11 @@ class LocalPrinterHandlerDefault : public PrinterHandler { ...@@ -44,6 +45,11 @@ class LocalPrinterHandlerDefault : public PrinterHandler {
PrintCallback callback) override; PrintCallback callback) override;
private: private:
static PrinterList EnumeratePrintersAsync(const std::string& locale);
static base::Value FetchCapabilitiesAsync(const std::string& device_name,
const std::string& locale);
static std::string GetDefaultPrinterAsync(const std::string& locale);
content::WebContents* const preview_web_contents_; content::WebContents* const preview_web_contents_;
// TaskRunner for blocking tasks. Threading behavior is platform-specific. // TaskRunner for blocking tasks. Threading behavior is platform-specific.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment