Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: clean up callback handling in webContents.print() #35141

Merged
merged 1 commit into from Aug 1, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
144 changes: 99 additions & 45 deletions patches/chromium/printing.patch
Expand Up @@ -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 @@
Expand All @@ -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(std::unique_ptr<PrinterQuery>)>;

void ShowWarningMessageBox(const std::u16string& message) {
Expand All @@ -144,15 +159,15 @@ 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<bool> auto_reset(&is_dialog_shown, true);

chrome::ShowWarningMessageBox(nullptr, std::u16string(), message);
+#endif
}

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();
Expand All @@ -163,15 +178,15 @@ 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();
+
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();
Expand All @@ -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_);
Expand All @@ -196,7 +211,7 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..560e84fcddf1108114cc863ac0bd9cb7
}

PrintViewManagerBase::~PrintViewManagerBase() {
@@ -209,7 +219,10 @@ PrintViewManagerBase::~PrintViewManagerBase() {
@@ -209,7 +230,10 @@ PrintViewManagerBase::~PrintViewManagerBase() {
DisconnectFromCurrentPrintJob();
}

Expand All @@ -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);
Expand All @@ -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,
Expand All @@ -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;
}

Expand All @@ -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,
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -288,15 +303,15 @@ 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);
}
+#endif

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);
Expand All @@ -313,27 +328,27 @@ 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.
- ShowPrintErrorDialog();
#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) {
Expand All @@ -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();
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
Expand All @@ -445,22 +502,19 @@ 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<content::RenderFrameHost> printing_rfh_ = nullptr;

+ // Respond with success of the print job.
+ 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
Expand Down