Remove PrintJobWorkerOwner

PrintJobWorkerOwner has very little implementation that is actually
shared between PrintJob and PrinterQuery (task_runner_). Moreover,
in all cases where PrintJobWorker refers to its owner, only one of the
two subclasses is actually acceptable. Make this more clear by removing
this class.

Bug: None
Change-Id: Icaf5ff15ee5856dcade54fc4b64bb92539dc7a6f
Reviewed-on: https://chromium-review.googlesource.com/1013118Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551345}

chrome/browser/BUILD.gn

@@ -3298,8 +3298,6 @@ jumbo_split_static_library("browser") {
       "printing/print_job_manager.h",
       "printing/print_job_worker.cc",
       "printing/print_job_worker.h",
-      "printing/print_job_worker_owner.cc",
-      "printing/print_job_worker_owner.h",
       "printing/print_view_manager_base.cc",
       "printing/print_view_manager_base.h",
       "printing/print_view_manager_common.cc",

chrome/browser/printing/print_job.cc

@@ -34,14 +34,16 @@ using base::TimeDelta;
 
 namespace printing {
 
-// Helper function to ensure |owner| is valid until at least |callback| returns.
-void HoldRefCallback(scoped_refptr<PrintJobWorkerOwner> owner,
-                     base::OnceClosure callback) {
+// Helper function to ensure |job| is valid until at least |callback| returns.
+void HoldRefCallback(scoped_refptr<PrintJob> job, base::OnceClosure callback) {
   std::move(callback).Run();
 }
 
 PrintJob::PrintJob()
-    : is_job_pending_(false), is_canceling_(false), quit_factory_(this) {
+    : is_job_pending_(false),
+      is_canceling_(false),
+      task_runner_(base::ThreadTaskRunnerHandle::Get()),
+      quit_factory_(this) {
   DCHECK(base::MessageLoopForUI::IsCurrent());
 }
 
@@ -60,7 +62,8 @@ void PrintJob::Initialize(PrinterQuery* query,
   DCHECK(!is_job_pending_);
   DCHECK(!is_canceling_);
   DCHECK(!document_);
-  worker_ = query->DetachWorker(this);
+  worker_ = query->DetachWorker();
+  worker_->SetPrintJob(this);
   settings_ = query->settings();
 
   auto new_doc =
@@ -135,21 +138,6 @@ void PrintJob::Observe(int type,
   OnNotifyPrintJobEvent(*content::Details<JobEventDetails>(details).ptr());
 }
 
-void PrintJob::GetSettingsDone(const PrintSettings& new_settings,
-                               PrintingContext::Result result) {
-  NOTREACHED();
-}
-
-std::unique_ptr<PrintJobWorker> PrintJob::DetachWorker(
-    PrintJobWorkerOwner* new_owner) {
-  NOTREACHED();
-  return nullptr;
-}
-
-const PrintSettings& PrintJob::settings() const {
-  return settings_;
-}
-
 void PrintJob::StartPrinting() {
   DCHECK(RunsTasksInCurrentSequence());
   if (!worker_->IsRunning() || is_job_pending_) {
@@ -305,7 +293,7 @@ void PrintJob::StartPdfToEmfConversion(
       settings_.print_text_with_gdi() && !settings_.printer_is_xps() &&
       base::FeatureList::IsEnabled(features::kGdiTextPrinting);
   PdfRenderSettings render_settings(
-      content_area, gfx::Point(0, 0), settings().dpi_size(),
+      content_area, gfx::Point(0, 0), settings_.dpi_size(),
       /*autorotate=*/true, settings_.color() == COLOR,
       print_text_with_gdi ? PdfRenderSettings::Mode::GDI_TEXT
                           : PdfRenderSettings::Mode::NORMAL);
@@ -361,7 +349,7 @@ void PrintJob::StartPdfToTextConversion(
       std::make_unique<PdfConversionState>(gfx::Size(), gfx::Rect());
   gfx::Rect page_area = gfx::Rect(0, 0, page_size.width(), page_size.height());
   PdfRenderSettings render_settings(
-      page_area, gfx::Point(0, 0), settings().dpi_size(),
+      page_area, gfx::Point(0, 0), settings_.dpi_size(),
       /*autorotate=*/true,
       /*use_color=*/true, PdfRenderSettings::Mode::TEXTONLY);
   pdf_conversion_state_->Start(
@@ -378,7 +366,7 @@ void PrintJob::StartPdfToPostScriptConversion(
   pdf_conversion_state_ = std::make_unique<PdfConversionState>(
       gfx::Size(), gfx::Rect());
   PdfRenderSettings render_settings(
-      content_area, physical_offsets, settings().dpi_size(),
+      content_area, physical_offsets, settings_.dpi_size(),
       /*autorotate=*/true, settings_.color() == COLOR,
       ps_level2 ? PdfRenderSettings::Mode::POSTSCRIPT_LEVEL2
                 : PdfRenderSettings::Mode::POSTSCRIPT_LEVEL3);
@@ -516,6 +504,15 @@ void PrintJob::ControlledWorkerShutdown() {
   ClearPrintedDocument();
 }
 
+bool PrintJob::RunsTasksInCurrentSequence() const {
+  return task_runner_->RunsTasksInCurrentSequence();
+}
+
+bool PrintJob::PostTask(const base::Location& from_here,
+                        base::OnceClosure task) {
+  return task_runner_->PostTask(from_here, std::move(task));
+}
+
 void PrintJob::HoldUntilStopIsCalled() {
 }
 

chrome/browser/printing/print_job.h

@@ -10,14 +10,17 @@
 
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
+#include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "build/build_config.h"
-#include "chrome/browser/printing/print_job_worker_owner.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
+#include "printing/print_settings.h"
 
 namespace base {
+class Location;
 class RefCountedMemory;
+class SequencedTaskRunner;
 }
 
 namespace printing {
@@ -25,15 +28,12 @@ namespace printing {
 class JobEventDetails;
 class MetafilePlayer;
 class PrintJobWorker;
-class PrintJobWorkerOwner;
 class PrintedDocument;
 #if defined(OS_WIN)
 class PrintedPage;
 #endif
 class PrinterQuery;
-
-void HoldRefCallback(scoped_refptr<PrintJobWorkerOwner> owner,
-                     base::OnceClosure callback);
+class PrintSettings;
 
 // Manages the print work for a specific document. Talks to the printer through
 // PrintingContext through PrintJobWorker. Hides access to PrintingContext in a
@@ -42,7 +42,7 @@ void HoldRefCallback(scoped_refptr<PrintJobWorkerOwner> owner,
 // reference to the job to be sure it is kept alive. All the code in this class
 // runs in the UI thread. All virtual functions are virtual only so that
 // TestPrintJob can override them in tests.
-class PrintJob : public PrintJobWorkerOwner,
+class PrintJob : public base::RefCountedThreadSafe<PrintJob>,
                  public content::NotificationObserver {
  public:
   // Create a empty PrintJob. When initializing with this constructor,
@@ -76,13 +76,6 @@ class PrintJob : public PrintJobWorkerOwner,
                const content::NotificationSource& source,
                const content::NotificationDetails& details) override;
 
-  // PrintJobWorkerOwner implementation.
-  void GetSettingsDone(const PrintSettings& new_settings,
-                       PrintingContext::Result result) override;
-  std::unique_ptr<PrintJobWorker> DetachWorker(
-      PrintJobWorkerOwner* new_owner) override;
-  const PrintSettings& settings() const override;
-
   // Starts the actual printing. Signals the worker that it should begin to
   // spool as soon as data is available.
   virtual void StartPrinting();
@@ -111,7 +104,17 @@ class PrintJob : public PrintJobWorkerOwner,
   // Access the current printed document. Warning: may be NULL.
   PrintedDocument* document() const;
 
+  // Returns true if tasks posted to this TaskRunner are sequenced
+  // with this call.
+  bool RunsTasksInCurrentSequence() const;
+
+  // Posts the given task to be run.
+  bool PostTask(const base::Location& from_here, base::OnceClosure task);
+
  protected:
+  // Refcounted class.
+  friend class base::RefCountedThreadSafe<PrintJob>;
+
   ~PrintJob() override;
 
   // The functions below are used for tests only.
@@ -202,6 +205,10 @@ class PrintJob : public PrintJobWorkerOwner,
   std::vector<int> pdf_page_mapping_;
 #endif  // defined(OS_WIN)
 
+  // Task runner reference. Used to send notifications in the right
+  // thread.
+  scoped_refptr<base::SequencedTaskRunner> task_runner_;
+
   // Used at shutdown so that we can quit a nested run loop.
   base::WeakPtrFactory<PrintJob> quit_factory_;
 

chrome/browser/printing/print_job_manager.h

@@ -31,8 +31,8 @@ class PrintQueriesQueue : public base::RefCountedThreadSafe<PrintQueriesQueue> {
   // TODO(maruel):  Have them vanish after a timeout (~5 minutes?)
   void QueuePrinterQuery(PrinterQuery* query);
 
-  // Pops a queued PrintJobWorkerOwner object that was previously queued or
-  // create new one. Can be called from any thread.
+  // Pops a queued PrinterQuery object that was previously queued or creates
+  // a new one. Can be called from any thread.
   scoped_refptr<PrinterQuery> PopPrinterQuery(int document_cookie);
 
   // Creates new query. Virtual so that tests can override it.

chrome/browser/printing/print_job_unittest.cc

@@ -26,16 +26,16 @@ namespace {
 
 class TestPrintJobWorker : public PrintJobWorker {
  public:
-  explicit TestPrintJobWorker(PrintJobWorkerOwner* owner)
+  explicit TestPrintJobWorker(PrinterQuery* query)
       : PrintJobWorker(content::ChildProcessHost::kInvalidUniqueID,
                        content::ChildProcessHost::kInvalidUniqueID,
-                       owner) {}
-  friend class TestOwner;
+                       query) {}
+  friend class TestQuery;
 };
 
-class TestOwner : public PrinterQuery {
+class TestQuery : public PrinterQuery {
  public:
-  TestOwner()
+  TestQuery()
       : PrinterQuery(content::ChildProcessHost::kInvalidUniqueID,
                      content::ChildProcessHost::kInvalidUniqueID) {}
 
@@ -44,30 +44,30 @@ class TestOwner : public PrinterQuery {
     FAIL();
   }
 
-  std::unique_ptr<PrintJobWorker> DetachWorker(
-      PrintJobWorkerOwner* new_owner) override {
+  std::unique_ptr<PrintJobWorker> DetachWorker() override {
     {
       // Do an actual detach to keep the parent class happy.
-      auto real_worker = PrinterQuery::DetachWorker(new_owner);
+      auto real_worker = PrinterQuery::DetachWorker();
     }
 
     // We're screwing up here since we're calling worker from the main thread.
     // That's fine for testing. It is actually simulating PrinterQuery behavior.
-    auto worker = std::make_unique<TestPrintJobWorker>(new_owner);
+    auto worker = std::make_unique<TestPrintJobWorker>(this);
     EXPECT_TRUE(worker->Start());
     worker->printing_context()->UseDefaultSettings();
     settings_ = worker->printing_context()->settings();
+
     return std::move(worker);
   }
 
   const PrintSettings& settings() const override { return settings_; }
 
  private:
-  ~TestOwner() override {}
+  ~TestQuery() override {}
 
   PrintSettings settings_;
 
-  DISALLOW_COPY_AND_ASSIGN(TestOwner);
+  DISALLOW_COPY_AND_ASSIGN(TestQuery);
 };
 
 class TestPrintJob : public PrintJob {
@@ -103,8 +103,8 @@ TEST(PrintJobTest, SimplePrint) {
   volatile bool check = false;
   scoped_refptr<PrintJob> job(new TestPrintJob(&check));
   EXPECT_TRUE(job->RunsTasksInCurrentSequence());
-  scoped_refptr<TestOwner> owner(new TestOwner);
-  job->Initialize(owner.get(), base::string16(), 1);
+  scoped_refptr<TestQuery> query = base::MakeRefCounted<TestQuery>();
+  job->Initialize(query.get(), base::string16(), 1);
   job->Stop();
   while (job->document()) {
     base::RunLoop().RunUntilIdle();

chrome/browser/printing/print_job_worker.cc

@@ -90,7 +90,7 @@ std::string PrintingContextDelegate::GetAppLocale() {
   return g_browser_process->GetApplicationLocale();
 }
 
-void NotificationCallback(PrintJobWorkerOwner* print_job,
+void NotificationCallback(PrintJob* print_job,
                           JobEventDetails::Type detail_type,
                           int job_id,
                           PrintedDocument* document) {
@@ -98,21 +98,26 @@ void NotificationCallback(PrintJobWorkerOwner* print_job,
       base::MakeRefCounted<JobEventDetails>(detail_type, job_id, document);
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_PRINT_JOB_EVENT,
-      // We know that is is a PrintJob object in this circumstance.
-      content::Source<PrintJob>(static_cast<PrintJob*>(print_job)),
+      content::Source<PrintJob>(print_job),
       content::Details<JobEventDetails>(details.get()));
 }
 
-void PostOnOwnerThread(scoped_refptr<PrintJobWorkerOwner> owner,
+// Helper function to ensure |query| is valid until at least |callback| returns.
+void HoldRefCallback(scoped_refptr<PrinterQuery> query,
+                     base::OnceClosure callback) {
+  std::move(callback).Run();
+}
+
+void PostOnQueryThread(scoped_refptr<PrinterQuery> query,
                        PrintingContext::PrintSettingsCallback callback,
                        PrintingContext::Result result) {
-  owner->PostTask(FROM_HERE,
-                  base::BindOnce(&HoldRefCallback, owner,
+  query->PostTask(FROM_HERE,
+                  base::BindOnce(&HoldRefCallback, query,
                                  base::BindOnce(std::move(callback), result)));
 }
 
 #if defined(OS_WIN)
-void PageNotificationCallback(PrintJobWorkerOwner* print_job,
+void PageNotificationCallback(PrintJob* print_job,
                               JobEventDetails::Type detail_type,
                               int job_id,
                               PrintedDocument* document,
@@ -121,8 +126,7 @@ void PageNotificationCallback(PrintJobWorkerOwner* print_job,
                                                        document, page);
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_PRINT_JOB_EVENT,
-      // We know that is is a PrintJob object in this circumstance.
-      content::Source<PrintJob>(static_cast<PrintJob*>(print_job)),
+      content::Source<PrintJob>(print_job),
       content::Details<JobEventDetails>(details.get()));
 }
 #endif
@@ -131,30 +135,39 @@ void PageNotificationCallback(PrintJobWorkerOwner* print_job,
 
 PrintJobWorker::PrintJobWorker(int render_process_id,
                                int render_frame_id,
-                               PrintJobWorkerOwner* owner)
+                               PrinterQuery* query)
     : printing_context_delegate_(
           std::make_unique<PrintingContextDelegate>(render_process_id,
                                                     render_frame_id)),
       printing_context_(
           PrintingContext::Create(printing_context_delegate_.get())),
-      owner_(owner),
+      query_(query),
       thread_("Printing_Worker"),
       weak_factory_(this) {
   // The object is created in the IO thread.
-  DCHECK(owner_->RunsTasksInCurrentSequence());
+  DCHECK(query_->RunsTasksInCurrentSequence());
 }
 
 PrintJobWorker::~PrintJobWorker() {
-  // The object is normally deleted in the UI thread, but when the user
-  // cancels printing or in the case of print preview, the worker is destroyed
-  // on the I/O thread.
-  DCHECK(owner_->RunsTasksInCurrentSequence());
+  // The object is normally deleted by PrintJob in the UI thread, but when the
+  // user cancels printing or in the case of print preview, the worker is
+  // destroyed with the PrinterQuery, which is on the I/O thread.
+  if (query_) {
+    DCHECK(!print_job_);
+    DCHECK(query_->RunsTasksInCurrentSequence());
+  } else {
+    DCHECK(print_job_);
+    DCHECK(print_job_->RunsTasksInCurrentSequence());
+  }
   Stop();
 }
 
-void PrintJobWorker::SetNewOwner(PrintJobWorkerOwner* new_owner) {
+void PrintJobWorker::SetPrintJob(PrintJob* print_job) {
   DCHECK(page_number_ == PageNumber::npos());
-  owner_ = new_owner;
+  print_job_ = print_job;
+
+  // Release the Printer Query reference. It is no longer needed.
+  query_ = nullptr;
 }
 
 void PrintJobWorker::GetSettings(bool ask_user_for_settings,
@@ -166,6 +179,9 @@ void PrintJobWorker::GetSettings(bool ask_user_for_settings,
   DCHECK(task_runner_->RunsTasksInCurrentSequence());
   DCHECK_EQ(page_number_, PageNumber::npos());
 
+  // This function is only called by the PrinterQuery.
+  DCHECK(query_);
+
   // Recursive task processing is needed for the dialog in case it needs to be
   // destroyed by a task.
   // TODO(thestig): This code is wrong. SetNestableTasksAllowed(true) is needed
@@ -181,14 +197,14 @@ void PrintJobWorker::GetSettings(bool ask_user_for_settings,
     BrowserThread::PostTask(
         BrowserThread::UI, FROM_HERE,
         base::BindOnce(
-            &HoldRefCallback, base::WrapRefCounted(owner_),
+            &HoldRefCallback, base::WrapRefCounted(query_),
             base::BindOnce(&PrintJobWorker::GetSettingsWithUI,
                            base::Unretained(this), document_page_count,
                            has_selection, is_scripted)));
   } else {
     BrowserThread::PostTask(
         BrowserThread::UI, FROM_HERE,
-        base::BindOnce(&HoldRefCallback, base::WrapRefCounted(owner_),
+        base::BindOnce(&HoldRefCallback, base::WrapRefCounted(query_),
                        base::BindOnce(&PrintJobWorker::UseDefaultSettings,
                                       base::Unretained(this))));
   }
@@ -197,11 +213,12 @@ void PrintJobWorker::GetSettings(bool ask_user_for_settings,
 void PrintJobWorker::SetSettings(
     std::unique_ptr<base::DictionaryValue> new_settings) {
   DCHECK(task_runner_->RunsTasksInCurrentSequence());
+  DCHECK(query_);
 
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       base::BindOnce(
-          &HoldRefCallback, base::WrapRefCounted(owner_),
+          &HoldRefCallback, base::WrapRefCounted(query_),
           base::BindOnce(&PrintJobWorker::UpdatePrintSettings,
                          base::Unretained(this), std::move(new_settings))));
 }
@@ -210,11 +227,12 @@ void PrintJobWorker::SetSettings(
 void PrintJobWorker::SetSettingsFromPOD(
     std::unique_ptr<printing::PrintSettings> new_settings) {
   DCHECK(task_runner_->RunsTasksInCurrentSequence());
+  DCHECK(query_);
 
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       base::BindOnce(
-          &HoldRefCallback, base::WrapRefCounted(owner_),
+          &HoldRefCallback, base::WrapRefCounted(query_),
           base::BindOnce(&PrintJobWorker::UpdatePrintSettingsFromPOD,
                          base::Unretained(this), std::move(new_settings))));
 }
@@ -246,12 +264,12 @@ void PrintJobWorker::GetSettingsDone(PrintingContext::Result result) {
   // http://crbug.com/73466
   // MessageLoop::current()->SetNestableTasksAllowed(false);
 
-  // We can't use OnFailure() here since owner_ may not support notifications.
+  // We can't use OnFailure() here since query_ does not support notifications.
 
-  // PrintJob will create the new PrintedDocument.
-  owner_->PostTask(FROM_HERE,
-                   base::BindOnce(&PrintJobWorkerOwner::GetSettingsDone,
-                                  base::WrapRefCounted(owner_),
+  DCHECK(query_);
+  query_->PostTask(FROM_HERE,
+                   base::BindOnce(&PrinterQuery::GetSettingsDone,
+                                  base::WrapRefCounted(query_),
                                   printing_context_->settings(), result));
 }
 
@@ -286,10 +304,10 @@ void PrintJobWorker::GetSettingsWithUI(
   if (web_contents && web_contents->IsFullscreenForCurrentTab())
     web_contents->ExitFullscreen(true);
 
-  // weak_factory_ creates pointers valid only on owner_ thread.
+  // weak_factory_ creates pointers valid only on query_ thread.
   printing_context_->AskUserForSettings(
       document_page_count, has_selection, is_scripted,
-      base::BindOnce(&PostOnOwnerThread, base::WrapRefCounted(owner_),
+      base::BindOnce(&PostOnQueryThread, base::WrapRefCounted(query_),
                      base::BindOnce(&PrintJobWorker::GetSettingsDone,
                                     weak_factory_.GetWeakPtr())));
 }
@@ -441,6 +459,8 @@ void PrintJobWorker::OnDocumentDone() {
   DCHECK(task_runner_->RunsTasksInCurrentSequence());
   DCHECK_EQ(page_number_, PageNumber::npos());
   DCHECK(document_.get());
+  // PrintJob must own this, because only PrintJob can send notifications.
+  DCHECK(print_job_);
 
   int job_id = printing_context_->job_id();
   if (printing_context_->DocumentDone() != PrintingContext::OK) {
@@ -448,10 +468,11 @@ void PrintJobWorker::OnDocumentDone() {
     return;
   }
 
-  owner_->PostTask(FROM_HERE, base::BindOnce(&NotificationCallback,
-                                             base::RetainedRef(owner_),
-                                             JobEventDetails::DOC_DONE, job_id,
-                                             base::RetainedRef(document_)));
+  print_job_->PostTask(
+      FROM_HERE,
+      base::BindOnce(&NotificationCallback, base::RetainedRef(print_job_),
+                     JobEventDetails::DOC_DONE, job_id,
+                     base::RetainedRef(document_)));
 
   // Makes sure the variables are reinitialized.
   document_ = NULL;
@@ -478,11 +499,12 @@ void PrintJobWorker::SpoolPage(PrintedPage* page) {
   }
 
   // Signal everyone that the page is printed.
-  owner_->PostTask(FROM_HERE,
-                   base::BindRepeating(
-                       &PageNotificationCallback, base::RetainedRef(owner_),
-                       JobEventDetails::PAGE_DONE, printing_context_->job_id(),
-                       base::RetainedRef(document_), base::RetainedRef(page)));
+  DCHECK(print_job_);
+  print_job_->PostTask(
+      FROM_HERE, base::BindRepeating(
+                     &PageNotificationCallback, base::RetainedRef(print_job_),
+                     JobEventDetails::PAGE_DONE, printing_context_->job_id(),
+                     base::RetainedRef(document_), base::RetainedRef(page)));
 }
 #else
 void PrintJobWorker::SpoolJob() {
@@ -494,13 +516,14 @@ void PrintJobWorker::SpoolJob() {
 
 void PrintJobWorker::OnFailure() {
   DCHECK(task_runner_->RunsTasksInCurrentSequence());
+  DCHECK(print_job_);
 
   // We may loose our last reference by broadcasting the FAILED event.
-  scoped_refptr<PrintJobWorkerOwner> handle(owner_);
+  scoped_refptr<PrintJob> handle(print_job_);
 
-  owner_->PostTask(
+  print_job_->PostTask(
       FROM_HERE,
-      base::BindOnce(&NotificationCallback, base::RetainedRef(owner_),
+      base::BindOnce(&NotificationCallback, base::RetainedRef(print_job_),
                      JobEventDetails::FAILED, 0, base::RetainedRef(document_)));
   Cancel();
 

chrome/browser/printing/print_job_worker.h

@@ -25,9 +25,9 @@ class DictionaryValue;
 namespace printing {
 
 class PrintJob;
-class PrintJobWorkerOwner;
 class PrintedDocument;
 class PrintedPage;
+class PrinterQuery;
 
 // Worker thread code. It manages the PrintingContext, which can be blocking
 // and/or run a message loop. This is the object that generates most
@@ -38,10 +38,12 @@ class PrintJobWorker {
  public:
   PrintJobWorker(int render_process_id,
                  int render_frame_id,
-                 PrintJobWorkerOwner* owner);
+                 PrinterQuery* query);
   virtual ~PrintJobWorker();
 
-  void SetNewOwner(PrintJobWorkerOwner* new_owner);
+  void SetPrintJob(PrintJob* print_job);
+
+  /* The following functions may only be called before calling SetPrintJob(). */
 
   // Initializes the print settings. If |ask_user_for_settings| is true, a
   // Print... dialog box will be shown to ask the user their preference.
@@ -63,6 +65,8 @@ class PrintJobWorker {
       std::unique_ptr<printing::PrintSettings> new_settings);
 #endif
 
+  /* The following functions may only be called after calling SetPrintJob(). */
+
   // Starts the printing loop. Every pages are printed as soon as the data is
   // available. Makes sure the new_document is the right one.
   void StartPrinting(PrintedDocument* new_document);
@@ -75,7 +79,9 @@ class PrintJobWorker {
   // the next page can be printed.
   void OnNewPage();
 
-  // This is the only function that can be called in a thread.
+  /* The following functions may be called before or after SetPrintJob(). */
+
+  // Cancels the job.
   void Cancel();
 
   // Returns true if the thread has been started, and not yet stopped.
@@ -141,7 +147,7 @@ class PrintJobWorker {
       std::unique_ptr<printing::PrintSettings> new_settings);
 #endif
 
-  // Reports settings back to owner_.
+  // Reports settings back to |query_|.
   void GetSettingsDone(PrintingContext::Result result);
 
   // Use the default settings. When using GTK+ or Mac, this can still end up
@@ -158,9 +164,13 @@ class PrintJobWorker {
   // The printed document. Only has read-only access.
   scoped_refptr<PrintedDocument> document_;
 
+  // The printer query that owns this worker thread at creation. It will own
+  // the object until DetachWorker() is called.
+  PrinterQuery* query_ = nullptr;
+
   // The print job owning this worker thread. It is guaranteed to outlive this
-  // object.
-  PrintJobWorkerOwner* owner_;
+  // object and should be set with SetPrintJob().
+  PrintJob* print_job_ = nullptr;
 
   // Current page number to print.
   PageNumber page_number_;

chrome/browser/printing/print_job_worker_owner.cc

-// Copyright 2014 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.
-
-#include "chrome/browser/printing/print_job_worker_owner.h"
-
-#include "base/location.h"
-#include "base/single_thread_task_runner.h"
-#include "base/threading/thread_task_runner_handle.h"
-
-namespace printing {
-
-PrintJobWorkerOwner::PrintJobWorkerOwner()
-    : task_runner_(base::ThreadTaskRunnerHandle::Get()) {}
-
-PrintJobWorkerOwner::~PrintJobWorkerOwner() {
-}
-
-bool PrintJobWorkerOwner::RunsTasksInCurrentSequence() const {
-  return task_runner_->RunsTasksInCurrentSequence();
-}
-
-bool PrintJobWorkerOwner::PostTask(const base::Location& from_here,
-                                   base::OnceClosure task) {
-  return task_runner_->PostTask(from_here, std::move(task));
-}
-
-}  // namespace printing

chrome/browser/printing/print_job_worker_owner.h

-// Copyright (c) 2011 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.
-
-#ifndef CHROME_BROWSER_PRINTING_PRINT_JOB_WORKER_OWNER_H__
-#define CHROME_BROWSER_PRINTING_PRINT_JOB_WORKER_OWNER_H__
-
-#include <memory>
-
-#include "base/memory/ref_counted.h"
-#include "printing/printing_context.h"
-
-namespace base {
-class Location;
-class SequencedTaskRunner;
-}
-
-namespace printing {
-
-class PrintJobWorker;
-class PrintSettings;
-
-class PrintJobWorkerOwner
-    : public base::RefCountedThreadSafe<PrintJobWorkerOwner> {
- public:
-  // Can only be called in single-threaded context.
-  PrintJobWorkerOwner();
-
-  // Finishes the initialization began by PrintJobWorker::GetSettings().
-  // Creates a new PrintedDocument if necessary. Solely meant to be called by
-  // PrintJobWorker.
-  virtual void GetSettingsDone(const PrintSettings& new_settings,
-                               PrintingContext::Result result) = 0;
-
-  // Detach the PrintJobWorker associated to this object.
-  virtual std::unique_ptr<PrintJobWorker> DetachWorker(
-      PrintJobWorkerOwner* new_owner) = 0;
-
-  // Access the current settings.
-  virtual const PrintSettings& settings() const = 0;
-
-  // Returns true if tasks posted to this TaskRunner are sequenced
-  // with this call.
-  bool RunsTasksInCurrentSequence() const;
-
-  // Posts the given task to be run.
-  bool PostTask(const base::Location& from_here, base::OnceClosure task);
-
- protected:
-  friend class base::RefCountedThreadSafe<PrintJobWorkerOwner>;
-
-  virtual ~PrintJobWorkerOwner();
-
-  // Task runner reference. Used to send notifications in the right
-  // thread.
-  scoped_refptr<base::SequencedTaskRunner> task_runner_;
-};
-
-}  // namespace printing
-
-#endif  // CHROME_BROWSER_PRINTING_PRINT_JOB_WORKER_OWNER_H__

chrome/browser/printing/printer_query.cc

@@ -9,18 +9,24 @@
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
+#include "base/location.h"
+#include "base/single_thread_task_runner.h"
 #include "base/threading/thread_restrictions.h"
+#include "base/threading/thread_task_runner_handle.h"
 #include "base/values.h"
 #include "chrome/browser/printing/print_job_worker.h"
 #include "content/public/browser/browser_thread.h"
+#include "printing/print_job_constants.h"
+#include "printing/print_settings.h"
 
 namespace printing {
 
 PrinterQuery::PrinterQuery(int render_process_id, int render_frame_id)
-    : worker_(std::make_unique<PrintJobWorker>(render_process_id,
+    : cookie_(PrintSettings::NewCookie()),
+      task_runner_(base::ThreadTaskRunnerHandle::Get()),
+      worker_(std::make_unique<PrintJobWorker>(render_process_id,
                                                render_frame_id,
-                                               this)),
-      cookie_(PrintSettings::NewCookie()) {
+                                               this)) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
 }
 
@@ -49,12 +55,10 @@ void PrinterQuery::GetSettingsDone(const PrintSettings& new_settings,
   }
 }
 
-std::unique_ptr<PrintJobWorker> PrinterQuery::DetachWorker(
-    PrintJobWorkerOwner* new_owner) {
+std::unique_ptr<PrintJobWorker> PrinterQuery::DetachWorker() {
   DCHECK(!callback_);
   DCHECK(worker_);
 
-  worker_->SetNewOwner(new_owner);
   return std::move(worker_);
 }
 
@@ -138,6 +142,15 @@ void PrinterQuery::StopWorker() {
   }
 }
 
+bool PrinterQuery::RunsTasksInCurrentSequence() const {
+  return task_runner_->RunsTasksInCurrentSequence();
+}
+
+bool PrinterQuery::PostTask(const base::Location& from_here,
+                            base::OnceClosure task) {
+  return task_runner_->PostTask(from_here, std::move(task));
+}
+
 bool PrinterQuery::is_callback_pending() const {
   return !callback_.is_null();
 }

chrome/browser/printing/printer_query.h

@@ -9,10 +9,15 @@
 
 #include "base/callback.h"
 #include "base/macros.h"
-#include "chrome/browser/printing/print_job_worker_owner.h"
+#include "base/memory/ref_counted.h"
+#include "printing/print_job_constants.h"
+#include "printing/print_settings.h"
+#include "printing/printing_context.h"
 
 namespace base {
 class DictionaryValue;
+class Location;
+class SequencedTaskRunner;
 }
 
 namespace printing {
@@ -20,7 +25,7 @@ namespace printing {
 class PrintJobWorker;
 
 // Query the printer for settings.
-class PrinterQuery : public PrintJobWorkerOwner {
+class PrinterQuery : public base::RefCountedThreadSafe<PrinterQuery> {
  public:
   // GetSettings() UI parameter.
   enum class GetSettingsAskParam {
@@ -31,12 +36,16 @@ class PrinterQuery : public PrintJobWorkerOwner {
   // Can only be called on the IO thread.
   PrinterQuery(int render_process_id, int render_frame_id);
 
-  // PrintJobWorkerOwner implementation.
-  void GetSettingsDone(const PrintSettings& new_settings,
-                       PrintingContext::Result result) override;
-  std::unique_ptr<PrintJobWorker> DetachWorker(
-      PrintJobWorkerOwner* new_owner) override;
-  const PrintSettings& settings() const override;
+  // Virtual so that tests can override.
+  virtual void GetSettingsDone(const PrintSettings& new_settings,
+                               PrintingContext::Result result);
+
+  // Detach the PrintJobWorker associated to this object. Virtual so that tests
+  // can override.
+  virtual std::unique_ptr<PrintJobWorker> DetachWorker();
+
+  // Virtual so that tests can override.
+  virtual const PrintSettings& settings() const;
 
   // Initializes the printing context. It is fine to call this function multiple
   // times to reinitialize the settings. |web_contents_observer| can be queried
@@ -72,18 +81,22 @@ class PrinterQuery : public PrintJobWorkerOwner {
   // Returns if a worker thread is still associated to this instance.
   bool is_valid() const;
 
+  // Returns true if tasks posted to this TaskRunner are sequenced
+  // with this call.
+  bool RunsTasksInCurrentSequence() const;
+
+  // Posts the given task to be run.
+  bool PostTask(const base::Location& from_here, base::OnceClosure task);
+
  protected:
   // Refcounted class.
-  ~PrinterQuery() override;
+  friend class base::RefCountedThreadSafe<PrinterQuery>;
+
+  virtual ~PrinterQuery();
 
   // For unit tests to manually set the print callback.
   void set_callback(base::OnceClosure callback);
 
-  // All the UI is done in a worker thread because many Win32 print functions
-  // are blocking and enters a message loop without your consent. There is one
-  // worker thread per print job.
-  std::unique_ptr<PrintJobWorker> worker_;
-
  private:
   // Lazy create the worker thread. There is one worker thread per print job.
   void StartWorker(base::OnceClosure callback);
@@ -103,6 +116,15 @@ class PrinterQuery : public PrintJobWorkerOwner {
   // Callback waiting to be run.
   base::OnceClosure callback_;
 
+  // Task runner reference. Used to send notifications in the right
+  // thread.
+  scoped_refptr<base::SequencedTaskRunner> task_runner_;
+
+  // All the UI is done in a worker thread because many Win32 print functions
+  // are blocking and enters a message loop without your consent. There is one
+  // worker thread per print job.
+  std::unique_ptr<PrintJobWorker> worker_;
+
   DISALLOW_COPY_AND_ASSIGN(PrinterQuery);
 };
 

chrome/browser/printing/test_print_job.cc

@@ -21,11 +21,13 @@ void TestPrintJob::Initialize(PrinterQuery* query,
                               int page_count) {
   // Since we do not actually print in these tests, just let this get destroyed
   // when this function exits.
-  std::unique_ptr<PrintJobWorker> worker = query->DetachWorker(this);
+  std::unique_ptr<PrintJobWorker> worker = query->DetachWorker();
+
   set_settings(query->settings());
 
   scoped_refptr<PrintedDocument> new_doc =
-      base::MakeRefCounted<PrintedDocument>(settings(), name, query->cookie());
+      base::MakeRefCounted<PrintedDocument>(query->settings(), name,
+                                            query->cookie());
 
   new_doc->set_page_count(page_count);
   UpdatePrintedDocument(new_doc.get());

chrome/browser/printing/test_printer_query.h

@@ -11,7 +11,6 @@
 #include "base/macros.h"
 #include "build/build_config.h"
 #include "chrome/browser/printing/print_job_manager.h"
-#include "chrome/browser/printing/print_job_worker_owner.h"
 #include "chrome/browser/printing/printer_query.h"
 
 namespace base {