From 654aab24c4190f379eaca6dc79c46386dca55ab3 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Fri, 29 Jul 2022 23:34:58 +0200 Subject: [PATCH] fix: clean up callback handling in webContents.print() --- patches/chromium/printing.patch | 144 ++++++++++++++++++++++---------- 1 file changed, 99 insertions(+), 45 deletions(-) diff --git a/patches/chromium/printing.patch b/patches/chromium/printing.patch index a229104e82827..769b81a8654d9 100644 --- a/patches/chromium/printing.patch +++ b/patches/chromium/printing.patch @@ -111,7 +111,7 @@ index 1e158ecd686e775f656d5a05a9d916ce8f075fa8..20613012d1e6f435c3211d78ec311cf0 void PrintJobWorkerOop::UnregisterServiceManagerClient() { diff --git a/chrome/browser/printing/print_view_manager_base.cc b/chrome/browser/printing/print_view_manager_base.cc -index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..560e84fcddf1108114cc863ac0bd9cb7f7f4da4c 100644 +index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..471440d0c1a23d028d0fb476f2c6315354305b0b 100644 --- a/chrome/browser/printing/print_view_manager_base.cc +++ b/chrome/browser/printing/print_view_manager_base.cc @@ -30,10 +30,10 @@ @@ -135,7 +135,22 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..560e84fcddf1108114cc863ac0bd9cb7 #include "mojo/public/cpp/system/buffer.h" #include "printing/buildflags/buildflags.h" #include "printing/metafile_skia.h" -@@ -87,6 +88,8 @@ using PrintSettingsCallback = +@@ -83,10 +84,23 @@ namespace printing { + + namespace { + ++std::string PrintReasonFromPrintStatus(PrintViewManager::PrintStatus status) { ++ if (status == PrintViewManager::PrintStatus::kInvalid) { ++ return "Invalid printer settings"; ++ } else if (status == PrintViewManager::PrintStatus::kCanceled) { ++ return "Print job canceled"; ++ } else if (status == PrintViewManager::PrintStatus::kFailed) { ++ return "Print job failed"; ++ } ++ return ""; ++} ++ + using PrintSettingsCallback = base::OnceCallback)>; void ShowWarningMessageBox(const std::u16string& message) { @@ -144,7 +159,7 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..560e84fcddf1108114cc863ac0bd9cb7 // Runs always on the UI thread. static bool is_dialog_shown = false; if (is_dialog_shown) -@@ -95,6 +98,7 @@ void ShowWarningMessageBox(const std::u16string& message) { +@@ -95,6 +109,7 @@ void ShowWarningMessageBox(const std::u16string& message) { base::AutoReset auto_reset(&is_dialog_shown, true); chrome::ShowWarningMessageBox(nullptr, std::u16string(), message); @@ -152,7 +167,7 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..560e84fcddf1108114cc863ac0bd9cb7 } void OnDidGetDefaultPrintSettings( -@@ -144,7 +148,9 @@ void OnDidUpdatePrintSettings( +@@ -144,7 +159,9 @@ void OnDidUpdatePrintSettings( DCHECK_CURRENTLY_ON(content::BrowserThread::UI); DCHECK(printer_query); mojom::PrintPagesParamsPtr params = CreateEmptyPrintPagesParamsPtr(); @@ -163,7 +178,7 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..560e84fcddf1108114cc863ac0bd9cb7 RenderParamsFromPrintSettings(printer_query->settings(), params->params.get()); params->params->document_cookie = printer_query->cookie(); -@@ -172,6 +178,7 @@ void OnDidScriptedPrint( +@@ -172,6 +189,7 @@ void OnDidScriptedPrint( mojom::PrintManagerHost::ScriptedPrintCallback callback) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); mojom::PrintPagesParamsPtr params = CreateEmptyPrintPagesParamsPtr(); @@ -171,7 +186,7 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..560e84fcddf1108114cc863ac0bd9cb7 if (printer_query->last_status() == mojom::ResultCode::kSuccess && printer_query->settings().dpi()) { RenderParamsFromPrintSettings(printer_query->settings(), -@@ -181,7 +188,8 @@ void OnDidScriptedPrint( +@@ -181,7 +199,8 @@ void OnDidScriptedPrint( } bool has_valid_cookie = params->params->document_cookie; bool has_dpi = !params->params->dpi.IsEmpty(); @@ -181,7 +196,7 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..560e84fcddf1108114cc863ac0bd9cb7 if (has_dpi && has_valid_cookie) { queue->QueuePrinterQuery(std::move(printer_query)); -@@ -196,12 +204,14 @@ PrintViewManagerBase::PrintViewManagerBase(content::WebContents* web_contents) +@@ -196,12 +215,14 @@ PrintViewManagerBase::PrintViewManagerBase(content::WebContents* web_contents) : PrintManager(web_contents), queue_(g_browser_process->print_job_manager()->queue()) { DCHECK(queue_); @@ -196,7 +211,7 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..560e84fcddf1108114cc863ac0bd9cb7 } PrintViewManagerBase::~PrintViewManagerBase() { -@@ -209,7 +219,10 @@ PrintViewManagerBase::~PrintViewManagerBase() { +@@ -209,7 +230,10 @@ PrintViewManagerBase::~PrintViewManagerBase() { DisconnectFromCurrentPrintJob(); } @@ -208,7 +223,7 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..560e84fcddf1108114cc863ac0bd9cb7 // Remember the ID for `rfh`, to enable checking that the `RenderFrameHost` // is still valid after a possible inner message loop runs in // `DisconnectFromCurrentPrintJob()`. -@@ -237,6 +250,9 @@ bool PrintViewManagerBase::PrintNow(content::RenderFrameHost* rfh) { +@@ -237,6 +261,9 @@ bool PrintViewManagerBase::PrintNow(content::RenderFrameHost* rfh) { #endif SetPrintingRFH(rfh); @@ -218,7 +233,7 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..560e84fcddf1108114cc863ac0bd9cb7 #if BUILDFLAG(ENABLE_PRINT_CONTENT_ANALYSIS) enterprise_connectors::ContentAnalysisDelegate::Data scanning_data; -@@ -405,7 +421,8 @@ void PrintViewManagerBase::GetDefaultPrintSettingsReply( +@@ -405,7 +432,8 @@ void PrintViewManagerBase::GetDefaultPrintSettingsReply( void PrintViewManagerBase::ScriptedPrintReply( ScriptedPrintCallback callback, int process_id, @@ -228,7 +243,7 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..560e84fcddf1108114cc863ac0bd9cb7 DCHECK_CURRENTLY_ON(content::BrowserThread::UI); #if BUILDFLAG(ENABLE_OOP_PRINTING) -@@ -420,8 +437,11 @@ void PrintViewManagerBase::ScriptedPrintReply( +@@ -420,8 +448,11 @@ void PrintViewManagerBase::ScriptedPrintReply( return; } @@ -241,7 +256,7 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..560e84fcddf1108114cc863ac0bd9cb7 } void PrintViewManagerBase::UpdatePrintingEnabled() { -@@ -429,8 +449,7 @@ void PrintViewManagerBase::UpdatePrintingEnabled() { +@@ -429,8 +460,7 @@ void PrintViewManagerBase::UpdatePrintingEnabled() { // The Unretained() is safe because ForEachRenderFrameHost() is synchronous. web_contents()->GetPrimaryMainFrame()->ForEachRenderFrameHost( base::BindRepeating(&PrintViewManagerBase::SendPrintingEnabled, @@ -251,7 +266,7 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..560e84fcddf1108114cc863ac0bd9cb7 } void PrintViewManagerBase::NavigationStopped() { -@@ -546,11 +565,14 @@ void PrintViewManagerBase::DidPrintDocument( +@@ -546,11 +576,14 @@ void PrintViewManagerBase::DidPrintDocument( void PrintViewManagerBase::GetDefaultPrintSettings( GetDefaultPrintSettingsCallback callback) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); @@ -266,7 +281,7 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..560e84fcddf1108114cc863ac0bd9cb7 #if BUILDFLAG(ENABLE_OOP_PRINTING) if (printing::features::kEnableOopPrintDriversJobPrint.Get() && !service_manager_client_id_.has_value()) { -@@ -588,18 +610,20 @@ void PrintViewManagerBase::UpdatePrintSettings( +@@ -588,18 +621,20 @@ void PrintViewManagerBase::UpdatePrintSettings( base::Value::Dict job_settings, UpdatePrintSettingsCallback callback) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); @@ -288,7 +303,7 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..560e84fcddf1108114cc863ac0bd9cb7 content::BrowserContext* context = web_contents() ? web_contents()->GetBrowserContext() : nullptr; PrefService* prefs = -@@ -609,6 +633,7 @@ void PrintViewManagerBase::UpdatePrintSettings( +@@ -609,6 +644,7 @@ void PrintViewManagerBase::UpdatePrintSettings( if (value > 0) job_settings.Set(kSettingRasterizePdfDpi, value); } @@ -296,7 +311,7 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..560e84fcddf1108114cc863ac0bd9cb7 auto callback_wrapper = base::BindOnce(&PrintViewManagerBase::UpdatePrintSettingsReply, -@@ -640,14 +665,14 @@ void PrintViewManagerBase::ScriptedPrint(mojom::ScriptedPrintParamsPtr params, +@@ -640,14 +676,14 @@ 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); @@ -313,7 +328,7 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..560e84fcddf1108114cc863ac0bd9cb7 return; } #endif -@@ -685,7 +710,6 @@ void PrintViewManagerBase::PrintingFailed(int32_t cookie, +@@ -685,7 +721,6 @@ void PrintViewManagerBase::PrintingFailed(int32_t cookie, PrintManager::PrintingFailed(cookie, reason); #if !BUILDFLAG(IS_ANDROID) // Android does not implement this function. @@ -321,19 +336,19 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..560e84fcddf1108114cc863ac0bd9cb7 #endif ReleasePrinterQuery(); -@@ -700,6 +724,11 @@ void PrintViewManagerBase::RemoveObserver(Observer& observer) { +@@ -700,6 +735,11 @@ void PrintViewManagerBase::RemoveObserver(Observer& observer) { } void PrintViewManagerBase::ShowInvalidPrinterSettingsError() { + if (!callback_.is_null()) { -+ std::string cb_str = "Invalid printer settings"; -+ std::move(callback_).Run(printing_succeeded_, cb_str); ++ printing_status_ = PrintStatus::kInvalid; ++ TerminatePrintJob(true); + } + base::ThreadTaskRunnerHandle::Get()->PostTask( FROM_HERE, base::BindOnce(&ShowWarningMessageBox, l10n_util::GetStringUTF16( -@@ -710,10 +739,12 @@ void PrintViewManagerBase::RenderFrameHostStateChanged( +@@ -710,10 +750,12 @@ void PrintViewManagerBase::RenderFrameHostStateChanged( content::RenderFrameHost* render_frame_host, content::RenderFrameHost::LifecycleState /*old_state*/, content::RenderFrameHost::LifecycleState new_state) { @@ -346,19 +361,30 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..560e84fcddf1108114cc863ac0bd9cb7 } void PrintViewManagerBase::DidStartLoading() { -@@ -773,6 +804,11 @@ void PrintViewManagerBase::OnJobDone() { - ReleasePrintJob(); - } - -+void PrintViewManagerBase::UserInitCanceled() { -+ printing_canceled_ = true; +@@ -769,7 +811,12 @@ void PrintViewManagerBase::OnJobDone() { + // Printing is done, we don't need it anymore. + // print_job_->is_job_pending() may still be true, depending on the order + // of object registration. +- printing_succeeded_ = true; ++ printing_status_ = PrintStatus::kSucceeded; + ReleasePrintJob(); +} + - void PrintViewManagerBase::OnFailed() { - TerminatePrintJob(true); ++void PrintViewManagerBase::UserInitCanceled() { ++ printing_status_ = PrintStatus::kCanceled; + ReleasePrintJob(); } -@@ -831,7 +867,10 @@ bool PrintViewManagerBase::CreateNewPrintJob( + +@@ -783,7 +830,7 @@ bool PrintViewManagerBase::RenderAllMissingPagesNow() { + + // Is the document already complete? + if (print_job_->document() && print_job_->document()->IsComplete()) { +- printing_succeeded_ = true; ++ printing_status_ = PrintStatus::kSucceeded; + return true; + } + +@@ -831,7 +878,10 @@ bool PrintViewManagerBase::CreateNewPrintJob( // Disconnect the current `print_job_`. auto weak_this = weak_ptr_factory_.GetWeakPtr(); @@ -370,21 +396,37 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..560e84fcddf1108114cc863ac0bd9cb7 if (!weak_this) return false; -@@ -912,6 +951,13 @@ void PrintViewManagerBase::ReleasePrintJob() { +@@ -852,7 +902,7 @@ bool PrintViewManagerBase::CreateNewPrintJob( + #endif + print_job_->AddObserver(*this); + +- printing_succeeded_ = false; ++ printing_status_ = PrintStatus::kFailed; + return true; + } + +@@ -912,6 +962,11 @@ void PrintViewManagerBase::ReleasePrintJob() { } #endif + if (!callback_.is_null()) { -+ std::string cb_str = ""; -+ if (!printing_succeeded_) -+ cb_str = printing_canceled_ ? "canceled" : "failed"; -+ std::move(callback_).Run(printing_succeeded_, cb_str); ++ bool success = printing_status_ == PrintStatus::kSucceeded; ++ std::move(callback_).Run(success, PrintReasonFromPrintStatus(printing_status_)); + } + if (!print_job_) return; -@@ -961,7 +1007,7 @@ bool PrintViewManagerBase::RunInnerMessageLoop() { +@@ -919,7 +974,7 @@ void PrintViewManagerBase::ReleasePrintJob() { + // printing_rfh_ should only ever point to a RenderFrameHost with a live + // RenderFrame. + DCHECK(rfh->IsRenderFrameLive()); +- GetPrintRenderFrame(rfh)->PrintingDone(printing_succeeded_); ++ GetPrintRenderFrame(rfh)->PrintingDone(printing_status_ == PrintStatus::kSucceeded); + } + + print_job_->RemoveObserver(*this); +@@ -961,7 +1016,7 @@ bool PrintViewManagerBase::RunInnerMessageLoop() { } bool PrintViewManagerBase::OpportunisticallyCreatePrintJob(int cookie) { @@ -393,7 +435,7 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..560e84fcddf1108114cc863ac0bd9cb7 return true; if (!cookie) { -@@ -1069,7 +1115,7 @@ void PrintViewManagerBase::SendPrintingEnabled(bool enabled, +@@ -1069,7 +1124,7 @@ void PrintViewManagerBase::SendPrintingEnabled(bool enabled, } void PrintViewManagerBase::CompletePrintNow(content::RenderFrameHost* rfh) { @@ -403,7 +445,7 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..560e84fcddf1108114cc863ac0bd9cb7 for (auto& observer : GetObservers()) observer.OnPrintNow(rfh); diff --git a/chrome/browser/printing/print_view_manager_base.h b/chrome/browser/printing/print_view_manager_base.h -index 746df417a23f7760818ba265d4a7d589dec8bf34..0027387d4717c59f2df3f279caf7aa0de6669400 100644 +index 746df417a23f7760818ba265d4a7d589dec8bf34..0d3e4491826be629c7251a69afc7ebde990e10e1 100644 --- a/chrome/browser/printing/print_view_manager_base.h +++ b/chrome/browser/printing/print_view_manager_base.h @@ -41,6 +41,8 @@ namespace printing { @@ -435,7 +477,22 @@ index 746df417a23f7760818ba265d4a7d589dec8bf34..0027387d4717c59f2df3f279caf7aa0d // Adds and removes observers for `PrintViewManagerBase` events. The order in // which notifications are sent to observers is undefined. Observers must be -@@ -241,7 +247,8 @@ class PrintViewManagerBase : public PrintManager, public PrintJob::Observer { +@@ -120,6 +126,14 @@ class PrintViewManagerBase : public PrintManager, public PrintJob::Observer { + void AddObserver(Observer& observer); + void RemoveObserver(Observer& observer); + ++ enum class PrintStatus { ++ kSucceeded, ++ kCanceled, ++ kFailed, ++ kInvalid, ++ kUnknown ++ }; ++ + protected: + explicit PrintViewManagerBase(content::WebContents* web_contents); + +@@ -241,7 +255,8 @@ class PrintViewManagerBase : public PrintManager, public PrintJob::Observer { // Runs `callback` with `params` to reply to ScriptedPrint(). void ScriptedPrintReply(ScriptedPrintCallback callback, int process_id, @@ -445,7 +502,7 @@ index 746df417a23f7760818ba265d4a7d589dec8bf34..0027387d4717c59f2df3f279caf7aa0d // 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 -@@ -314,9 +321,15 @@ class PrintViewManagerBase : public PrintManager, public PrintJob::Observer { +@@ -314,8 +329,11 @@ class PrintViewManagerBase : public PrintManager, public PrintJob::Observer { // The current RFH that is printing with a system printing dialog. raw_ptr printing_rfh_ = nullptr; @@ -453,14 +510,11 @@ index 746df417a23f7760818ba265d4a7d589dec8bf34..0027387d4717c59f2df3f279caf7aa0d + CompletionCallback callback_; + // Indication of success of the print job. - bool printing_succeeded_ = false; +- bool printing_succeeded_ = false; ++ PrintStatus printing_status_ = PrintStatus::kUnknown; -+ // 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/chrome/browser/ui/webui/print_preview/fake_print_render_frame.cc b/chrome/browser/ui/webui/print_preview/fake_print_render_frame.cc index b2bd74f28f70bc601ec47820030ad965b19cf068..027e4c0b78d44b69504d248755bf7f25ff423361 100644 --- a/chrome/browser/ui/webui/print_preview/fake_print_render_frame.cc