diff options
author | piman@chromium.org <piman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-02-07 02:38:19 +0000 |
---|---|---|
committer | piman@chromium.org <piman@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-02-07 02:38:19 +0000 |
commit | f068891234d8819ab930dd86df43b252b9e3e53b (patch) | |
tree | 92b34eb3adccc70170efe29a7d263ab5f44037db /chrome/browser/printing/print_job.cc | |
parent | 04b5b37d788a848ac1b09e2e85637abd14086ff7 (diff) | |
download | chromium_src-f068891234d8819ab930dd86df43b252b9e3e53b.zip chromium_src-f068891234d8819ab930dd86df43b252b9e3e53b.tar.gz chromium_src-f068891234d8819ab930dd86df43b252b9e3e53b.tar.bz2 |
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
Diffstat (limited to 'chrome/browser/printing/print_job.cc')
-rw-r--r-- | chrome/browser/printing/print_job.cc | 57 |
1 files changed, 38 insertions, 19 deletions
diff --git a/chrome/browser/printing/print_job.cc b/chrome/browser/printing/print_job.cc index 206431b..a899b17 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; } |