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: printing optional access crash on Windows #38976

Merged
merged 1 commit into from
Jul 10, 2023
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
44 changes: 25 additions & 19 deletions patches/chromium/printing.patch
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@

using RenderMode = PdfRenderSettings::Mode;
RenderMode mode = print_with_reduced_rasterization
@@ -465,8 +467,10 @@ void PrintJob::StartPdfToPostScriptConversion(

Check failure on line 81 in patches/chromium/printing.patch

View check run for this annotation

trop / Backportable? - 25-x-y

patches/chromium/printing.patch#L81

Patch Conflict
Raw output
++<<<<<<< HEAD
 +index 6756967e1df3d985eab45aaaefcf5dd2f7025b9a..fbee0055b80b9c53e9d42245aa704b3a35717553 100644
++=======
+ index c976fb2a814e4ff09650982034371e40d1ab77bc..3ef781934426efd662db583492ffdef43b693b0c 100644
++>>>>>>> fix: printing optional access crash on Windows
if (ps_level2) {
mode = PdfRenderSettings::Mode::POSTSCRIPT_LEVEL2;
} else {
Expand All @@ -91,7 +91,7 @@
: PdfRenderSettings::Mode::POSTSCRIPT_LEVEL3;
}
diff --git a/chrome/browser/printing/print_view_manager_base.cc b/chrome/browser/printing/print_view_manager_base.cc
index c976fb2a814e4ff09650982034371e40d1ab77bc..c9115c7bbead588f3099bb194eb293b0e78ad211 100644
index c976fb2a814e4ff09650982034371e40d1ab77bc..3ef781934426efd662db583492ffdef43b693b0c 100644
--- a/chrome/browser/printing/print_view_manager_base.cc
+++ b/chrome/browser/printing/print_view_manager_base.cc
@@ -23,7 +23,9 @@
Expand Down Expand Up @@ -270,7 +270,7 @@
set_cookie(0);
- std::move(callback).Run(nullptr);
+ std::move(callback).Run(nullptr, false);
}

Check failure on line 273 in patches/chromium/printing.patch

View check run for this annotation

trop / Backportable? - 25-x-y

patches/chromium/printing.patch#L273

Patch Conflict
Raw output
++<<<<<<< HEAD
 +@@ -725,7 +763,7 @@ void PrintViewManagerBase::UpdatePrintSettings(
++=======
+ @@ -704,7 +751,21 @@ void PrintViewManagerBase::UpdatePrintSettings(
+      }
+    }
+  
+ -#if BUILDFLAG(IS_WIN)
+ +  std::unique_ptr<PrinterQuery> query =
+ +      queue_->CreatePrinterQuery(GetCurrentTargetFrame()->GetGlobalId());
+ +  auto* query_ptr = query.get();
+ +  // We need to clone this before calling SetSettings because some environments
+ +  // evaluate job_settings.Clone() first, and some std::move(job_settings) first,
+ +  // for the former things work correctly but for the latter the cloned value is null.
+ +  auto job_settings_copy = job_settings.Clone();
+ +  query_ptr->SetSettings(
+ +      std::move(job_settings_copy),
+ +      base::BindOnce(&PrintViewManagerBase::CompleteUpdatePrintSettings,
+ +                     weak_ptr_factory_.GetWeakPtr(), std::move(query),
+ +                     std::move(job_settings), std::move(print_settings),
+ +                     std::move(callback)));
+ +
+ +#if 0 // See https://chromium-review.googlesource.com/412367
+    // TODO(crbug.com/1424368):  Remove this if the printable areas can be made
+    // fully available from `PrintBackend::GetPrinterSemanticCapsAndDefaults()`
+    // for in-browser queries.
+ @@ -726,15 +787,13 @@ void PrintViewManagerBase::UpdatePrintSettings(
+    }
+  #endif
+  
+ -  CompleteUpdatePrintSettings(std::move(job_settings),
+ -                              std::move(print_settings), std::move(callback));
+  }
+  #endif  // BUILDFLAG(ENABLE_PRINT_PREVIEW)
+  
++>>>>>>> fix: printing optional access crash on Windows
}

@@ -598,10 +639,12 @@ void PrintViewManagerBase::DidPrintDocument(
Expand Down Expand Up @@ -314,25 +314,31 @@
+#endif // Printing is always enabled.

std::unique_ptr<PrintSettings> print_settings =
PrintSettingsFromJobSettings(job_settings);

Check failure on line 317 in patches/chromium/printing.patch

View check run for this annotation

trop / Backportable? - 25-x-y

patches/chromium/printing.patch#L317

Patch Conflict
Raw output
++<<<<<<< HEAD
 +@@ -741,14 +779,14 @@ void PrintViewManagerBase::ScriptedPrint(mojom::ScriptedPrintParamsPtr params,
++=======
+ @@ -750,14 +809,14 @@ void PrintViewManagerBase::ScriptedPrint(mojom::ScriptedPrintParamsPtr params,
++>>>>>>> fix: printing optional access crash on Windows
@@ -704,6 +751,16 @@ void PrintViewManagerBase::UpdatePrintSettings(
@@ -704,7 +751,21 @@ void PrintViewManagerBase::UpdatePrintSettings(
}
}

-#if BUILDFLAG(IS_WIN)
+ std::unique_ptr<PrinterQuery> query =
+ queue_->CreatePrinterQuery(GetCurrentTargetFrame()->GetGlobalId());
+ auto* query_ptr = query.get();
+ // We need to clone this before calling SetSettings because some environments
+ // evaluate job_settings.Clone() first, and some std::move(job_settings) first,
+ // for the former things work correctly but for the latter the cloned value is null.
+ auto job_settings_copy = job_settings.Clone();
+ query_ptr->SetSettings(
+ job_settings.Clone(),
+ std::move(job_settings_copy),
codebytere marked this conversation as resolved.
Show resolved Hide resolved
+ base::BindOnce(&PrintViewManagerBase::CompleteUpdatePrintSettings,
+ weak_ptr_factory_.GetWeakPtr(), std::move(query),
+ std::move(job_settings), std::move(print_settings),
+ std::move(callback)));
+
#if BUILDFLAG(IS_WIN)
+#if 0 // See https://chromium-review.googlesource.com/412367
// TODO(crbug.com/1424368): Remove this if the printable areas can be made

Check failure on line 338 in patches/chromium/printing.patch

View check run for this annotation

trop / Backportable? - 25-x-y

patches/chromium/printing.patch#L338

Patch Conflict
Raw output
++<<<<<<< HEAD
 +@@ -786,6 +824,7 @@ void PrintViewManagerBase::PrintingFailed(int32_t cookie,
++=======
+ @@ -792,6 +851,7 @@ void PrintViewManagerBase::PrintingFailed(int32_t cookie,
++>>>>>>> fix: printing optional access crash on Windows
// fully available from `PrintBackend::GetPrinterSemanticCapsAndDefaults()`
@@ -726,15 +783,13 @@ void PrintViewManagerBase::UpdatePrintSettings(
// for in-browser queries.
@@ -726,15 +787,13 @@ void PrintViewManagerBase::UpdatePrintSettings(
}
#endif

Expand All @@ -341,7 +347,7 @@
}
#endif // BUILDFLAG(ENABLE_PRINT_PREVIEW)

void PrintViewManagerBase::IsPrintingEnabled(

Check failure on line 350 in patches/chromium/printing.patch

View check run for this annotation

trop / Backportable? - 25-x-y

patches/chromium/printing.patch#L350

Patch Conflict
Raw output
++<<<<<<< HEAD
 +@@ -795,7 +834,7 @@ void PrintViewManagerBase::PrintingFailed(int32_t cookie,
++=======
+ @@ -801,7 +861,7 @@ void PrintViewManagerBase::PrintingFailed(int32_t cookie,
++>>>>>>> fix: printing optional access crash on Windows
IsPrintingEnabledCallback callback) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
- std::move(callback).Run(printing_enabled_.GetValue());
Expand All @@ -349,13 +355,13 @@
}

void PrintViewManagerBase::ScriptedPrint(mojom::ScriptedPrintParamsPtr params,
@@ -750,14 +805,14 @@ void PrintViewManagerBase::ScriptedPrint(mojom::ScriptedPrintParamsPtr params,
@@ -750,14 +809,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);
- std::move(callback).Run(nullptr);
+ std::move(callback).Run(nullptr, false);
return;

Check failure on line 364 in patches/chromium/printing.patch

View check run for this annotation

trop / Backportable? - 25-x-y

patches/chromium/printing.patch#L363-L364

Patch Conflict
Raw output
++<<<<<<< HEAD
 +@@ -807,15 +846,24 @@ void PrintViewManagerBase::RemoveObserver(Observer& observer) {
 +   observers_.RemoveObserver(&observer);
++=======
+ @@ -813,15 +873,24 @@ void PrintViewManagerBase::RemoveTestObserver(TestObserver& observer) {
+    test_observers_.RemoveObserver(&observer);
++>>>>>>> fix: printing optional access crash on Windows
}
#if BUILDFLAG(ENABLE_OOP_PRINTING)
if (printing::features::kEnableOopPrintDriversJobPrint.Get() &&
Expand All @@ -366,15 +372,15 @@
return;
}
#endif
@@ -792,6 +847,7 @@ void PrintViewManagerBase::PrintingFailed(int32_t cookie,
@@ -792,6 +851,7 @@ void PrintViewManagerBase::PrintingFailed(int32_t cookie,

PrintManager::PrintingFailed(cookie, reason);

+#if 0 // Electron does not use Chromium error dialogs
// `PrintingFailed()` can occur because asynchronous compositing results
// don't complete until after a print job has already failed and been
// destroyed. In such cases the error notification to the user will
@@ -801,7 +857,7 @@ void PrintViewManagerBase::PrintingFailed(int32_t cookie,
@@ -801,7 +861,7 @@ void PrintViewManagerBase::PrintingFailed(int32_t cookie,
print_job_->document()->cookie() == cookie) {
ShowPrintErrorDialogForGenericError();
}
Expand All @@ -383,8 +389,8 @@
ReleasePrinterQuery();
}

@@ -813,15 +869,24 @@ void PrintViewManagerBase::RemoveTestObserver(TestObserver& observer) {
@@ -813,15 +873,24 @@ void PrintViewManagerBase::RemoveTestObserver(TestObserver& observer) {
test_observers_.RemoveObserver(&observer);

Check failure on line 393 in patches/chromium/printing.patch

View check run for this annotation

trop / Backportable? - 25-x-y

patches/chromium/printing.patch#L393

Patch Conflict
Raw output
++<<<<<<< HEAD
 +@@ -867,7 +915,12 @@ void PrintViewManagerBase::OnJobDone() {
++=======
+ @@ -873,7 +942,12 @@ void PrintViewManagerBase::OnJobDone() {
++>>>>>>> fix: printing optional access crash on Windows
}

+void PrintViewManagerBase::ShowInvalidPrinterSettingsError() {
Expand All @@ -402,13 +408,13 @@
if (new_state == content::RenderFrameHost::LifecycleState::kActive &&
render_frame_host->GetProcess()->IsPdf() &&
!render_frame_host->GetMainFrame()->GetParentOrOuterDocument()) {
GetPrintRenderFrame(render_frame_host)->ConnectToPdfRenderer();

Check failure on line 411 in patches/chromium/printing.patch

View check run for this annotation

trop / Backportable? - 25-x-y

patches/chromium/printing.patch#L411

Patch Conflict
Raw output
++<<<<<<< HEAD
 +@@ -876,9 +929,10 @@ void PrintViewManagerBase::OnCanceling() {
++=======
+ @@ -882,9 +956,10 @@ void PrintViewManagerBase::OnCanceling() {
++>>>>>>> fix: printing optional access crash on Windows
}
+#endif
}

void PrintViewManagerBase::RenderFrameDeleted(
@@ -873,7 +938,12 @@ void PrintViewManagerBase::OnJobDone() {
@@ -873,7 +942,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.
Expand All @@ -418,11 +424,11 @@
+}
+
+void PrintViewManagerBase::UserInitCanceled() {
+ printing_status_ = PrintStatus::kCanceled;

Check failure on line 427 in patches/chromium/printing.patch

View check run for this annotation

trop / Backportable? - 25-x-y

patches/chromium/printing.patch#L427

Patch Conflict
Raw output
++<<<<<<< HEAD
 +@@ -888,7 +942,7 @@ bool PrintViewManagerBase::RenderAllMissingPagesNow() {
++=======
+ @@ -894,7 +969,7 @@ bool PrintViewManagerBase::RenderAllMissingPagesNow() {
++>>>>>>> fix: printing optional access crash on Windows
ReleasePrintJob();
}

@@ -882,9 +952,10 @@ void PrintViewManagerBase::OnCanceling() {
@@ -882,9 +956,10 @@ void PrintViewManagerBase::OnCanceling() {
}

void PrintViewManagerBase::OnFailed() {
Expand All @@ -431,10 +437,10 @@
ShowPrintErrorDialogForGenericError();
-
+#endif
TerminatePrintJob(true);

Check failure on line 440 in patches/chromium/printing.patch

View check run for this annotation

trop / Backportable? - 25-x-y

patches/chromium/printing.patch#L440

Patch Conflict
Raw output
++<<<<<<< HEAD
 +@@ -936,7 +990,10 @@ bool PrintViewManagerBase::CreateNewPrintJob(
++=======
+ @@ -942,7 +1017,10 @@ bool PrintViewManagerBase::CreateNewPrintJob(
++>>>>>>> fix: printing optional access crash on Windows
}

@@ -894,7 +965,7 @@ bool PrintViewManagerBase::RenderAllMissingPagesNow() {
@@ -894,7 +969,7 @@ bool PrintViewManagerBase::RenderAllMissingPagesNow() {

// Is the document already complete?
if (print_job_->document() && print_job_->document()->IsComplete()) {
Expand All @@ -443,11 +449,11 @@
return true;
}

@@ -942,7 +1013,10 @@ bool PrintViewManagerBase::CreateNewPrintJob(
@@ -942,7 +1017,10 @@ bool PrintViewManagerBase::CreateNewPrintJob(

// Disconnect the current `print_job_`.
auto weak_this = weak_ptr_factory_.GetWeakPtr();
- DisconnectFromCurrentPrintJob();

Check failure on line 456 in patches/chromium/printing.patch

View check run for this annotation

trop / Backportable? - 25-x-y

patches/chromium/printing.patch#L456

Patch Conflict
Raw output
++<<<<<<< HEAD
 +@@ -957,7 +1014,7 @@ bool PrintViewManagerBase::CreateNewPrintJob(
++=======
+ @@ -963,7 +1041,7 @@ bool PrintViewManagerBase::CreateNewPrintJob(
++>>>>>>> fix: printing optional access crash on Windows
+ if (callback_.is_null()) {
+ // Disconnect the current |print_job_| only when calling window.print()
+ DisconnectFromCurrentPrintJob();
Expand All @@ -455,16 +461,16 @@
if (!weak_this)
return false;

@@ -963,7 +1037,7 @@ bool PrintViewManagerBase::CreateNewPrintJob(
@@ -963,7 +1041,7 @@ bool PrintViewManagerBase::CreateNewPrintJob(
#endif
print_job_->AddObserver(*this);

- printing_succeeded_ = false;
+ printing_status_ = PrintStatus::kFailed;

Check failure on line 469 in patches/chromium/printing.patch

View check run for this annotation

trop / Backportable? - 25-x-y

patches/chromium/printing.patch#L469

Patch Conflict
Raw output
++<<<<<<< HEAD
 +@@ -1019,6 +1076,11 @@ void PrintViewManagerBase::ReleasePrintJob() {
++=======
+ @@ -1031,6 +1109,11 @@ void PrintViewManagerBase::ReleasePrintJob() {
++>>>>>>> fix: printing optional access crash on Windows
return true;
}

@@ -1031,6 +1105,11 @@ void PrintViewManagerBase::ReleasePrintJob() {
@@ -1031,6 +1109,11 @@ void PrintViewManagerBase::ReleasePrintJob() {
}
#endif

Expand All @@ -476,7 +482,7 @@
if (!print_job_)
return;

@@ -1038,7 +1117,7 @@ void PrintViewManagerBase::ReleasePrintJob() {
@@ -1038,7 +1121,7 @@ void PrintViewManagerBase::ReleasePrintJob() {

Check failure on line 485 in patches/chromium/printing.patch

View check run for this annotation

trop / Backportable? - 25-x-y

patches/chromium/printing.patch#L485

Patch Conflict
Raw output
++<<<<<<< HEAD
 +@@ -1026,7 +1088,7 @@ void PrintViewManagerBase::ReleasePrintJob() {
++=======
+ @@ -1038,7 +1121,7 @@ void PrintViewManagerBase::ReleasePrintJob() {
++>>>>>>> fix: printing optional access crash on Windows
// printing_rfh_ should only ever point to a RenderFrameHost with a live
// RenderFrame.
DCHECK(rfh->IsRenderFrameLive());
Expand All @@ -485,16 +491,16 @@
}

print_job_->RemoveObserver(*this);
@@ -1080,7 +1159,7 @@ bool PrintViewManagerBase::RunInnerMessageLoop() {
@@ -1080,7 +1163,7 @@ bool PrintViewManagerBase::RunInnerMessageLoop() {
}

bool PrintViewManagerBase::OpportunisticallyCreatePrintJob(int cookie) {
- if (print_job_)

Check failure on line 498 in patches/chromium/printing.patch

View check run for this annotation

trop / Backportable? - 25-x-y

patches/chromium/printing.patch#L498

Patch Conflict
Raw output
++<<<<<<< HEAD
 +@@ -1068,7 +1130,7 @@ bool PrintViewManagerBase::RunInnerMessageLoop() {
++=======
+ @@ -1080,7 +1163,7 @@ bool PrintViewManagerBase::RunInnerMessageLoop() {
++>>>>>>> fix: printing optional access crash on Windows
+ if (print_job_ && print_job_->document())
return true;

if (!cookie) {
@@ -1224,7 +1303,7 @@ void PrintViewManagerBase::ReleasePrinterQuery() {
@@ -1224,7 +1307,7 @@ void PrintViewManagerBase::ReleasePrinterQuery() {
}

void PrintViewManagerBase::CompletePrintNow(content::RenderFrameHost* rfh) {
Expand All @@ -502,8 +508,8 @@
+ GetPrintRenderFrame(rfh)->PrintRequestedPages(/*silent=*/true, /*job_settings=*/base::Value::Dict());

for (auto& observer : GetTestObservers()) {
observer.OnPrintNow(rfh);

Check failure on line 511 in patches/chromium/printing.patch

View check run for this annotation

trop / Backportable? - 25-x-y

patches/chromium/printing.patch#L511

Patch Conflict
Raw output
++<<<<<<< HEAD
 +@@ -1176,7 +1238,7 @@ void PrintViewManagerBase::ReleasePrinterQuery() {
++=======
+ @@ -1224,7 +1307,7 @@ void PrintViewManagerBase::ReleasePrinterQuery() {
++>>>>>>> fix: printing optional access crash on Windows
@@ -1274,7 +1353,7 @@ void PrintViewManagerBase::CompleteScriptedPrintAfterContentAnalysis(
@@ -1274,7 +1357,7 @@ void PrintViewManagerBase::CompleteScriptedPrintAfterContentAnalysis(
set_analyzing_content(/*analyzing*/ false);
if (!allowed || !printing_rfh_ || IsCrashed() ||
!printing_rfh_->IsRenderFrameLive()) {
Expand Down