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.

Sign up for GitHub

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

Jump to bottom

fix: crash on printer dialog cancellation #32632

Merged
merged 3 commits into from Feb 1, 2022
Merged

fix: crash on printer dialog cancellation #32632

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0fe7d3a
fix: crash on printer dialog cancellation
codebytere Jan 26, 2022
14a3b28
fix: remove commented out code
codebytere Jan 27, 2022
7b809d0
chore: address review
codebytere Feb 1, 2022
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
209 changes: 93 additions & 116 deletions patches/chromium/printing.patch
View file
Open in desktop
Expand Up @@ -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) {
Expand Down Expand Up @@ -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>(JobEventDetails::USER_INIT_CANCELED,
+ 0, nullptr);
+ content::NotificationService::current()->Notify(
+ chrome::NOTIFICATION_PRINT_JOB_EVENT, content::Source<PrintJob>(this),
+ content::Details<JobEventDetails>(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<PrintJob> {
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<PrintJob> {
// 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<JobEventDetails> {
@@ -257,6 +257,9 @@ class JobEventDetails : public base::RefCountedThreadSafe<JobEventDetails> {
public:
// Event type.
enum Type {
Expand All @@ -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 @@
Expand All @@ -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)
}

Expand All @@ -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..e0017f2998009fcbb5086481b8bc2f23e6c4080e 100644
--- a/chrome/browser/printing/print_view_manager_base.cc
+++ b/chrome/browser/printing/print_view_manager_base.cc
@@ -29,10 +29,10 @@
Expand Down Expand Up @@ -217,7 +154,19 @@ index c9f1502da55d89de0eede73a7d39047c090594d0..1320afa83016ea72e5dbc23b1ed63cf9
}

#if BUILDFLAG(ENABLE_PRINT_PREVIEW)
@@ -217,7 +221,9 @@ void UpdatePrintSettingsReplyOnIO(
@@ -209,6 +213,11 @@ void NotifySystemDialogCancelled(base::WeakPtr<PrintViewManagerBase> manager) {
}
#endif // defined(OS_WIN)

+void NotifyUserCancelled(base::WeakPtr<PrintViewManagerBase> manager) {
+ if (manager)
+ manager->UserInitCancelled();
+}
+
void UpdatePrintSettingsReplyOnIO(
scoped_refptr<PrintQueriesQueue> queue,
std::unique_ptr<PrinterQuery> printer_query,
@@ -217,7 +226,9 @@ void UpdatePrintSettingsReplyOnIO(
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
DCHECK(printer_query);
mojom::PrintPagesParamsPtr params = CreateEmptyPrintPagesParamsPtr();
Expand All @@ -228,7 +177,45 @@ 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)
@@ -267,9 +278,17 @@ void UpdatePrintSettingsOnIO(
void ScriptedPrintReplyOnIO(
scoped_refptr<PrintQueriesQueue> queue,
std::unique_ptr<PrinterQuery> printer_query,
- mojom::PrintManagerHost::ScriptedPrintCallback callback) {
+ mojom::PrintManagerHost::ScriptedPrintCallback callback,
+ base::WeakPtr<PrintViewManagerBase> manager) {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
mojom::PrintPagesParamsPtr params = CreateEmptyPrintPagesParamsPtr();
+
+ if (printer_query->last_status() == mojom::ResultCode::kCanceled) {
+ content::GetUIThreadTaskRunner({})->PostTask(
+ FROM_HERE,
+ base::BindOnce(&NotifyUserCancelled, std::move(manager)));
+ }
+
if (printer_query->last_status() == mojom::ResultCode::kSuccess &&
printer_query->settings().dpi()) {
RenderParamsFromPrintSettings(printer_query->settings(),
@@ -293,7 +312,8 @@ void ScriptedPrintOnIO(mojom::ScriptedPrintParamsPtr params,
mojom::PrintManagerHost::ScriptedPrintCallback callback,
scoped_refptr<PrintQueriesQueue> queue,
int process_id,
- int routing_id) {
+ int routing_id,
+ base::WeakPtr<PrintViewManagerBase> manager) {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
#if defined(OS_WIN) && BUILDFLAG(GOOGLE_CHROME_BRANDING)
ModuleDatabase::GetInstance()->DisableThirdPartyBlocking();
@@ -310,7 +330,7 @@ void ScriptedPrintOnIO(mojom::ScriptedPrintParamsPtr params,
params->has_selection, params->margin_type, params->is_scripted,
params->is_modifiable,
base::BindOnce(&ScriptedPrintReplyOnIO, queue, std::move(printer_query),
- std::move(callback)));
+ std::move(callback), std::move(manager)));
}

} // namespace
@@ -319,12 +339,14 @@ PrintViewManagerBase::PrintViewManagerBase(content::WebContents* web_contents)
: PrintManager(web_contents),
queue_(g_browser_process->print_job_manager()->queue()) {
DCHECK(queue_);
Expand All @@ -243,7 +230,7 @@ index c9f1502da55d89de0eede73a7d39047c090594d0..1320afa83016ea72e5dbc23b1ed63cf9
}

PrintViewManagerBase::~PrintViewManagerBase() {
@@ -332,7 +340,10 @@ PrintViewManagerBase::~PrintViewManagerBase() {
@@ -332,7 +354,10 @@ PrintViewManagerBase::~PrintViewManagerBase() {
DisconnectFromCurrentPrintJob();
}

Expand All @@ -255,22 +242,22 @@ 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 +372,13 @@ 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);
+ }
+ // 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(
@@ -506,9 +537,9 @@ void PrintViewManagerBase::ScriptedPrintReply(
void PrintViewManagerBase::UpdatePrintingEnabled() {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
// The Unretained() is safe because ForEachFrame() is synchronous.
Expand All @@ -283,7 +270,7 @@ index c9f1502da55d89de0eede73a7d39047c090594d0..1320afa83016ea72e5dbc23b1ed63cf9
}

void PrintViewManagerBase::NavigationStopped() {
@@ -622,12 +639,13 @@ void PrintViewManagerBase::DidPrintDocument(
@@ -622,12 +653,13 @@ void PrintViewManagerBase::DidPrintDocument(
void PrintViewManagerBase::GetDefaultPrintSettings(
GetDefaultPrintSettingsCallback callback) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
Expand All @@ -298,7 +285,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 +677,20 @@ void PrintViewManagerBase::UpdatePrintSettings(
base::Value job_settings,
UpdatePrintSettingsCallback callback) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
Expand All @@ -320,23 +307,32 @@ index c9f1502da55d89de0eede73a7d39047c090594d0..1320afa83016ea72e5dbc23b1ed63cf9
content::BrowserContext* context =
web_contents() ? web_contents()->GetBrowserContext() : nullptr;
PrefService* prefs =
@@ -666,6 +686,7 @@ void PrintViewManagerBase::UpdatePrintSettings(
@@ -666,6 +700,7 @@ void PrintViewManagerBase::UpdatePrintSettings(
if (value > 0)
job_settings.SetIntKey(kSettingRasterizePdfDpi, value);
}
+#endif

auto callback_wrapper =
base::BindOnce(&PrintViewManagerBase::UpdatePrintSettingsReply,
@@ -714,7 +735,6 @@ void PrintViewManagerBase::PrintingFailed(int32_t cookie) {
@@ -702,7 +737,7 @@ void PrintViewManagerBase::ScriptedPrint(mojom::ScriptedPrintParamsPtr params,
content::GetIOThreadTaskRunner({})->PostTask(
FROM_HERE, base::BindOnce(&ScriptedPrintOnIO, std::move(params),
std::move(callback_wrapper), queue_, process_id,
- routing_id));
+ routing_id, weak_ptr_factory_.GetWeakPtr()));
}

void PrintViewManagerBase::PrintingFailed(int32_t cookie) {
@@ -714,7 +749,6 @@ void PrintViewManagerBase::PrintingFailed(int32_t cookie) {
PrintManager::PrintingFailed(cookie);

#if BUILDFLAG(ENABLE_PRINT_PREVIEW)
- ShowPrintErrorDialog();
#endif

ReleasePrinterQuery();
@@ -729,6 +749,11 @@ void PrintViewManagerBase::RemoveObserver(Observer& observer) {
@@ -729,6 +763,11 @@ void PrintViewManagerBase::RemoveObserver(Observer& observer) {
}

void PrintViewManagerBase::ShowInvalidPrinterSettingsError() {
Expand All @@ -348,7 +344,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 +778,10 @@ void PrintViewManagerBase::RenderFrameHostStateChanged(
content::RenderFrameHost* render_frame_host,
content::RenderFrameHost::LifecycleState /*old_state*/,
content::RenderFrameHost::LifecycleState new_state) {
Expand All @@ -359,19 +355,19 @@ index c9f1502da55d89de0eede73a7d39047c090594d0..1320afa83016ea72e5dbc23b1ed63cf9
}

void PrintViewManagerBase::DidStartLoading() {
@@ -808,6 +835,11 @@ void PrintViewManagerBase::OnJobDone() {
@@ -808,6 +849,11 @@ void PrintViewManagerBase::OnJobDone() {
ReleasePrintJob();
}

+void PrintViewManagerBase::OnUserInitCancelled() {
+void PrintViewManagerBase::UserInitCancelled() {
+ printing_cancelled_ = true;
+ ReleasePrintJob();
+}
+
void PrintViewManagerBase::OnFailed() {
TerminatePrintJob(true);
}
@@ -869,7 +901,10 @@ bool PrintViewManagerBase::CreateNewPrintJob(
@@ -869,7 +915,10 @@ bool PrintViewManagerBase::CreateNewPrintJob(

// Disconnect the current |print_job_|.
auto weak_this = weak_ptr_factory_.GetWeakPtr();
Expand All @@ -383,22 +379,11 @@ 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 +993,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";
Expand All @@ -408,15 +393,7 @@ index c9f1502da55d89de0eede73a7d39047c090594d0..1320afa83016ea72e5dbc23b1ed63cf9
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 +1045,7 @@ bool PrintViewManagerBase::RunInnerMessageLoop() {
}

bool PrintViewManagerBase::OpportunisticallyCreatePrintJob(int cookie) {
Expand All @@ -426,7 +403,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..db7f2b8bf206849426fb2e050fac387a51a30dad 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 {
Expand All @@ -450,14 +427,14 @@ 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;

base::ObserverList<Observer>& GetObservers() { return observers_; }
@@ -106,6 +111,7 @@ class PrintViewManagerBase : public PrintManager, public PrintJob::Observer {
ScriptedPrintCallback callback) override;
void ShowInvalidPrinterSettingsError() override;
void PrintingFailed(int32_t cookie) override;
+ void UserInitCancelled();

// Adds and removes observers for `PrintViewManagerBase` events. The order in
// which notifications are sent to observers is undefined. Observers must be
@@ -252,9 +258,15 @@ 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;
Expand Down