|
@@ -0,0 +1,163 @@
|
|
|
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
|
|
+From: Lei Zhang <[email protected]>
|
|
|
+Date: Tue, 10 Aug 2021 21:38:36 +0000
|
|
|
+Subject: Do more class validity checks in PrintViewManagerBase.
|
|
|
+
|
|
|
+PrintViewManagerBase runs a nested loop. In some situations,
|
|
|
+PrintViewManagerBase and related classes like PrintViewManager and
|
|
|
+PrintPreviewHandler can get deleted while the nested loop is running.
|
|
|
+When this happens, the nested loop exists to a PrintViewManagerBase
|
|
|
+that is no longer valid.
|
|
|
+
|
|
|
+Use base::WeakPtrs liberally to check for this condition and exit
|
|
|
+safely.
|
|
|
+
|
|
|
+(cherry picked from commit a2cb1fb333d2faacb2fe1380f8d2621b5ee6af7e)
|
|
|
+
|
|
|
+Bug: 1231134
|
|
|
+Change-Id: I21ec131574331ce973d22594c11e70088147e149
|
|
|
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3057880
|
|
|
+Reviewed-by: Alan Screen <[email protected]>
|
|
|
+Commit-Queue: Lei Zhang <[email protected]>
|
|
|
+Cr-Original-Commit-Position: refs/heads/master@{#906269}
|
|
|
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3086110
|
|
|
+Bot-Commit: Rubber Stamper <[email protected]>
|
|
|
+Cr-Commit-Position: refs/branch-heads/4515@{#2024}
|
|
|
+Cr-Branched-From: 488fc70865ddaa05324ac00a54a6eb783b4bc41c-refs/heads/master@{#885287}
|
|
|
+
|
|
|
+diff --git a/chrome/browser/printing/print_view_manager.cc b/chrome/browser/printing/print_view_manager.cc
|
|
|
+index b7a2fa5a10b2461e83d78fca63e3165d5eb1350f..9f11d5053ff19e4ccfdbe54e7f59e311da3c6e0a 100644
|
|
|
+--- a/chrome/browser/printing/print_view_manager.cc
|
|
|
++++ b/chrome/browser/printing/print_view_manager.cc
|
|
|
+@@ -91,7 +91,11 @@ bool PrintViewManager::PrintForSystemDialogNow(
|
|
|
+ DCHECK(!on_print_dialog_shown_callback_);
|
|
|
+ on_print_dialog_shown_callback_ = std::move(dialog_shown_callback);
|
|
|
+ is_switching_to_system_dialog_ = true;
|
|
|
++
|
|
|
++ auto weak_this = weak_factory_.GetWeakPtr();
|
|
|
+ DisconnectFromCurrentPrintJob();
|
|
|
++ if (!weak_this)
|
|
|
++ return false;
|
|
|
+
|
|
|
+ // Don't print / print preview crashed tabs.
|
|
|
+ if (IsCrashed())
|
|
|
+diff --git a/chrome/browser/printing/print_view_manager.h b/chrome/browser/printing/print_view_manager.h
|
|
|
+index 8770b7bc60cb28d70cde3af8303939bf7ac27892..16c13724c397fade0eb64f9b5ec26c7bf51570ac 100644
|
|
|
+--- a/chrome/browser/printing/print_view_manager.h
|
|
|
++++ b/chrome/browser/printing/print_view_manager.h
|
|
|
+@@ -130,6 +130,11 @@ class PrintViewManager : public PrintViewManagerBase,
|
|
|
+
|
|
|
+ WEB_CONTENTS_USER_DATA_KEY_DECL();
|
|
|
+
|
|
|
++ // Keep this last so that all weak pointers will be invalidated at the
|
|
|
++ // beginning of destruction. Note that PrintViewManagerBase has its own
|
|
|
++ // base::WeakPtrFactory as well, but PrintViewManager should use this one.
|
|
|
++ base::WeakPtrFactory<PrintViewManager> weak_factory_{this};
|
|
|
++
|
|
|
+ DISALLOW_COPY_AND_ASSIGN(PrintViewManager);
|
|
|
+ };
|
|
|
+
|
|
|
+diff --git a/chrome/browser/printing/print_view_manager_base.cc b/chrome/browser/printing/print_view_manager_base.cc
|
|
|
+index 75908fc9978db020d752e7fc7211d1a075c11ffb..b896f69f3ec272001c5c6c0071bc23a3c9134d70 100644
|
|
|
+--- a/chrome/browser/printing/print_view_manager_base.cc
|
|
|
++++ b/chrome/browser/printing/print_view_manager_base.cc
|
|
|
+@@ -192,7 +192,10 @@ bool PrintViewManagerBase::PrintNow(content::RenderFrameHost* rfh,
|
|
|
+ bool silent,
|
|
|
+ base::Value settings,
|
|
|
+ CompletionCallback callback) {
|
|
|
++ auto weak_this = weak_ptr_factory_.GetWeakPtr();
|
|
|
+ DisconnectFromCurrentPrintJob();
|
|
|
++ if (!weak_this)
|
|
|
++ return false;
|
|
|
+
|
|
|
+ // Don't print / print preview crashed tabs.
|
|
|
+ if (IsCrashed())
|
|
|
+@@ -625,6 +628,8 @@ bool PrintViewManagerBase::RenderAllMissingPagesNow() {
|
|
|
+ // or in DidPrintDocument(). The check is done in
|
|
|
+ // ShouldQuitFromInnerMessageLoop().
|
|
|
+ // BLOCKS until all the pages are received. (Need to enable recursive task)
|
|
|
++ // WARNING: Do not do any work after RunInnerMessageLoop() returns, as `this`
|
|
|
++ // may have gone away.
|
|
|
+ if (!RunInnerMessageLoop()) {
|
|
|
+ // This function is always called from DisconnectFromCurrentPrintJob() so we
|
|
|
+ // know that the job will be stopped/canceled in any case.
|
|
|
+@@ -651,8 +656,11 @@ bool PrintViewManagerBase::CreateNewPrintJob(
|
|
|
+ DCHECK(query);
|
|
|
+
|
|
|
+ if (callback_.is_null()) {
|
|
|
++ auto weak_this = weak_ptr_factory_.GetWeakPtr();
|
|
|
+ // Disconnect the current |print_job_| only when calling window.print()
|
|
|
+ DisconnectFromCurrentPrintJob();
|
|
|
++ if (!weak_this)
|
|
|
++ return false;
|
|
|
+ }
|
|
|
+
|
|
|
+ // We can't print if there is no renderer.
|
|
|
+@@ -681,7 +689,10 @@ bool PrintViewManagerBase::CreateNewPrintJob(
|
|
|
+ void PrintViewManagerBase::DisconnectFromCurrentPrintJob() {
|
|
|
+ // Make sure all the necessary rendered page are done. Don't bother with the
|
|
|
+ // return value.
|
|
|
++ auto weak_this = weak_ptr_factory_.GetWeakPtr();
|
|
|
+ bool result = RenderAllMissingPagesNow();
|
|
|
++ if (!weak_this)
|
|
|
++ return;
|
|
|
+
|
|
|
+ // Verify that assertion.
|
|
|
+ if (print_job_ && print_job_->document() &&
|
|
|
+@@ -763,7 +774,10 @@ bool PrintViewManagerBase::RunInnerMessageLoop() {
|
|
|
+
|
|
|
+ quit_inner_loop_ = run_loop.QuitClosure();
|
|
|
+
|
|
|
++ auto weak_this = weak_ptr_factory_.GetWeakPtr();
|
|
|
+ run_loop.Run();
|
|
|
++ if (!weak_this)
|
|
|
++ return false;
|
|
|
+
|
|
|
+ // If the inner-loop quit closure is still set then we timed out.
|
|
|
+ bool success = !quit_inner_loop_;
|
|
|
+diff --git a/chrome/browser/printing/print_view_manager_base.h b/chrome/browser/printing/print_view_manager_base.h
|
|
|
+index 095d9dfcc3ee14b646b63c29e406434c1623704a..ed4c0ec98bb84753828d0649799077edc9796317 100644
|
|
|
+--- a/chrome/browser/printing/print_view_manager_base.h
|
|
|
++++ b/chrome/browser/printing/print_view_manager_base.h
|
|
|
+@@ -115,6 +115,8 @@ class PrintViewManagerBase : public content::NotificationObserver,
|
|
|
+
|
|
|
+ // Makes sure the current print_job_ has all its data before continuing, and
|
|
|
+ // disconnect from it.
|
|
|
++ // WARNING: `this` may not be alive after DisconnectFromCurrentPrintJob()
|
|
|
++ // returns.
|
|
|
+ void DisconnectFromCurrentPrintJob();
|
|
|
+
|
|
|
+ // Manages the low-level talk to the printer.
|
|
|
+@@ -168,6 +170,7 @@ class PrintViewManagerBase : public content::NotificationObserver,
|
|
|
+ // Requests the RenderView to render all the missing pages for the print job.
|
|
|
+ // No-op if no print job is pending. Returns true if at least one page has
|
|
|
+ // been requested to the renderer.
|
|
|
++ // WARNING: `this` may not be alive after RenderAllMissingPagesNow() returns.
|
|
|
+ bool RenderAllMissingPagesNow();
|
|
|
+
|
|
|
+ // Checks that synchronization is correct with |print_job_| based on |cookie|.
|
|
|
+@@ -201,6 +204,7 @@ class PrintViewManagerBase : public content::NotificationObserver,
|
|
|
+ // while the blocking inner message loop is running. This is useful in cases
|
|
|
+ // where the RenderView is about to be destroyed while a printing job isn't
|
|
|
+ // finished.
|
|
|
++ // WARNING: `this` may not be alive after RunInnerMessageLoop() returns.
|
|
|
+ bool RunInnerMessageLoop();
|
|
|
+
|
|
|
+ // In the case of Scripted Printing, where the renderer is controlling the
|
|
|
+diff --git a/chrome/browser/ui/webui/print_preview/print_preview_handler.cc b/chrome/browser/ui/webui/print_preview/print_preview_handler.cc
|
|
|
+index 2cea700c82790f696775ece3080662232326aabc..ab89b180e6f9586d6280d884f126fb46b278be04 100644
|
|
|
+--- a/chrome/browser/ui/webui/print_preview/print_preview_handler.cc
|
|
|
++++ b/chrome/browser/ui/webui/print_preview/print_preview_handler.cc
|
|
|
+@@ -864,9 +864,12 @@ void PrintPreviewHandler::HandleShowSystemDialog(
|
|
|
+ if (!initiator)
|
|
|
+ return;
|
|
|
+
|
|
|
++ auto weak_this = weak_factory_.GetWeakPtr();
|
|
|
+ auto* print_view_manager = PrintViewManager::FromWebContents(initiator);
|
|
|
+ print_view_manager->PrintForSystemDialogNow(base::BindOnce(
|
|
|
+ &PrintPreviewHandler::ClosePreviewDialog, weak_factory_.GetWeakPtr()));
|
|
|
++ if (!weak_this)
|
|
|
++ return;
|
|
|
+
|
|
|
+ // Cancel the pending preview request if exists.
|
|
|
+ print_preview_ui()->OnCancelPendingPreviewRequest();
|