summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--chrome/browser/printing/print_job.cc74
-rw-r--r--chrome/browser/printing/print_job.h17
-rw-r--r--chrome/browser/printing/print_job_worker.cc3
-rw-r--r--chrome/browser/printing/printing_layout_uitest.cc4
4 files changed, 66 insertions, 32 deletions
diff --git a/chrome/browser/printing/print_job.cc b/chrome/browser/printing/print_job.cc
index 54713cf..0b6ec2f 100644
--- a/chrome/browser/printing/print_job.cc
+++ b/chrome/browser/printing/print_job.cc
@@ -46,7 +46,6 @@ PrintJob::PrintJob(PrintedPagesSource* source)
source_(source),
is_job_pending_(false),
is_print_dialog_box_shown_(false),
- is_blocking_(false),
is_canceling_(false) {
}
@@ -57,7 +56,6 @@ PrintJob::PrintJob()
settings_(),
is_job_pending_(false),
is_print_dialog_box_shown_(false),
- is_blocking_(false),
is_canceling_(false) {
}
@@ -65,7 +63,6 @@ PrintJob::~PrintJob() {
// The job should be finished (or at least canceled) when it is destroyed.
DCHECK(!is_job_pending_);
DCHECK(!is_print_dialog_box_shown_);
- DCHECK(!is_blocking_);
DCHECK(!is_canceling_);
DCHECK(worker_->message_loop() == NULL);
DCHECK_EQ(ui_message_loop_, MessageLoop::current());
@@ -77,7 +74,6 @@ void PrintJob::Initialize(PrintJobWorkerOwner* job,
DCHECK(!worker_.get());
DCHECK(!is_job_pending_);
DCHECK(!is_print_dialog_box_shown_);
- DCHECK(!is_blocking_);
DCHECK(!is_canceling_);
DCHECK(!document_.get());
source_ = source;
@@ -124,7 +120,6 @@ void PrintJob::Observe(NotificationType type,
void PrintJob::GetSettingsDone(const PrintSettings& new_settings,
PrintingContext::Result result) {
DCHECK(!is_job_pending_);
- DCHECK(!is_blocking_);
if (!source_ || result == PrintingContext::FAILED) {
// The source is gone, there's nothing to do.
@@ -175,9 +170,8 @@ void PrintJob::GetSettings(GetSettingsAskParam ask_user_for_settings,
DCHECK_EQ(ui_message_loop_, MessageLoop::current());
DCHECK(!is_job_pending_);
DCHECK(!is_print_dialog_box_shown_);
- DCHECK(!is_blocking_);
// Is not reentrant.
- if (is_job_pending_ || is_blocking_)
+ if (is_job_pending_)
return;
// Lazy create the worker thread. There is one worker thread per print job.
@@ -207,8 +201,7 @@ void PrintJob::StartPrinting() {
DCHECK(worker_->message_loop());
DCHECK(!is_job_pending_);
DCHECK(!is_print_dialog_box_shown_);
- DCHECK(!is_blocking_);
- if (!worker_->message_loop() || is_job_pending_ || is_blocking_)
+ if (!worker_->message_loop() || is_job_pending_)
return;
// Real work is done in PrintJobWorker::StartPrinting().
@@ -235,25 +228,20 @@ void PrintJob::Stop() {
MessageLoop* worker_loop = worker_->message_loop();
if (worker_loop) {
if (is_print_dialog_box_shown_) {
- // Make sure there is no dialog box.
+ // Make sure there is no Print... dialog box.
worker_loop->PostTask(FROM_HERE, NewRunnableMethod(
worker_.get(), &PrintJobWorker::DismissDialog));
is_print_dialog_box_shown_ = false;
}
- // It will wait infinitely for the worker thread to quit.
- worker_->NonBlockingStop();
+
+ ControlledWorkerShutdown();
+
is_job_pending_ = false;
NotificationService::current()->RemoveObserver(
this, NOTIFY_PRINT_JOB_EVENT, Source<PrintJob>(this));
}
// Flush the cached document.
UpdatePrintedDocument(NULL);
-
- if (is_blocking_) {
- // Make sure we don't get stuck in an inner message loop.
- MessageLoop::current()->Quit();
- is_blocking_ = false;
- }
}
void PrintJob::Cancel() {
@@ -285,8 +273,7 @@ void PrintJob::Cancel() {
bool PrintJob::RequestMissingPages() {
DCHECK_EQ(ui_message_loop_, MessageLoop::current());
DCHECK(!is_print_dialog_box_shown_);
- DCHECK(!is_blocking_);
- if (!is_job_pending_ || is_print_dialog_box_shown_ || is_blocking_)
+ if (!is_job_pending_ || is_print_dialog_box_shown_)
return false;
MessageLoop* worker_loop = worker_.get() ? worker_->message_loop() : NULL;
@@ -314,7 +301,6 @@ bool PrintJob::FlushJob(int timeout_ms) {
false));
}
- is_blocking_ = true;
// Stop() will eventually be called, which will get out of the inner message
// loop. But, don't take it for granted and set a timer in case something goes
// wrong.
@@ -431,6 +417,52 @@ void PrintJob::OnDocumentDone() {
Details<JobEventDetails>(details.get()));
}
+void PrintJob::ControlledWorkerShutdown() {
+ DCHECK_EQ(ui_message_loop_, MessageLoop::current());
+ // We could easily get into a deadlock case if worker_->Stop() is used; the
+ // printer driver created a window as a child of the browser window. By
+ // canceling the job, the printer driver initiated dialog box is destroyed,
+ // which sends a blocking message to its parent window. If the browser window
+ // thread is not processing messages, a deadlock occurs.
+ //
+ // This function ensures that the dialog box will be destroyed in a timely
+ // manner by the mere fact that the thread will terminate. So the potential
+ // deadlock is eliminated.
+ worker_->StopSoon();
+
+ // 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();
+ for (; thread_handle;) {
+ // Note that we don't do any kind of message priorization 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 occured. Assume the thread quit.
+ NOTREACHED();
+ break;
+ }
+ }
+
+ // Now make sure the thread object is cleaned up.
+ worker_->Stop();
+}
+
// Takes settings_ ownership and will be deleted in the receiving thread.
JobEventDetails::JobEventDetails(Type type,
PrintedDocument* document,
diff --git a/chrome/browser/printing/print_job.h b/chrome/browser/printing/print_job.h
index 6f0accd..6d9a6bf 100644
--- a/chrome/browser/printing/print_job.h
+++ b/chrome/browser/printing/print_job.h
@@ -27,8 +27,8 @@
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-#ifndef CHROME_BROWSER_PRINTING_PRINT_JOB_H__
-#define CHROME_BROWSER_PRINTING_PRINT_JOB_H__
+#ifndef CHROME_BROWSER_PRINTING_PRINT_JOB_H_
+#define CHROME_BROWSER_PRINTING_PRINT_JOB_H_
#include "base/ref_counted.h"
#include "chrome/browser/printing/print_job_worker_owner.h"
@@ -153,6 +153,10 @@ class PrintJob : public base::RefCountedThreadSafe<PrintJob>,
// notification.
void OnDocumentDone();
+ // Terminates the worker thread in a very controlled way, to work around any
+ // eventual deadlock.
+ void ControlledWorkerShutdown();
+
// Main message loop reference. Used to send notifications in the right
// thread.
MessageLoop* const ui_message_loop_;
@@ -178,14 +182,11 @@ class PrintJob : public base::RefCountedThreadSafe<PrintJob>,
// Is the Print... dialog box currently shown.
bool is_print_dialog_box_shown_;
- // Is GetSettings() blocking to act like a synchronous function?
- bool is_blocking_;
-
// Is Canceling? If so, try to not cause recursion if on FAILED notification,
// the notified calls Cancel() again.
bool is_canceling_;
- DISALLOW_EVIL_CONSTRUCTORS(PrintJob);
+ DISALLOW_COPY_AND_ASSIGN(PrintJob);
};
// Details for a NOTIFY_PRINT_JOB_EVENT notification. The members may be NULL.
@@ -241,9 +242,9 @@ class JobEventDetails : public base::RefCountedThreadSafe<JobEventDetails> {
scoped_refptr<PrintedPage> page_;
const Type type_;
- DISALLOW_EVIL_CONSTRUCTORS(JobEventDetails);
+ DISALLOW_COPY_AND_ASSIGN(JobEventDetails);
};
} // namespace printing
-#endif // CHROME_BROWSER_PRINTING_PRINT_JOB_H__
+#endif // CHROME_BROWSER_PRINTING_PRINT_JOB_H_
diff --git a/chrome/browser/printing/print_job_worker.cc b/chrome/browser/printing/print_job_worker.cc
index 5a26ef91..39a92bb 100644
--- a/chrome/browser/printing/print_job_worker.cc
+++ b/chrome/browser/printing/print_job_worker.cc
@@ -162,11 +162,12 @@ void PrintJobWorker::OnDocumentChanged(PrintedDocument* new_document) {
}
void PrintJobWorker::OnNewPage() {
- DCHECK_EQ(message_loop(), MessageLoop::current());
if (!document_.get()) {
// Spurious message.
return;
}
+ // message_loop() could return NULL when the print job is cancelled.
+ DCHECK_EQ(message_loop(), MessageLoop::current());
DCHECK(printing_context_.context());
if (!printing_context_.context())
return;
diff --git a/chrome/browser/printing/printing_layout_uitest.cc b/chrome/browser/printing/printing_layout_uitest.cc
index 9a6d04c..ffd20d7 100644
--- a/chrome/browser/printing/printing_layout_uitest.cc
+++ b/chrome/browser/printing/printing_layout_uitest.cc
@@ -565,7 +565,7 @@ TEST_F(PrintingLayoutTestHidden, ManyTimes) {
}
// Prints a popup and immediately closes it.
-TEST_F(PrintingLayoutTest, DISABLED_Delayed) {
+TEST_F(PrintingLayoutTest, Delayed) {
if (IsTestCaseDisabled())
return;
@@ -605,7 +605,7 @@ TEST_F(PrintingLayoutTest, DISABLED_Delayed) {
}
// Prints a popup and immediately closes it.
-TEST_F(PrintingLayoutTest, DISABLED_IFrame) {
+TEST_F(PrintingLayoutTest, IFrame) {
if (IsTestCaseDisabled())
return;
generated by cgit v1.1 at 2025-01-22 00:10:33 +0000