From d77640c7295be71cbc8c810b864ed09dfac9c605 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Tue, 31 Aug 2021 10:37:30 +0900 Subject: [PATCH 1/2] chore: cherry-pick fix for 1231134 from chromium (#30637) * chore: cherry-pick fix for 1231134 from chromium * chore: update patches Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com> --- patches/chromium/.patches | 1 + patches/chromium/cherry-pick-1231134.patch | 163 +++++++++++++++++++++ 2 files changed, 164 insertions(+) create mode 100644 patches/chromium/cherry-pick-1231134.patch diff --git a/patches/chromium/.patches b/patches/chromium/.patches index 5b8fa03fdbf73..e1f759deef89b 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -182,5 +182,6 @@ cherry-pick-cd98d7c0dae9.patch replace_first_of_two_waitableevents_in_creditcardaccessmanager.patch cherry-pick-ac9dc1235e28.patch cherry-pick-1227933.patch +cherry-pick-1231134.patch cherry-pick-1233564.patch cherry-pick-1234009.patch diff --git a/patches/chromium/cherry-pick-1231134.patch b/patches/chromium/cherry-pick-1231134.patch new file mode 100644 index 0000000000000..ccee88fd79b02 --- /dev/null +++ b/patches/chromium/cherry-pick-1231134.patch @@ -0,0 +1,163 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Lei Zhang +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 +Commit-Queue: Lei Zhang +Cr-Original-Commit-Position: refs/heads/master@{#906269} +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3086110 +Bot-Commit: Rubber Stamper +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 26271f6689f1f887b82333d281f26012d2752d63..e73d0972b1b6d6db1fbb3825791e5b90146c2c5e 100644 +--- a/chrome/browser/printing/print_view_manager.cc ++++ b/chrome/browser/printing/print_view_manager.cc +@@ -89,7 +89,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 8b2f150a1e6a042898cba14c971e1f80d04116ca..b5cba8a2dfb9021527e4cc5569635770e85949b3 100644 +--- a/chrome/browser/printing/print_view_manager.h ++++ b/chrome/browser/printing/print_view_manager.h +@@ -128,6 +128,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 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 3b6e1d2609af952fa825688833387518cafdb352..3ddd927654e0134c28a7f73c6ec30b0c1ffe5c49 100644 +--- a/chrome/browser/printing/print_view_manager_base.cc ++++ b/chrome/browser/printing/print_view_manager_base.cc +@@ -370,7 +370,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()) +@@ -850,6 +853,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. +@@ -876,8 +881,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. +@@ -906,7 +914,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() && +@@ -988,7 +999,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 ccb9808bdb334a78ed7b64dd3030caff52055ad6..b2ad5c1010b233e038cad9e2b5e39f3c0027d63e 100644 +--- a/chrome/browser/printing/print_view_manager_base.h ++++ b/chrome/browser/printing/print_view_manager_base.h +@@ -122,6 +122,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 82a2ac9bc0e5d32438a6ec6bd500cae7da8739fe..fe8a580e2fc5dbb74a57bf8488888a9696ce0007 100644 +--- a/chrome/browser/ui/webui/print_preview/print_preview_handler.cc ++++ b/chrome/browser/ui/webui/print_preview/print_preview_handler.cc +@@ -726,9 +726,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(); From a1608e4fce3fba2405cbabdd1e328fa686d61804 Mon Sep 17 00:00:00 2001 From: PatchUp <73610968+patchup[bot]@users.noreply.github.com> Date: Tue, 31 Aug 2021 01:49:00 +0000 Subject: [PATCH 2/2] chore: update patches --- patches/chromium/cherry-pick-1231134.patch | 28 +++++++++++----------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/patches/chromium/cherry-pick-1231134.patch b/patches/chromium/cherry-pick-1231134.patch index ccee88fd79b02..2c95e166f67c4 100644 --- a/patches/chromium/cherry-pick-1231134.patch +++ b/patches/chromium/cherry-pick-1231134.patch @@ -26,10 +26,10 @@ 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 26271f6689f1f887b82333d281f26012d2752d63..e73d0972b1b6d6db1fbb3825791e5b90146c2c5e 100644 +index b7a2fa5a10b2461e83d78fca63e3165d5eb1350f..9f11d5053ff19e4ccfdbe54e7f59e311da3c6e0a 100644 --- a/chrome/browser/printing/print_view_manager.cc +++ b/chrome/browser/printing/print_view_manager.cc -@@ -89,7 +89,11 @@ bool PrintViewManager::PrintForSystemDialogNow( +@@ -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; @@ -42,10 +42,10 @@ index 26271f6689f1f887b82333d281f26012d2752d63..e73d0972b1b6d6db1fbb3825791e5b90 // 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 8b2f150a1e6a042898cba14c971e1f80d04116ca..b5cba8a2dfb9021527e4cc5569635770e85949b3 100644 +index 8770b7bc60cb28d70cde3af8303939bf7ac27892..16c13724c397fade0eb64f9b5ec26c7bf51570ac 100644 --- a/chrome/browser/printing/print_view_manager.h +++ b/chrome/browser/printing/print_view_manager.h -@@ -128,6 +128,11 @@ class PrintViewManager : public PrintViewManagerBase, +@@ -130,6 +130,11 @@ class PrintViewManager : public PrintViewManagerBase, WEB_CONTENTS_USER_DATA_KEY_DECL(); @@ -58,10 +58,10 @@ index 8b2f150a1e6a042898cba14c971e1f80d04116ca..b5cba8a2dfb9021527e4cc5569635770 }; diff --git a/chrome/browser/printing/print_view_manager_base.cc b/chrome/browser/printing/print_view_manager_base.cc -index 3b6e1d2609af952fa825688833387518cafdb352..3ddd927654e0134c28a7f73c6ec30b0c1ffe5c49 100644 +index 75908fc9978db020d752e7fc7211d1a075c11ffb..b896f69f3ec272001c5c6c0071bc23a3c9134d70 100644 --- a/chrome/browser/printing/print_view_manager_base.cc +++ b/chrome/browser/printing/print_view_manager_base.cc -@@ -370,7 +370,10 @@ bool PrintViewManagerBase::PrintNow(content::RenderFrameHost* rfh, +@@ -192,7 +192,10 @@ bool PrintViewManagerBase::PrintNow(content::RenderFrameHost* rfh, bool silent, base::Value settings, CompletionCallback callback) { @@ -72,7 +72,7 @@ index 3b6e1d2609af952fa825688833387518cafdb352..3ddd927654e0134c28a7f73c6ec30b0c // Don't print / print preview crashed tabs. if (IsCrashed()) -@@ -850,6 +853,8 @@ bool PrintViewManagerBase::RenderAllMissingPagesNow() { +@@ -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) @@ -81,7 +81,7 @@ index 3b6e1d2609af952fa825688833387518cafdb352..3ddd927654e0134c28a7f73c6ec30b0c if (!RunInnerMessageLoop()) { // This function is always called from DisconnectFromCurrentPrintJob() so we // know that the job will be stopped/canceled in any case. -@@ -876,8 +881,11 @@ bool PrintViewManagerBase::CreateNewPrintJob( +@@ -651,8 +656,11 @@ bool PrintViewManagerBase::CreateNewPrintJob( DCHECK(query); if (callback_.is_null()) { @@ -93,7 +93,7 @@ index 3b6e1d2609af952fa825688833387518cafdb352..3ddd927654e0134c28a7f73c6ec30b0c } // We can't print if there is no renderer. -@@ -906,7 +914,10 @@ bool PrintViewManagerBase::CreateNewPrintJob( +@@ -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. @@ -104,7 +104,7 @@ index 3b6e1d2609af952fa825688833387518cafdb352..3ddd927654e0134c28a7f73c6ec30b0c // Verify that assertion. if (print_job_ && print_job_->document() && -@@ -988,7 +999,10 @@ bool PrintViewManagerBase::RunInnerMessageLoop() { +@@ -763,7 +774,10 @@ bool PrintViewManagerBase::RunInnerMessageLoop() { quit_inner_loop_ = run_loop.QuitClosure(); @@ -116,10 +116,10 @@ index 3b6e1d2609af952fa825688833387518cafdb352..3ddd927654e0134c28a7f73c6ec30b0c // 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 ccb9808bdb334a78ed7b64dd3030caff52055ad6..b2ad5c1010b233e038cad9e2b5e39f3c0027d63e 100644 +index 095d9dfcc3ee14b646b63c29e406434c1623704a..ed4c0ec98bb84753828d0649799077edc9796317 100644 --- a/chrome/browser/printing/print_view_manager_base.h +++ b/chrome/browser/printing/print_view_manager_base.h -@@ -122,6 +122,8 @@ class PrintViewManagerBase : public content::NotificationObserver, +@@ -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. @@ -145,10 +145,10 @@ index ccb9808bdb334a78ed7b64dd3030caff52055ad6..b2ad5c1010b233e038cad9e2b5e39f3c // 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 82a2ac9bc0e5d32438a6ec6bd500cae7da8739fe..fe8a580e2fc5dbb74a57bf8488888a9696ce0007 100644 +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 -@@ -726,9 +726,12 @@ void PrintPreviewHandler::HandleShowSystemDialog( +@@ -864,9 +864,12 @@ void PrintPreviewHandler::HandleShowSystemDialog( if (!initiator) return;