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

* refactor: clean up callback handling in webContents.print() * chore: update patches Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
electron · Jul 21, 2022 · 1ed1911 · 1ed1911
1 parent d4e9748
commit 1ed1911
Showing 1 changed file with 99 additions and 45 deletions.
diff --git a/patches/chromium/printing.patch b/patches/chromium/printing.patch
@@ -134,7 +134,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..e6201cbf8d1ed64b1b53ed43d89882163c93ce89 100644
+index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..78f3db8a6fb8b9184cfd41188b11851ac8b8df85 100644
 --- a/chrome/browser/printing/print_view_manager_base.cc
 +++ b/chrome/browser/printing/print_view_manager_base.cc
 @@ -30,8 +30,6 @@
@@ -146,7 +146,22 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..e6201cbf8d1ed64b1b53ed43d8988216
  #include "chrome/common/pref_names.h"
  #include "chrome/grit/generated_resources.h"
  #include "components/prefs/pref_service.h"
-@@ -87,6 +85,8 @@ using PrintSettingsCallback =
+@@ -83,10 +81,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) {
@@ -155,15 +170,15 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..e6201cbf8d1ed64b1b53ed43d8988216
    // Runs always on the UI thread.
    static bool is_dialog_shown = false;
    if (is_dialog_shown)
-@@ -95,6 +95,7 @@ void ShowWarningMessageBox(const std::u16string& message) {
+@@ -95,6 +106,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 +145,9 @@ void OnDidUpdatePrintSettings(
+@@ -144,7 +156,9 @@ void OnDidUpdatePrintSettings(
    DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
    DCHECK(printer_query);
    mojom::PrintPagesParamsPtr params = CreateEmptyPrintPagesParamsPtr();
@@ -174,15 +189,15 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..e6201cbf8d1ed64b1b53ed43d8988216
      RenderParamsFromPrintSettings(printer_query->settings(),
                                    params->params.get());
      params->params->document_cookie = printer_query->cookie();
-@@ -172,6 +175,7 @@ void OnDidScriptedPrint(
+@@ -172,6 +186,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 +185,8 @@ void OnDidScriptedPrint(
+@@ -181,7 +196,8 @@ void OnDidScriptedPrint(
    }
    bool has_valid_cookie = params->params->document_cookie;
    bool has_dpi = !params->params->dpi.IsEmpty();
@@ -192,7 +207,7 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..e6201cbf8d1ed64b1b53ed43d8988216
 
    if (has_dpi && has_valid_cookie) {
      queue->QueuePrinterQuery(std::move(printer_query));
-@@ -196,12 +201,14 @@ PrintViewManagerBase::PrintViewManagerBase(content::WebContents* web_contents)
+@@ -196,12 +212,14 @@ PrintViewManagerBase::PrintViewManagerBase(content::WebContents* web_contents)
      : PrintManager(web_contents),
        queue_(g_browser_process->print_job_manager()->queue()) {
    DCHECK(queue_);
@@ -207,7 +222,7 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..e6201cbf8d1ed64b1b53ed43d8988216
  }
 
  PrintViewManagerBase::~PrintViewManagerBase() {
-@@ -209,7 +216,10 @@ PrintViewManagerBase::~PrintViewManagerBase() {
+@@ -209,7 +227,10 @@ PrintViewManagerBase::~PrintViewManagerBase() {
    DisconnectFromCurrentPrintJob();
  }
 
@@ -219,7 +234,7 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..e6201cbf8d1ed64b1b53ed43d8988216
    // 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 +247,9 @@ bool PrintViewManagerBase::PrintNow(content::RenderFrameHost* rfh) {
+@@ -237,6 +258,9 @@ bool PrintViewManagerBase::PrintNow(content::RenderFrameHost* rfh) {
  #endif
 
    SetPrintingRFH(rfh);
@@ -229,7 +244,7 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..e6201cbf8d1ed64b1b53ed43d8988216
 
  #if BUILDFLAG(ENABLE_PRINT_CONTENT_ANALYSIS)
    enterprise_connectors::ContentAnalysisDelegate::Data scanning_data;
-@@ -405,7 +418,8 @@ void PrintViewManagerBase::GetDefaultPrintSettingsReply(
+@@ -405,7 +429,8 @@ void PrintViewManagerBase::GetDefaultPrintSettingsReply(
  void PrintViewManagerBase::ScriptedPrintReply(
      ScriptedPrintCallback callback,
      int process_id,
@@ -239,7 +254,7 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..e6201cbf8d1ed64b1b53ed43d8988216
    DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
  #if BUILDFLAG(ENABLE_OOP_PRINTING)
-@@ -420,8 +434,11 @@ void PrintViewManagerBase::ScriptedPrintReply(
+@@ -420,8 +445,11 @@ void PrintViewManagerBase::ScriptedPrintReply(
      return;
    }
 
@@ -252,7 +267,7 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..e6201cbf8d1ed64b1b53ed43d8988216
  }
 
  void PrintViewManagerBase::UpdatePrintingEnabled() {
-@@ -429,8 +446,7 @@ void PrintViewManagerBase::UpdatePrintingEnabled() {
+@@ -429,8 +457,7 @@ void PrintViewManagerBase::UpdatePrintingEnabled() {
    // The Unretained() is safe because ForEachRenderFrameHost() is synchronous.
    web_contents()->GetPrimaryMainFrame()->ForEachRenderFrameHost(
        base::BindRepeating(&PrintViewManagerBase::SendPrintingEnabled,
@@ -262,7 +277,7 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..e6201cbf8d1ed64b1b53ed43d8988216
  }
 
  void PrintViewManagerBase::NavigationStopped() {
-@@ -546,11 +562,14 @@ void PrintViewManagerBase::DidPrintDocument(
+@@ -546,11 +573,14 @@ void PrintViewManagerBase::DidPrintDocument(
  void PrintViewManagerBase::GetDefaultPrintSettings(
      GetDefaultPrintSettingsCallback callback) {
    DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
@@ -277,7 +292,7 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..e6201cbf8d1ed64b1b53ed43d8988216
  #if BUILDFLAG(ENABLE_OOP_PRINTING)
    if (printing::features::kEnableOopPrintDriversJobPrint.Get() &&
        !service_manager_client_id_.has_value()) {
-@@ -588,18 +607,20 @@ void PrintViewManagerBase::UpdatePrintSettings(
+@@ -588,18 +618,20 @@ void PrintViewManagerBase::UpdatePrintSettings(
      base::Value::Dict job_settings,
      UpdatePrintSettingsCallback callback) {
    DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
@@ -299,15 +314,15 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..e6201cbf8d1ed64b1b53ed43d8988216
    content::BrowserContext* context =
        web_contents() ? web_contents()->GetBrowserContext() : nullptr;
    PrefService* prefs =
-@@ -609,6 +630,7 @@ void PrintViewManagerBase::UpdatePrintSettings(
+@@ -609,6 +641,7 @@ void PrintViewManagerBase::UpdatePrintSettings(
      if (value > 0)
        job_settings.Set(kSettingRasterizePdfDpi, value);
    }
 +#endif
 
    auto callback_wrapper =
        base::BindOnce(&PrintViewManagerBase::UpdatePrintSettingsReply,
-@@ -640,14 +662,14 @@ void PrintViewManagerBase::ScriptedPrint(mojom::ScriptedPrintParamsPtr params,
+@@ -640,14 +673,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);
@@ -324,27 +339,27 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..e6201cbf8d1ed64b1b53ed43d8988216
      return;
    }
  #endif
-@@ -685,7 +707,6 @@ void PrintViewManagerBase::PrintingFailed(int32_t cookie,
+@@ -685,7 +718,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 +721,11 @@ void PrintViewManagerBase::RemoveObserver(Observer& observer) {
+@@ -700,6 +732,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 +736,12 @@ void PrintViewManagerBase::RenderFrameHostStateChanged(
+@@ -710,10 +747,12 @@ void PrintViewManagerBase::RenderFrameHostStateChanged(
      content::RenderFrameHost* render_frame_host,
      content::RenderFrameHost::LifecycleState /*old_state*/,
      content::RenderFrameHost::LifecycleState new_state) {
@@ -357,19 +372,30 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..e6201cbf8d1ed64b1b53ed43d8988216
  }
 
  void PrintViewManagerBase::DidStartLoading() {
-@@ -773,6 +801,11 @@ void PrintViewManagerBase::OnJobDone() {
-   ReleasePrintJob();
- }
-
-+void PrintViewManagerBase::UserInitCanceled() {
-+  printing_canceled_ = true;
+@@ -769,7 +808,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 +864,10 @@ bool PrintViewManagerBase::CreateNewPrintJob(
+
+@@ -783,7 +827,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 +875,10 @@ bool PrintViewManagerBase::CreateNewPrintJob(
 
    // Disconnect the current `print_job_`.
    auto weak_this = weak_ptr_factory_.GetWeakPtr();
@@ -381,21 +407,37 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..e6201cbf8d1ed64b1b53ed43d8988216
    if (!weak_this)
      return false;
 
-@@ -912,6 +948,13 @@ void PrintViewManagerBase::ReleasePrintJob() {
+@@ -852,7 +899,7 @@ bool PrintViewManagerBase::CreateNewPrintJob(
+ #endif
+   print_job_->AddObserver(*this);
+
+-  printing_succeeded_ = false;
++  printing_status_ = PrintStatus::kFailed;
+   return true;
+ }
+
+@@ -912,6 +959,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 +1004,7 @@ bool PrintViewManagerBase::RunInnerMessageLoop() {
+@@ -919,7 +971,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 +1013,7 @@ bool PrintViewManagerBase::RunInnerMessageLoop() {
  }
 
  bool PrintViewManagerBase::OpportunisticallyCreatePrintJob(int cookie) {
@@ -404,7 +446,7 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..e6201cbf8d1ed64b1b53ed43d8988216
      return true;
 
    if (!cookie) {
-@@ -1069,7 +1112,7 @@ void PrintViewManagerBase::SendPrintingEnabled(bool enabled,
+@@ -1069,7 +1121,7 @@ void PrintViewManagerBase::SendPrintingEnabled(bool enabled,
  }
 
  void PrintViewManagerBase::CompletePrintNow(content::RenderFrameHost* rfh) {
@@ -414,7 +456,7 @@ index 4bd871e5cadc693aea6e8c71b3fe3296b743d9ec..e6201cbf8d1ed64b1b53ed43d8988216
    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 {
@@ -446,7 +488,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,
@@ -456,22 +513,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 f3c3f85edb19489d079dc93411be64828ae581e2..ff303dcbc034cd8f1530fe1543729e98d3035826 100644
 --- a/chrome/browser/ui/webui/print_preview/fake_print_render_frame.cc