diff --git a/patches/chromium/.patches b/patches/chromium/.patches index c2f0b55b8b8c2..68e2c048aa919 100644 --- a/patches/chromium/.patches +++ b/patches/chromium/.patches @@ -113,3 +113,4 @@ revert_stop_using_nsrunloop_in_renderer_process.patch fix_dont_delete_SerialPortManager_on_main_thread.patch feat_add_data_transfer_to_requestsingleinstancelock.patch fix_crash_when_saving_edited_pdf_files.patch +drop_extra_printingcontext_calls_to_newpage_pagedone.patch diff --git a/patches/chromium/drop_extra_printingcontext_calls_to_newpage_pagedone.patch b/patches/chromium/drop_extra_printingcontext_calls_to_newpage_pagedone.patch new file mode 100644 index 0000000000000..a27193a4aeb64 --- /dev/null +++ b/patches/chromium/drop_extra_printingcontext_calls_to_newpage_pagedone.patch @@ -0,0 +1,511 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Alan Screen +Date: Wed, 5 Jan 2022 23:15:29 +0000 +Subject: Drop extra PrintingContext calls to NewPage/PageDone + +When system calls were refactored out of PrintedDocument in +https://crrev.com/948253, there were extra calls to PrintingContext +NewPage() and PageDone() left in PrintedDocument::RenderPrintedDocument. +These had no effect on Windows or Linux, but caused an issue for macOS. + +This was undetected by tests which use TestPrintingContext, and would +only be detectable if a test was capable of using system drivers (which +generally does not occur on the bots). Adding a test to detect this is +difficult at this time, but should be easier to do once later portions +of the out-of-process print drivers commit chains fill in more testing +capabilities. At that point it should be easier to fire off a request +to have the macOS Preview be the output. https://crbug.com/1284745 +has been filed to capture this needed effort. + +Remove NewPage()/PageDone() as common methods to override from +PrintingContext and make these specific to macOS only. Update the +various implementations of PrintingContext::PrintDocument() to include +the aborted and in-print-job checks that were previously in all +NewPage() implementations. The same corresponding checks from +PageDone() can be handled by PrintedDocument::RenderPrintedDocument() +and PrintedDocumentWin::RenderPrintedPage(). There is no concern about +`in_print_job_` having changed during the lifetime of +RenderPrintedDocument()/RenderPrintedPage(), so just add extra checks +against printing having been asynchronouly aborted before allowing a +final successful return. + +Bug: 1283651 +Change-Id: I52992bc7550dd25d4ad9a1dc633c7d9452a27bb1 +Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3367333 +Reviewed-by: Lei Zhang +Commit-Queue: Alan Screen +Cr-Commit-Position: refs/heads/main@{#955936} + +diff --git a/chrome/browser/printing/print_job_worker.cc b/chrome/browser/printing/print_job_worker.cc +index 4283a5743c695a7376722f80925722d9e7a6496e..992a59c32ea082e3593c0183819d1b174fc8db7a 100644 +--- a/chrome/browser/printing/print_job_worker.cc ++++ b/chrome/browser/printing/print_job_worker.cc +@@ -524,12 +524,6 @@ void PrintJobWorker::SpoolPage(PrintedPage* page) { + DCHECK(task_runner_->RunsTasksInCurrentSequence()); + DCHECK_NE(page_number_, PageNumber::npos()); + +- // Preprocess. +- if (printing_context_->NewPage() != mojom::ResultCode::kSuccess) { +- OnFailure(); +- return; +- } +- + // Actual printing. + if (document_->RenderPrintedPage(*page, printing_context_.get()) != + mojom::ResultCode::kSuccess) { +@@ -537,12 +531,6 @@ void PrintJobWorker::SpoolPage(PrintedPage* page) { + return; + } + +- // Postprocess. +- if (printing_context_->PageDone() != mojom::ResultCode::kSuccess) { +- OnFailure(); +- return; +- } +- + // Signal everyone that the page is printed. + DCHECK(print_job_); + print_job_->PostTask(FROM_HERE, +diff --git a/printing/emf_win_unittest.cc b/printing/emf_win_unittest.cc +index e830a1017f2262d2d1c226aa16d41a6ffe53aadf..7daa9f9bedd664123d7590b9831169e99688b9c9 100644 +--- a/printing/emf_win_unittest.cc ++++ b/printing/emf_win_unittest.cc +@@ -115,7 +115,6 @@ TEST_F(EmfPrintingTest, Enumerate) { + // current directory. + // TODO(maruel): Clean the .PRN file generated in current directory. + context.NewDocument(u"EmfTest.Enumerate"); +- context.NewPage(); + // Process one at a time. + RECT page_bounds = emf.GetPageBounds(1).ToRECT(); + Emf::Enumerator emf_enum(emf, context.context(), &page_bounds); +@@ -129,7 +128,6 @@ TEST_F(EmfPrintingTest, Enumerate) { + EXPECT_TRUE(itr->SafePlayback(&emf_enum.context_)) + << " index: " << index << " type: " << itr->record()->iType; + } +- context.PageDone(); + context.DocumentDone(); + } + +diff --git a/printing/printed_document.cc b/printing/printed_document.cc +index 54dd798f4c76c34fd6414f68d3ad11a15c1673af..81ebe973fc6a99d66f5d8792aa19e01764f33d3f 100644 +--- a/printing/printed_document.cc ++++ b/printing/printed_document.cc +@@ -187,17 +187,18 @@ const MetafilePlayer* PrintedDocument::GetMetafile() { + + mojom::ResultCode PrintedDocument::RenderPrintedDocument( + PrintingContext* context) { +- mojom::ResultCode result = context->NewPage(); ++ base::AutoLock lock(lock_); ++ mojom::ResultCode result = context->PrintDocument( ++ *GetMetafile(), *immutable_.settings_, mutable_.expected_page_count_); + if (result != mojom::ResultCode::kSuccess) + return result; +- { +- base::AutoLock lock(lock_); +- result = context->PrintDocument(*GetMetafile(), *immutable_.settings_, +- mutable_.expected_page_count_); +- if (result != mojom::ResultCode::kSuccess) +- return result; +- } +- return context->PageDone(); ++ ++ // Beware of any asynchronous aborts of the print job that happened during ++ // printing. ++ if (context->PrintingAborted()) ++ return mojom::ResultCode::kCanceled; ++ ++ return mojom::ResultCode::kSuccess; + } + + bool PrintedDocument::IsComplete() const { +diff --git a/printing/printed_document_win.cc b/printing/printed_document_win.cc +index 4024150677fb64f8f8c9d53dfa73cf709c819a7c..8e34b288ec518c4e2d0c5d8113f38440ad2c648b 100644 +--- a/printing/printed_document_win.cc ++++ b/printing/printed_document_win.cc +@@ -24,8 +24,17 @@ mojom::ResultCode PrintedDocument::RenderPrintedPage( + #endif + + DCHECK(context); +- return context->RenderPage(page, +- immutable_.settings_->page_setup_device_units()); ++ mojom::ResultCode result = context->RenderPage( ++ page, immutable_.settings_->page_setup_device_units()); ++ if (result != mojom::ResultCode::kSuccess) ++ return result; ++ ++ // Beware of any asynchronous aborts of the print job that happened during ++ // printing. ++ if (context->PrintingAborted()) ++ return mojom::ResultCode::kCanceled; ++ ++ return mojom::ResultCode::kSuccess; + } + + } // namespace printing +diff --git a/printing/printing_context.h b/printing/printing_context.h +index e87170e6957733f06bcc296bcca3fc331557ed46..e196a7d448ca41ea02d523a4de410dedf4298b5e 100644 +--- a/printing/printing_context.h ++++ b/printing/printing_context.h +@@ -122,18 +122,12 @@ class COMPONENT_EXPORT(PRINTING) PrintingContext { + virtual mojom::ResultCode NewDocument( + const std::u16string& document_name) = 0; + +- // Starts a new page. +- virtual mojom::ResultCode NewPage() = 0; +- + #if defined(OS_WIN) + // Renders a page. + virtual mojom::ResultCode RenderPage(const PrintedPage& page, + const PageSetup& page_setup) = 0; + #endif + +- // Closes the printed page. +- virtual mojom::ResultCode PageDone() = 0; +- + // Prints the document contained in `metafile`. + virtual mojom::ResultCode PrintDocument(const MetafilePlayer& metafile, + const PrintSettings& settings, +@@ -178,6 +172,8 @@ class COMPONENT_EXPORT(PRINTING) PrintingContext { + // Reinitializes the settings for object reuse. + void ResetSettings(); + ++ bool PrintingAborted() const { return abort_printing_; } ++ + int job_id() const { return job_id_; } + + protected: +diff --git a/printing/printing_context_android.cc b/printing/printing_context_android.cc +index c28a40eb0a9ce0058d9f85948eda5f83d0c64791..5b2f096cb2a705e7c4c7ebbeddbf93cac1b9eafb 100644 +--- a/printing/printing_context_android.cc ++++ b/printing/printing_context_android.cc +@@ -213,30 +213,13 @@ mojom::ResultCode PrintingContextAndroid::NewDocument( + return mojom::ResultCode::kSuccess; + } + +-mojom::ResultCode PrintingContextAndroid::NewPage() { +- if (abort_printing_) +- return mojom::ResultCode::kCanceled; +- DCHECK(in_print_job_); +- +- // Intentional No-op. +- +- return mojom::ResultCode::kSuccess; +-} +- +-mojom::ResultCode PrintingContextAndroid::PageDone() { +- if (abort_printing_) +- return mojom::ResultCode::kCanceled; +- DCHECK(in_print_job_); +- +- // Intentional No-op. +- +- return mojom::ResultCode::kSuccess; +-} +- + mojom::ResultCode PrintingContextAndroid::PrintDocument( + const MetafilePlayer& metafile, + const PrintSettings& settings, + uint32_t num_pages) { ++ if (abort_printing_) ++ return mojom::ResultCode::kCanceled; ++ DCHECK(in_print_job_); + DCHECK(is_file_descriptor_valid()); + + return metafile.SaveToFileDescriptor(fd_) ? mojom::ResultCode::kSuccess +diff --git a/printing/printing_context_android.h b/printing/printing_context_android.h +index 676d98949da675e3283969dd3c9c61028dc1c7b5..b4f2f25edec45f91f241b0b1a7bc73b3f51af252 100644 +--- a/printing/printing_context_android.h ++++ b/printing/printing_context_android.h +@@ -60,8 +60,6 @@ class COMPONENT_EXPORT(PRINTING) PrintingContextAndroid + mojom::ResultCode UpdatePrinterSettings( + const PrinterSettings& printer_settings) override; + mojom::ResultCode NewDocument(const std::u16string& document_name) override; +- mojom::ResultCode NewPage() override; +- mojom::ResultCode PageDone() override; + mojom::ResultCode PrintDocument(const MetafilePlayer& metafile, + const PrintSettings& settings, + uint32_t num_pages) override; +diff --git a/printing/printing_context_chromeos.cc b/printing/printing_context_chromeos.cc +index d996c3e1f7b2d9f96f5c50aadff67f8273c7e760..670df66ae406fa883f801682de699aab16779591 100644 +--- a/printing/printing_context_chromeos.cc ++++ b/printing/printing_context_chromeos.cc +@@ -402,32 +402,14 @@ mojom::ResultCode PrintingContextChromeos::NewDocument( + return mojom::ResultCode::kSuccess; + } + +-mojom::ResultCode PrintingContextChromeos::NewPage() { +- if (abort_printing_) +- return mojom::ResultCode::kCanceled; +- +- DCHECK(in_print_job_); +- +- // Intentional No-op. +- +- return mojom::ResultCode::kSuccess; +-} +- +-mojom::ResultCode PrintingContextChromeos::PageDone() { +- if (abort_printing_) +- return mojom::ResultCode::kCanceled; +- +- DCHECK(in_print_job_); +- +- // Intentional No-op. +- +- return mojom::ResultCode::kSuccess; +-} +- + mojom::ResultCode PrintingContextChromeos::PrintDocument( + const MetafilePlayer& metafile, + const PrintSettings& settings, + uint32_t num_pages) { ++ if (abort_printing_) ++ return mojom::ResultCode::kCanceled; ++ DCHECK(in_print_job_); ++ + #if defined(USE_CUPS) + std::vector buffer; + if (!metafile.GetDataAsVector(&buffer)) +diff --git a/printing/printing_context_chromeos.h b/printing/printing_context_chromeos.h +index 77353372b2c6e003428d37ab634167e8dc824fa4..06cb16c8a569a4d90ccdccc5fefcaaf527e49f5f 100644 +--- a/printing/printing_context_chromeos.h ++++ b/printing/printing_context_chromeos.h +@@ -39,8 +39,6 @@ class COMPONENT_EXPORT(PRINTING) PrintingContextChromeos + mojom::ResultCode UpdatePrinterSettings( + const PrinterSettings& printer_settings) override; + mojom::ResultCode NewDocument(const std::u16string& document_name) override; +- mojom::ResultCode NewPage() override; +- mojom::ResultCode PageDone() override; + mojom::ResultCode PrintDocument(const MetafilePlayer& metafile, + const PrintSettings& settings, + uint32_t num_pages) override; +diff --git a/printing/printing_context_linux.cc b/printing/printing_context_linux.cc +index c5adfa317747cda3d370a2ace52d843662f4ce91..204cec8311bec69674f1da2223e8d6c4b5a13ba3 100644 +--- a/printing/printing_context_linux.cc ++++ b/printing/printing_context_linux.cc +@@ -151,30 +151,13 @@ mojom::ResultCode PrintingContextLinux::NewDocument( + return mojom::ResultCode::kSuccess; + } + +-mojom::ResultCode PrintingContextLinux::NewPage() { +- if (abort_printing_) +- return mojom::ResultCode::kCanceled; +- DCHECK(in_print_job_); +- +- // Intentional No-op. +- +- return mojom::ResultCode::kSuccess; +-} +- +-mojom::ResultCode PrintingContextLinux::PageDone() { +- if (abort_printing_) +- return mojom::ResultCode::kCanceled; +- DCHECK(in_print_job_); +- +- // Intentional No-op. +- +- return mojom::ResultCode::kSuccess; +-} +- + mojom::ResultCode PrintingContextLinux::PrintDocument( + const MetafilePlayer& metafile, + const PrintSettings& settings, + uint32_t num_pages) { ++ if (abort_printing_) ++ return mojom::ResultCode::kCanceled; ++ DCHECK(in_print_job_); + DCHECK(print_dialog_); + // TODO(crbug.com/1252685) Plumb error code back from + // `PrintDialogGtkInterface`. +diff --git a/printing/printing_context_linux.h b/printing/printing_context_linux.h +index 17d768a24141954df80bed3c5dd6735c34115195..653170ba60e8341c972cb0126109d48394e0de7b 100644 +--- a/printing/printing_context_linux.h ++++ b/printing/printing_context_linux.h +@@ -45,8 +45,6 @@ class COMPONENT_EXPORT(PRINTING) PrintingContextLinux : public PrintingContext { + mojom::ResultCode UpdatePrinterSettings( + const PrinterSettings& printer_settings) override; + mojom::ResultCode NewDocument(const std::u16string& document_name) override; +- mojom::ResultCode NewPage() override; +- mojom::ResultCode PageDone() override; + mojom::ResultCode PrintDocument(const MetafilePlayer& metafile, + const PrintSettings& settings, + uint32_t num_pages) override; +diff --git a/printing/printing_context_mac.h b/printing/printing_context_mac.h +index 85363bd922bf0ab2630e3d5f350de0c58792963a..221019f5df71e1d66accbf2ea2d161bd1125666f 100644 +--- a/printing/printing_context_mac.h ++++ b/printing/printing_context_mac.h +@@ -35,8 +35,6 @@ class COMPONENT_EXPORT(PRINTING) PrintingContextMac : public PrintingContext { + mojom::ResultCode UpdatePrinterSettings( + const PrinterSettings& printer_settings) override; + mojom::ResultCode NewDocument(const std::u16string& document_name) override; +- mojom::ResultCode NewPage() override; +- mojom::ResultCode PageDone() override; + mojom::ResultCode PrintDocument(const MetafilePlayer& metafile, + const PrintSettings& settings, + uint32_t num_pages) override; +@@ -105,6 +103,12 @@ class COMPONENT_EXPORT(PRINTING) PrintingContextMac : public PrintingContext { + // Returns true is the pair is set. + bool SetKeyValue(base::StringPiece key, base::StringPiece value); + ++ // Starts a new page. ++ mojom::ResultCode NewPage(); ++ ++ // Closes the printed page. ++ mojom::ResultCode PageDone(); ++ + // The native print info object. + base::scoped_nsobject print_info_; + +diff --git a/printing/printing_context_no_system_dialog.cc b/printing/printing_context_no_system_dialog.cc +index c10123f35b11e1600c813080585a2282d775dba1..253507fcee5476f497421316bc42a0794f97b12b 100644 +--- a/printing/printing_context_no_system_dialog.cc ++++ b/printing/printing_context_no_system_dialog.cc +@@ -98,26 +98,6 @@ mojom::ResultCode PrintingContextNoSystemDialog::NewDocument( + return mojom::ResultCode::kSuccess; + } + +-mojom::ResultCode PrintingContextNoSystemDialog::NewPage() { +- if (abort_printing_) +- return mojom::ResultCode::kCanceled; +- DCHECK(in_print_job_); +- +- // Intentional No-op. +- +- return mojom::ResultCode::kSuccess; +-} +- +-mojom::ResultCode PrintingContextNoSystemDialog::PageDone() { +- if (abort_printing_) +- return mojom::ResultCode::kCanceled; +- DCHECK(in_print_job_); +- +- // Intentional No-op. +- +- return mojom::ResultCode::kSuccess; +-} +- + mojom::ResultCode PrintingContextNoSystemDialog::PrintDocument( + const MetafilePlayer& metafile, + const PrintSettings& settings, +diff --git a/printing/printing_context_no_system_dialog.h b/printing/printing_context_no_system_dialog.h +index 2753d7ba7a09e1c5e88c1eb21a9b146034ea53f5..cb0d0b8c12535c83dbfc07ecb2817f0504b4a8ba 100644 +--- a/printing/printing_context_no_system_dialog.h ++++ b/printing/printing_context_no_system_dialog.h +@@ -32,8 +32,6 @@ class COMPONENT_EXPORT(PRINTING) PrintingContextNoSystemDialog + mojom::ResultCode UpdatePrinterSettings( + const PrinterSettings& printer_settings) override; + mojom::ResultCode NewDocument(const std::u16string& document_name) override; +- mojom::ResultCode NewPage() override; +- mojom::ResultCode PageDone() override; + mojom::ResultCode PrintDocument(const MetafilePlayer& metafile, + const PrintSettings& settings, + uint32_t num_pages) override; +diff --git a/printing/printing_context_win.cc b/printing/printing_context_win.cc +index 0addb52cd2f65b6f05b7cfb7cf8602baaf592541..4f1fb12619fc9bee0cce104ba60621008284d675 100644 +--- a/printing/printing_context_win.cc ++++ b/printing/printing_context_win.cc +@@ -366,18 +366,6 @@ mojom::ResultCode PrintingContextWin::NewDocument( + return mojom::ResultCode::kSuccess; + } + +-mojom::ResultCode PrintingContextWin::NewPage() { +- if (abort_printing_) +- return mojom::ResultCode::kCanceled; +- DCHECK(context_); +- DCHECK(in_print_job_); +- +- // Intentional No-op. MetafileSkia::SafePlayback takes care of calling +- // ::StartPage(). +- +- return mojom::ResultCode::kSuccess; +-} +- + mojom::ResultCode PrintingContextWin::RenderPage(const PrintedPage& page, + const PageSetup& page_setup) { + if (abort_printing_) +@@ -416,17 +404,6 @@ mojom::ResultCode PrintingContextWin::RenderPage(const PrintedPage& page, + return mojom::ResultCode::kSuccess; + } + +-mojom::ResultCode PrintingContextWin::PageDone() { +- if (abort_printing_) +- return mojom::ResultCode::kCanceled; +- DCHECK(in_print_job_); +- +- // Intentional No-op. MetafileSkia::SafePlayback takes care of calling +- // ::EndPage(). +- +- return mojom::ResultCode::kSuccess; +-} +- + mojom::ResultCode PrintingContextWin::PrintDocument( + const MetafilePlayer& metafile, + const PrintSettings& settings, +diff --git a/printing/printing_context_win.h b/printing/printing_context_win.h +index 700bfe2c744cc18b1be25b9db02dd690b5e306f9..babd443a1586c92a70ff6362a824e29a6395fae2 100644 +--- a/printing/printing_context_win.h ++++ b/printing/printing_context_win.h +@@ -35,10 +35,8 @@ class COMPONENT_EXPORT(PRINTING) PrintingContextWin : public PrintingContext { + mojom::ResultCode UpdatePrinterSettings( + const PrinterSettings& printer_settings) override; + mojom::ResultCode NewDocument(const std::u16string& document_name) override; +- mojom::ResultCode NewPage() override; + mojom::ResultCode RenderPage(const PrintedPage& page, + const PageSetup& page_setup) override; +- mojom::ResultCode PageDone() override; + mojom::ResultCode PrintDocument(const MetafilePlayer& metafile, + const PrintSettings& settings, + uint32_t num_pages) override; +diff --git a/printing/test_printing_context.cc b/printing/test_printing_context.cc +index 4f0bf0ae891164120d36c79a7df98845f38e55de..208e8a16b5648383ff26d8dec4a15f6485ef4454 100644 +--- a/printing/test_printing_context.cc ++++ b/printing/test_printing_context.cc +@@ -116,15 +116,6 @@ mojom::ResultCode TestPrintingContext::NewDocument( + return mojom::ResultCode::kSuccess; + } + +-mojom::ResultCode TestPrintingContext::NewPage() { +- if (abort_printing_) +- return mojom::ResultCode::kCanceled; +- DCHECK(in_print_job_); +- +- // No-op. +- return mojom::ResultCode::kSuccess; +-} +- + #if defined(OS_WIN) + mojom::ResultCode TestPrintingContext::RenderPage(const PrintedPage& page, + const PageSetup& page_setup) { +@@ -138,15 +129,6 @@ mojom::ResultCode TestPrintingContext::RenderPage(const PrintedPage& page, + } + #endif // defined(OS_WIN) + +-mojom::ResultCode TestPrintingContext::PageDone() { +- if (abort_printing_) +- return mojom::ResultCode::kCanceled; +- DCHECK(in_print_job_); +- +- // No-op. +- return mojom::ResultCode::kSuccess; +-} +- + mojom::ResultCode TestPrintingContext::PrintDocument( + const MetafilePlayer& metafile, + const PrintSettings& settings, +diff --git a/printing/test_printing_context.h b/printing/test_printing_context.h +index e2a0f906216f5bae2d05dd6efd6246257a3deea4..579a9269459470e8eac6e506d87503d828bed4cb 100644 +--- a/printing/test_printing_context.h ++++ b/printing/test_printing_context.h +@@ -58,12 +58,10 @@ class TestPrintingContext : public PrintingContext { + mojom::ResultCode UpdatePrinterSettings( + const PrinterSettings& printer_settings) override; + mojom::ResultCode NewDocument(const std::u16string& document_name) override; +- mojom::ResultCode NewPage() override; + #if defined(OS_WIN) + mojom::ResultCode RenderPage(const PrintedPage& page, + const PageSetup& page_setup) override; + #endif +- mojom::ResultCode PageDone() override; + mojom::ResultCode PrintDocument(const MetafilePlayer& metafile, + const PrintSettings& settings, + uint32_t num_pages) override;