electron/patches/chromium/drop_extra_printingcontext_calls_to_newpage_pagedone.patch

From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Alan Screen <[email protected]>
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 <[email protected]>
Commit-Queue: Alan Screen <[email protected]>
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<char> 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<NSPrintInfo> 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;