From f068891234d8819ab930dd86df43b252b9e3e53b Mon Sep 17 00:00:00 2001
From: "piman@chromium.org"
 <piman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Fri, 7 Feb 2014 02:38:19 +0000
Subject: [PATCH] Revert 249561 "Don't run internal message loop from
 PrintJob::Co..."

> Don't run internal message loop from PrintJob::ControlledWorkerShutdown()
>
> If loops is running, and render is closed, the RenderViewHostImpl that called ControlledWorkerShutdown is going to be destroyed. So RenderViewHostImpl crashes immediately after returning from ControlledWorkerShutdown.
>
> Solution it to use delayed messages instead of nested loop.
>
> BUG=313274
> NOTRY=true
>
> Review URL: https://codereview.chromium.org/136733005

Suspect CL for http://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29/builds/26310/steps/unit_tests/logs/stdio

[ RUN      ] PrintJobTest.SimplePrint
c:\b\build\slave\cr-win-dbg\build\src\chrome\browser\printing\print_job_unittest.cc(110): error: Value of: job->is_stopping()
  Actual: false
Expected: true
[  FAILED  ] PrintJobTest.SimplePrint (218 ms)

TBR=vitalybuka@chromium.org

Review URL: https://codereview.chromium.org/150653005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@249585 0039d316-1c4b-4281-b951-d872f2087c98
---
 chrome/browser/printing/print_job.cc | 57 ++++++++++++++++++----------
 chrome/browser/printing/print_job.h  |  2 +-
 2 files changed, 39 insertions(+), 20 deletions(-)

diff --git a/chrome/browser/printing/print_job.cc b/chrome/browser/printing/print_job.cc
index 206431b84b574d..a899b1728c3529 100644
--- a/chrome/browser/printing/print_job.cc
+++ b/chrome/browser/printing/print_job.cc
@@ -163,12 +163,16 @@ void PrintJob::Stop() {
   // Be sure to live long enough.
   scoped_refptr<PrintJob> handle(this);
 
-  if (worker_->message_loop()) {
+  base::MessageLoop* worker_loop = worker_->message_loop();
+  if (worker_loop) {
     ControlledWorkerShutdown();
-  } else {
-    // Flush the cached document.
-    UpdatePrintedDocument(NULL);
+
+    is_job_pending_ = false;
+    registrar_.Remove(this, chrome::NOTIFICATION_PRINT_JOB_EVENT,
+                      content::Source<PrintJob>(this));
   }
+  // Flush the cached document.
+  UpdatePrintedDocument(NULL);
 }
 
 void PrintJob::Cancel() {
@@ -323,14 +327,32 @@ void PrintJob::ControlledWorkerShutdown() {
   // deadlock is eliminated.
   worker_->StopSoon();
 
-  // Delay shutdown until the worker terminates.  We want this code path
-  // to wait on the thread to quit before continuing.
-  if (worker_->IsRunning()) {
-    base::MessageLoop::current()->PostDelayedTask(
-        FROM_HERE,
-        base::Bind(&PrintJob::ControlledWorkerShutdown, this),
-        base::TimeDelta::FromMilliseconds(100));
-    return;
+  // Run a tight message loop until the worker terminates. It may seems like a
+  // hack but I see no other way to get it to work flawlessly. The issues here
+  // are:
+  // - We don't want to run tasks while the thread is quitting.
+  // - We want this code path to wait on the thread to quit before continuing.
+  MSG msg;
+  HANDLE thread_handle = worker_->thread_handle().platform_handle();
+  for (; thread_handle;) {
+    // Note that we don't do any kind of message prioritization since we don't
+    // execute any pending task or timer.
+    DWORD result = MsgWaitForMultipleObjects(1, &thread_handle,
+                                             FALSE, INFINITE, QS_ALLINPUT);
+    if (result == WAIT_OBJECT_0 + 1) {
+      while (PeekMessage(&msg, NULL, 0, 0, TRUE) > 0) {
+        TranslateMessage(&msg);
+        DispatchMessage(&msg);
+      }
+      // Continue looping.
+    } else if (result == WAIT_OBJECT_0) {
+      // The thread quit.
+      break;
+    } else {
+      // An error occurred. Assume the thread quit.
+      NOTREACHED();
+      break;
+    }
   }
 #endif
 
@@ -343,16 +365,13 @@ void PrintJob::ControlledWorkerShutdown() {
       FROM_HERE,
       base::Bind(&PrintJobWorker::Stop,
                  base::Unretained(worker_.get())),
-      base::Bind(&PrintJob::HoldUntilStopIsCalled, this),
+      base::Bind(&PrintJob::HoldUntilStopIsCalled,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 scoped_refptr<PrintJob>(this)),
       false);
-
-  is_job_pending_ = false;
-  registrar_.Remove(this, chrome::NOTIFICATION_PRINT_JOB_EVENT,
-                    content::Source<PrintJob>(this));
-  UpdatePrintedDocument(NULL);
 }
 
-void PrintJob::HoldUntilStopIsCalled() {
+void PrintJob::HoldUntilStopIsCalled(const scoped_refptr<PrintJob>&) {
   is_stopped_ = true;
   is_stopping_ = false;
 }
diff --git a/chrome/browser/printing/print_job.h b/chrome/browser/printing/print_job.h
index 4efc54b21f631b..e34225012315e7 100644
--- a/chrome/browser/printing/print_job.h
+++ b/chrome/browser/printing/print_job.h
@@ -118,7 +118,7 @@ class PrintJob : public PrintJobWorkerOwner,
   // Called at shutdown when running a nested message loop.
   void Quit();
 
-  void HoldUntilStopIsCalled();
+  void HoldUntilStopIsCalled(const scoped_refptr<PrintJob>& job);
 
   content::NotificationRegistrar registrar_;