From 56c6d25e98d6c0caef124dc81113f264efccc8ef Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Tue, 1 Feb 2022 20:00:09 +0100 Subject: [PATCH] fix: crash on printer dialog cancellation (#32632) * fix: crash on printer dialog cancellation * fix: remove commented out code * chore: address review --- patches/chromium/printing.patch | 235 +++++++++++++++----------------- 1 file changed, 113 insertions(+), 122 deletions(-) diff --git a/patches/chromium/printing.patch b/patches/chromium/printing.patch index aa2bc07b2789..cc7c628d4868 100644 --- a/patches/chromium/printing.patch +++ b/patches/chromium/printing.patch @@ -11,7 +11,7 @@ majority of changes originally come from these PRs: This patch also fixes callback for manual user cancellation and success. diff --git a/chrome/browser/printing/print_job.cc b/chrome/browser/printing/print_job.cc -index 0de0532d64897c91ce0f72d165976e12e1dec03e..13167ca3f9c0d4895fecd40ab1e2d397c6e85a0b 100644 +index 0de0532d64897c91ce0f72d165976e12e1dec03e..735da67a83b10c56d747e2a2b512f7b7d1aae142 100644 --- a/chrome/browser/printing/print_job.cc +++ b/chrome/browser/printing/print_job.cc @@ -88,6 +88,7 @@ bool PrintWithReducedRasterization(PrefService* prefs) { @@ -54,50 +54,11 @@ index 0de0532d64897c91ce0f72d165976e12e1dec03e..13167ca3f9c0d4895fecd40ab1e2d397 ? PdfRenderSettings::Mode::POSTSCRIPT_LEVEL3_WITH_TYPE42_FONTS : PdfRenderSettings::Mode::POSTSCRIPT_LEVEL3; } -@@ -504,6 +510,20 @@ void PrintJob::OnPageDone(PrintedPage* page) { - } - #endif // defined(OS_WIN) - -+void PrintJob::OnUserInitCancelled() { -+ DCHECK_CURRENTLY_ON(content::BrowserThread::UI); -+ // Make sure a `Cancel()` is broadcast. -+ auto details = base::MakeRefCounted(JobEventDetails::USER_INIT_CANCELED, -+ 0, nullptr); -+ content::NotificationService::current()->Notify( -+ chrome::NOTIFICATION_PRINT_JOB_EVENT, content::Source(this), -+ content::Details(details.get())); -+ -+ for (auto& observer : observers_) { -+ observer.OnUserInitCancelled(); -+ } -+} -+ - void PrintJob::OnFailed() { - DCHECK_CURRENTLY_ON(content::BrowserThread::UI); - diff --git a/chrome/browser/printing/print_job.h b/chrome/browser/printing/print_job.h -index e19f62354edb8acad722c6680296b7d2f55f51fe..b5539171655d78634ee89faf3516d23ce5718353 100644 +index e19f62354edb8acad722c6680296b7d2f55f51fe..51c41b4dbab81ffe5840d59ef45b661cf5c5534b 100644 --- a/chrome/browser/printing/print_job.h +++ b/chrome/browser/printing/print_job.h -@@ -53,6 +53,7 @@ class PrintJob : public base::RefCountedThreadSafe { - public: - virtual void OnDocDone(int job_id, PrintedDocument* document) {} - virtual void OnJobDone() {} -+ virtual void OnUserInitCancelled() {} - virtual void OnFailed() {} - }; - -@@ -100,6 +101,9 @@ class PrintJob : public base::RefCountedThreadSafe { - // Called when the document is done printing. - virtual void OnDocDone(int job_id, PrintedDocument* document); - -+ // Called if the user cancels the print job. -+ virtual void OnUserInitCancelled(); -+ - // Called if the document fails to print. - virtual void OnFailed(); - -@@ -257,6 +261,9 @@ class JobEventDetails : public base::RefCountedThreadSafe { +@@ -257,6 +257,9 @@ class JobEventDetails : public base::RefCountedThreadSafe { public: // Event type. enum Type { @@ -108,7 +69,7 @@ index e19f62354edb8acad722c6680296b7d2f55f51fe..b5539171655d78634ee89faf3516d23c NEW_DOC, diff --git a/chrome/browser/printing/print_job_worker.cc b/chrome/browser/printing/print_job_worker.cc -index d8f67ab5116483eb2eeb4cc09f19bbcbccb74b37..b4ab0984765c9711ddacf32f7147108cdfbc5096 100644 +index d8f67ab5116483eb2eeb4cc09f19bbcbccb74b37..4283a5743c695a7376722f80925722d9e7a6496e 100644 --- a/chrome/browser/printing/print_job_worker.cc +++ b/chrome/browser/printing/print_job_worker.cc @@ -20,13 +20,13 @@ @@ -126,18 +87,7 @@ index d8f67ab5116483eb2eeb4cc09f19bbcbccb74b37..b4ab0984765c9711ddacf32f7147108c #include "printing/backend/print_backend.h" #include "printing/buildflags/buildflags.h" #include "printing/mojom/print.mojom.h" -@@ -125,6 +125,10 @@ void FailedNotificationCallback(PrintJob* print_job) { - print_job->OnFailed(); - } - -+void UserInitCancelledNotificationCallback(PrintJob* print_job) { -+ print_job->OnUserInitCancelled(); -+} -+ - #if defined(OS_WIN) - void PageNotificationCallback(PrintJob* print_job, PrintedPage* page) { - print_job->OnPageDone(page); -@@ -245,16 +249,21 @@ void PrintJobWorker::UpdatePrintSettings(base::Value new_settings, +@@ -245,16 +245,21 @@ void PrintJobWorker::UpdatePrintSettings(base::Value new_settings, #endif // defined(OS_LINUX) && defined(USE_CUPS) } @@ -162,21 +112,8 @@ index d8f67ab5116483eb2eeb4cc09f19bbcbccb74b37..b4ab0984765c9711ddacf32f7147108c } #if defined(OS_CHROMEOS) -@@ -270,6 +279,12 @@ void PrintJobWorker::UpdatePrintSettingsFromPOD( - - void PrintJobWorker::GetSettingsDone(SettingsCallback callback, - mojom::ResultCode result) { -+ if (result == mojom::ResultCode::kCanceled) { -+ print_job_->PostTask( -+ FROM_HERE, -+ base::BindOnce(&UserInitCancelledNotificationCallback, -+ base::RetainedRef(print_job_.get()))); -+ } - std::move(callback).Run(printing_context_->TakeAndResetSettings(), result); - } - diff --git a/chrome/browser/printing/print_view_manager_base.cc b/chrome/browser/printing/print_view_manager_base.cc -index c9f1502da55d89de0eede73a7d39047c090594d0..1320afa83016ea72e5dbc23b1ed63cf91d439102 100644 +index c9f1502da55d89de0eede73a7d39047c090594d0..c857a48e00c3535c74691f4e36d44e0478e3b15c 100644 --- a/chrome/browser/printing/print_view_manager_base.cc +++ b/chrome/browser/printing/print_view_manager_base.cc @@ -29,10 +29,10 @@ @@ -228,7 +165,26 @@ index c9f1502da55d89de0eede73a7d39047c090594d0..1320afa83016ea72e5dbc23b1ed63cf9 RenderParamsFromPrintSettings(printer_query->settings(), params->params.get()); params->params->document_cookie = printer_query->cookie(); -@@ -319,12 +325,14 @@ PrintViewManagerBase::PrintViewManagerBase(content::WebContents* web_contents) +@@ -270,6 +276,7 @@ void ScriptedPrintReplyOnIO( + mojom::PrintManagerHost::ScriptedPrintCallback callback) { + DCHECK_CURRENTLY_ON(content::BrowserThread::IO); + mojom::PrintPagesParamsPtr params = CreateEmptyPrintPagesParamsPtr(); ++ + if (printer_query->last_status() == mojom::ResultCode::kSuccess && + printer_query->settings().dpi()) { + RenderParamsFromPrintSettings(printer_query->settings(), +@@ -279,8 +286,9 @@ void ScriptedPrintReplyOnIO( + } + bool has_valid_cookie = params->params->document_cookie; + bool has_dpi = !params->params->dpi.IsEmpty(); ++ bool canceled = printer_query->last_status() == mojom::ResultCode::kCanceled; + content::GetUIThreadTaskRunner({})->PostTask( +- FROM_HERE, base::BindOnce(std::move(callback), std::move(params))); ++ FROM_HERE, base::BindOnce(std::move(callback), std::move(params), canceled)); + + if (has_dpi && has_valid_cookie) { + queue->QueuePrinterQuery(std::move(printer_query)); +@@ -319,12 +327,14 @@ PrintViewManagerBase::PrintViewManagerBase(content::WebContents* web_contents) : PrintManager(web_contents), queue_(g_browser_process->print_job_manager()->queue()) { DCHECK(queue_); @@ -243,7 +199,7 @@ index c9f1502da55d89de0eede73a7d39047c090594d0..1320afa83016ea72e5dbc23b1ed63cf9 } PrintViewManagerBase::~PrintViewManagerBase() { -@@ -332,7 +340,10 @@ PrintViewManagerBase::~PrintViewManagerBase() { +@@ -332,7 +342,10 @@ PrintViewManagerBase::~PrintViewManagerBase() { DisconnectFromCurrentPrintJob(); } @@ -255,22 +211,39 @@ index c9f1502da55d89de0eede73a7d39047c090594d0..1320afa83016ea72e5dbc23b1ed63cf9 auto weak_this = weak_ptr_factory_.GetWeakPtr(); DisconnectFromCurrentPrintJob(); if (!weak_this) -@@ -347,7 +358,13 @@ bool PrintViewManagerBase::PrintNow(content::RenderFrameHost* rfh) { +@@ -347,7 +360,9 @@ bool PrintViewManagerBase::PrintNow(content::RenderFrameHost* rfh) { // go in `ReleasePrintJob()`. SetPrintingRFH(rfh); - GetPrintRenderFrame(rfh)->PrintRequestedPages(); + callback_ = std::move(callback); + -+ if (!callback_.is_null()) { -+ print_job_->AddObserver(*this); -+ } -+ + GetPrintRenderFrame(rfh)->PrintRequestedPages(silent, std::move(settings)); for (auto& observer : GetObservers()) observer.OnPrintNow(rfh); -@@ -506,9 +523,9 @@ void PrintViewManagerBase::ScriptedPrintReply( +@@ -491,7 +506,8 @@ void PrintViewManagerBase::GetDefaultPrintSettingsReply( + void PrintViewManagerBase::ScriptedPrintReply( + ScriptedPrintCallback callback, + int process_id, +- mojom::PrintPagesParamsPtr params) { ++ mojom::PrintPagesParamsPtr params, ++ bool canceled) { + DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + + if (!content::RenderProcessHost::FromID(process_id)) { +@@ -499,16 +515,19 @@ void PrintViewManagerBase::ScriptedPrintReply( + return; + } + ++ if (canceled) ++ UserInitCanceled(); ++ + set_cookie(params->params->document_cookie); +- std::move(callback).Run(std::move(params)); ++ std::move(callback).Run(std::move(params), canceled); + } + void PrintViewManagerBase::UpdatePrintingEnabled() { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); // The Unretained() is safe because ForEachFrame() is synchronous. @@ -283,7 +256,7 @@ index c9f1502da55d89de0eede73a7d39047c090594d0..1320afa83016ea72e5dbc23b1ed63cf9 } void PrintViewManagerBase::NavigationStopped() { -@@ -622,12 +639,13 @@ void PrintViewManagerBase::DidPrintDocument( +@@ -622,12 +641,13 @@ void PrintViewManagerBase::DidPrintDocument( void PrintViewManagerBase::GetDefaultPrintSettings( GetDefaultPrintSettingsCallback callback) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); @@ -298,7 +271,7 @@ index c9f1502da55d89de0eede73a7d39047c090594d0..1320afa83016ea72e5dbc23b1ed63cf9 content::RenderFrameHost* render_frame_host = GetCurrentTargetFrame(); auto callback_wrapper = base::BindOnce(&PrintViewManagerBase::GetDefaultPrintSettingsReply, -@@ -645,18 +663,20 @@ void PrintViewManagerBase::UpdatePrintSettings( +@@ -645,18 +665,20 @@ void PrintViewManagerBase::UpdatePrintSettings( base::Value job_settings, UpdatePrintSettingsCallback callback) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); @@ -320,7 +293,7 @@ index c9f1502da55d89de0eede73a7d39047c090594d0..1320afa83016ea72e5dbc23b1ed63cf9 content::BrowserContext* context = web_contents() ? web_contents()->GetBrowserContext() : nullptr; PrefService* prefs = -@@ -666,6 +686,7 @@ void PrintViewManagerBase::UpdatePrintSettings( +@@ -666,6 +688,7 @@ void PrintViewManagerBase::UpdatePrintSettings( if (value > 0) job_settings.SetIntKey(kSettingRasterizePdfDpi, value); } @@ -328,7 +301,16 @@ index c9f1502da55d89de0eede73a7d39047c090594d0..1320afa83016ea72e5dbc23b1ed63cf9 auto callback_wrapper = base::BindOnce(&PrintViewManagerBase::UpdatePrintSettingsReply, -@@ -714,7 +735,6 @@ void PrintViewManagerBase::PrintingFailed(int32_t cookie) { +@@ -691,7 +714,7 @@ void PrintViewManagerBase::ScriptedPrint(mojom::ScriptedPrintParamsPtr params, + // didn't happen for some reason. + bad_message::ReceivedBadMessage( + render_process_host, bad_message::PVMB_SCRIPTED_PRINT_FENCED_FRAME); +- std::move(callback).Run(CreateEmptyPrintPagesParamsPtr()); ++ std::move(callback).Run(CreateEmptyPrintPagesParamsPtr(), false); + return; + } + int process_id = render_process_host->GetID(); +@@ -714,7 +737,6 @@ void PrintViewManagerBase::PrintingFailed(int32_t cookie) { PrintManager::PrintingFailed(cookie); #if BUILDFLAG(ENABLE_PRINT_PREVIEW) @@ -336,7 +318,7 @@ index c9f1502da55d89de0eede73a7d39047c090594d0..1320afa83016ea72e5dbc23b1ed63cf9 #endif ReleasePrinterQuery(); -@@ -729,6 +749,11 @@ void PrintViewManagerBase::RemoveObserver(Observer& observer) { +@@ -729,6 +751,11 @@ void PrintViewManagerBase::RemoveObserver(Observer& observer) { } void PrintViewManagerBase::ShowInvalidPrinterSettingsError() { @@ -348,7 +330,7 @@ index c9f1502da55d89de0eede73a7d39047c090594d0..1320afa83016ea72e5dbc23b1ed63cf9 base::ThreadTaskRunnerHandle::Get()->PostTask( FROM_HERE, base::BindOnce(&ShowWarningMessageBox, l10n_util::GetStringUTF16( -@@ -739,8 +764,10 @@ void PrintViewManagerBase::RenderFrameHostStateChanged( +@@ -739,8 +766,10 @@ void PrintViewManagerBase::RenderFrameHostStateChanged( content::RenderFrameHost* render_frame_host, content::RenderFrameHost::LifecycleState /*old_state*/, content::RenderFrameHost::LifecycleState new_state) { @@ -359,19 +341,19 @@ index c9f1502da55d89de0eede73a7d39047c090594d0..1320afa83016ea72e5dbc23b1ed63cf9 } void PrintViewManagerBase::DidStartLoading() { -@@ -808,6 +835,11 @@ void PrintViewManagerBase::OnJobDone() { +@@ -808,6 +837,11 @@ void PrintViewManagerBase::OnJobDone() { ReleasePrintJob(); } -+void PrintViewManagerBase::OnUserInitCancelled() { -+ printing_cancelled_ = true; ++void PrintViewManagerBase::UserInitCanceled() { ++ printing_canceled_ = true; + ReleasePrintJob(); +} + void PrintViewManagerBase::OnFailed() { TerminatePrintJob(true); } -@@ -869,7 +901,10 @@ bool PrintViewManagerBase::CreateNewPrintJob( +@@ -869,7 +903,10 @@ bool PrintViewManagerBase::CreateNewPrintJob( // Disconnect the current |print_job_|. auto weak_this = weak_ptr_factory_.GetWeakPtr(); @@ -383,40 +365,21 @@ index c9f1502da55d89de0eede73a7d39047c090594d0..1320afa83016ea72e5dbc23b1ed63cf9 if (!weak_this) return false; -@@ -891,8 +926,6 @@ bool PrintViewManagerBase::CreateNewPrintJob( - : PrintJob::Source::PRINT_PREVIEW, - /*source_id=*/""); - #endif -- print_job_->AddObserver(*this); -- - printing_succeeded_ = false; - return true; - } -@@ -944,14 +977,21 @@ void PrintViewManagerBase::ReleasePrintJob() { +@@ -944,6 +981,13 @@ void PrintViewManagerBase::ReleasePrintJob() { content::RenderFrameHost* rfh = printing_rfh_; printing_rfh_ = nullptr; + if (!callback_.is_null()) { -+ print_job_->RemoveObserver(*this); -+ + std::string cb_str = ""; + if (!printing_succeeded_) -+ cb_str = printing_cancelled_ ? "cancelled" : "failed"; ++ cb_str = printing_canceled_ ? "canceled" : "failed"; + std::move(callback_).Run(printing_succeeded_, cb_str); + } + if (!print_job_) return; - if (rfh) - GetPrintRenderFrame(rfh)->PrintingDone(printing_succeeded_); - -- print_job_->RemoveObserver(*this); -- - // Don't close the worker thread. - print_job_ = nullptr; - } -@@ -989,7 +1029,7 @@ bool PrintViewManagerBase::RunInnerMessageLoop() { +@@ -989,7 +1033,7 @@ bool PrintViewManagerBase::RunInnerMessageLoop() { } bool PrintViewManagerBase::OpportunisticallyCreatePrintJob(int cookie) { @@ -426,7 +389,7 @@ index c9f1502da55d89de0eede73a7d39047c090594d0..1320afa83016ea72e5dbc23b1ed63cf9 if (!cookie) { diff --git a/chrome/browser/printing/print_view_manager_base.h b/chrome/browser/printing/print_view_manager_base.h -index 5771a3ebd76145c6cf8a2ccc33abc886802ed59f..1562d6331a9cafd530db42c436e878bac427566d 100644 +index 5771a3ebd76145c6cf8a2ccc33abc886802ed59f..fb3af5e3f57f868fd00716f76f70aa63c4cb99c9 100644 --- a/chrome/browser/printing/print_view_manager_base.h +++ b/chrome/browser/printing/print_view_manager_base.h @@ -37,6 +37,8 @@ namespace printing { @@ -450,15 +413,25 @@ index 5771a3ebd76145c6cf8a2ccc33abc886802ed59f..1562d6331a9cafd530db42c436e878ba #if BUILDFLAG(ENABLE_PRINT_PREVIEW) // Prints the document in |print_data| with settings specified in -@@ -143,6 +148,7 @@ class PrintViewManagerBase : public PrintManager, public PrintJob::Observer { - // PrintJob::Observer overrides: - void OnDocDone(int job_id, PrintedDocument* document) override; - void OnJobDone() override; -+ void OnUserInitCancelled() override; - void OnFailed() override; +@@ -106,6 +111,7 @@ class PrintViewManagerBase : public PrintManager, public PrintJob::Observer { + ScriptedPrintCallback callback) override; + void ShowInvalidPrinterSettingsError() override; + void PrintingFailed(int32_t cookie) override; ++ void UserInitCanceled(); - base::ObserverList& GetObservers() { return observers_; } -@@ -252,9 +258,15 @@ class PrintViewManagerBase : public PrintManager, public PrintJob::Observer { + // Adds and removes observers for `PrintViewManagerBase` events. The order in + // which notifications are sent to observers is undefined. Observers must be +@@ -197,7 +203,8 @@ class PrintViewManagerBase : public PrintManager, public PrintJob::Observer { + // Runs `callback` with `params` to reply to ScriptedPrint(). + void ScriptedPrintReply(ScriptedPrintCallback callback, + int process_id, +- mojom::PrintPagesParamsPtr params); ++ mojom::PrintPagesParamsPtr params, ++ bool canceled); + + // 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 +@@ -252,9 +259,15 @@ class PrintViewManagerBase : public PrintManager, public PrintJob::Observer { // The current RFH that is printing with a system printing dialog. raw_ptr printing_rfh_ = nullptr; @@ -468,14 +441,14 @@ index 5771a3ebd76145c6cf8a2ccc33abc886802ed59f..1562d6331a9cafd530db42c436e878ba // Indication of success of the print job. bool printing_succeeded_ = false; -+ // Indication of whether the print job was manually cancelled -+ bool printing_cancelled_ = false; ++ // Indication of whether the print job was manually canceled ++ bool printing_canceled_ = false; + // Set while running an inner message loop inside RenderAllMissingPagesNow(). // This means we are _blocking_ until all the necessary pages have been // rendered or the print settings are being loaded. diff --git a/components/printing/common/print.mojom b/components/printing/common/print.mojom -index 51ebcb4ae399018d3fd8566656596a7ef1f148af..5f2b807fc364131f4c3e6a1646ec522ddc826d9c 100644 +index 51ebcb4ae399018d3fd8566656596a7ef1f148af..c0fbff95137e2e5bccb9702a8cc858df2d989964 100644 --- a/components/printing/common/print.mojom +++ b/components/printing/common/print.mojom @@ -274,7 +274,7 @@ interface PrintPreviewUI { @@ -487,8 +460,17 @@ index 51ebcb4ae399018d3fd8566656596a7ef1f148af..5f2b807fc364131f4c3e6a1646ec522d // Tells the RenderFrame to switch the CSS to print media type, render every // requested page using the print preview document's frame/node, and then +@@ -341,7 +341,7 @@ interface PrintManagerHost { + // Request the print settings from the user. This step is about showing + // UI to the user to select the final print settings. + [Sync] +- ScriptedPrint(ScriptedPrintParams params) => (PrintPagesParams settings); ++ ScriptedPrint(ScriptedPrintParams params) => (PrintPagesParams settings, bool canceled); + + // Tells the browser that there are invalid printer settings. + ShowInvalidPrinterSettingsError(); diff --git a/components/printing/renderer/print_render_frame_helper.cc b/components/printing/renderer/print_render_frame_helper.cc -index 650b5550f982fa5c5c522efaa9b8e305b7edc5e7..260b5521dccadf07eba2c67fa3d9c3da80b49104 100644 +index 650b5550f982fa5c5c522efaa9b8e305b7edc5e7..1b776a614d6e0f6c3ae13ef3c705bc19544cfe12 100644 --- a/components/printing/renderer/print_render_frame_helper.cc +++ b/components/printing/renderer/print_render_frame_helper.cc @@ -39,6 +39,7 @@ @@ -657,6 +639,15 @@ index 650b5550f982fa5c5c522efaa9b8e305b7edc5e7..260b5521dccadf07eba2c67fa3d9c3da notify_browser_of_print_failure_ = false; GetPrintManagerHost()->ShowInvalidPrinterSettingsError(); return false; +@@ -2350,7 +2380,7 @@ mojom::PrintPagesParamsPtr PrintRenderFrameHelper::GetPrintSettingsFromUser( + std::move(params), + base::BindOnce( + [](base::OnceClosure quit_closure, mojom::PrintPagesParamsPtr* output, +- mojom::PrintPagesParamsPtr input) { ++ mojom::PrintPagesParamsPtr input, bool canceled) { + *output = std::move(input); + std::move(quit_closure).Run(); + }, @@ -2579,18 +2609,7 @@ void PrintRenderFrameHelper::RequestPrintPreview(PrintPreviewRequestType type) { }