Remove PrintManagerHost::DidGetDocumentCookie() interface.

In PrintManager and derived classes, set the document cookie used for printing in those classes in the browser process. Don't pass it to a renderer, get the value back, and then set it. Bug: 1202661 Change-Id: I9482c886d7aa3d2e17f06d8d218f87f8e27784f4 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2909373 Reviewed-by: Alan Screen <awscreen@chromium.org> Reviewed-by: Robert Sesek <rsesek@chromium.org> Reviewed-by: Andrey Kosyakov <caseq@chromium.org> Commit-Queue: Lei Zhang <thestig@chromium.org> Cr-Commit-Position: refs/heads/master@{#886032}
FairyWorld · May 24, 2021 · eead1ba · eead1ba
1 parent 15aa757
commit eead1ba
Show file tree

Hide file tree

Showing 8 changed files with 77 additions and 67 deletions.
diff --git a/chrome/browser/printing/print_view_manager_base.cc b/chrome/browser/printing/print_view_manager_base.cc
@@ -148,16 +148,6 @@ void CreateQueryWithSettings(base::Value job_settings,
 }
 #endif  // BUILDFLAG(ENABLE_PRINT_PREVIEW)
 
-// Runs |callback| with |params| to reply to
-// mojom::PrintManagerHost::GetDefaultPrintSettings.
-void GetDefaultPrintSettingsReply(
-    mojom::PrintManagerHost::GetDefaultPrintSettingsCallback callback,
-    mojom::PrintParamsPtr params) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  DCHECK(params);
-  std::move(callback).Run(std::move(params));
-}
-
 void GetDefaultPrintSettingsReplyOnIO(
     scoped_refptr<PrintQueriesQueue> queue,
     std::unique_ptr<PrinterQuery> printer_query,
@@ -170,8 +160,7 @@ void GetDefaultPrintSettingsReplyOnIO(
   }
 
   content::GetUIThreadTaskRunner({})->PostTask(
-      FROM_HERE, base::BindOnce(&GetDefaultPrintSettingsReply,
-                                std::move(callback), std::move(params)));
+      FROM_HERE, base::BindOnce(std::move(callback), std::move(params)));
 
   // If printing was enabled.
   if (printer_query) {
@@ -212,17 +201,6 @@ mojom::PrintPagesParamsPtr CreateEmptyPrintPagesParamsPtr() {
 }
 
 #if BUILDFLAG(ENABLE_PRINT_PREVIEW)
-// Runs |callback| with |params| to reply to
-// mojom::PrintManagerHost::UpdatePrintSettings.
-void UpdatePrintSettingsReply(
-    mojom::PrintManagerHost::UpdatePrintSettingsCallback callback,
-    mojom::PrintPagesParamsPtr params,
-    bool canceled) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  DCHECK(params);
-  std::move(callback).Run(std::move(params), canceled);
-}
-
 #if defined(OS_WIN)
 content::WebContents* GetWebContentsForRenderFrame(int render_process_id,
                                                    int render_frame_id) {
@@ -274,8 +252,8 @@ void UpdatePrintSettingsReplyOnIO(
 #endif
 
   content::GetUIThreadTaskRunner({})->PostTask(
-      FROM_HERE, base::BindOnce(&UpdatePrintSettingsReply, std::move(callback),
-                                std::move(params), canceled));
+      FROM_HERE,
+      base::BindOnce(std::move(callback), std::move(params), canceled));
 
   if (printer_query->cookie() && printer_query->settings().dpi()) {
     queue->QueuePrinterQuery(std::move(printer_query));
@@ -306,27 +284,10 @@ void UpdatePrintSettingsOnIO(
 }
 #endif  // BUILDFLAG(ENABLE_PRINT_PREVIEW)
 
-// Runs |callback| with |params| to reply to
-// mojom::PrintManagerHost::ScriptedPrint.
-void ScriptedPrintReply(mojom::PrintManagerHost::ScriptedPrintCallback callback,
-                        mojom::PrintPagesParamsPtr params,
-                        int process_id) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-
-  if (!content::RenderProcessHost::FromID(process_id)) {
-    // Early return if the renderer is not alive.
-    return;
-  }
-
-  DCHECK(params);
-  std::move(callback).Run(std::move(params));
-}
-
 void ScriptedPrintReplyOnIO(
     scoped_refptr<PrintQueriesQueue> queue,
     std::unique_ptr<PrinterQuery> printer_query,
-    mojom::PrintManagerHost::ScriptedPrintCallback callback,
-    int process_id) {
+    mojom::PrintManagerHost::ScriptedPrintCallback callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   mojom::PrintPagesParamsPtr params = CreateEmptyPrintPagesParamsPtr();
   if (printer_query->last_status() == PrintingContext::OK &&
@@ -339,8 +300,7 @@ void ScriptedPrintReplyOnIO(
   bool has_valid_cookie = params->params->document_cookie;
   bool has_dpi = !params->params->dpi.IsEmpty();
   content::GetUIThreadTaskRunner({})->PostTask(
-      FROM_HERE, base::BindOnce(&ScriptedPrintReply, std::move(callback),
-                                std::move(params), process_id));
+      FROM_HERE, base::BindOnce(std::move(callback), std::move(params)));
 
   if (has_dpi && has_valid_cookie) {
     queue->QueuePrinterQuery(std::move(printer_query));
@@ -370,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), process_id));
+                     std::move(callback)));
 }
 
 }  // namespace
@@ -496,6 +456,7 @@ void PrintViewManagerBase::StartLocalPrintJob(
     PrinterHandler::PrintCallback callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
+  set_cookie(cookie);
   DidGetPrintedPagesCount(cookie, page_count);
 
   if (!PrintJobHasDocument(cookie)) {
@@ -516,8 +477,41 @@ void PrintViewManagerBase::StartLocalPrintJob(
                 settings.page_setup_device_units().printable_area().origin());
   std::move(callback).Run(base::Value());
 }
+
+void PrintViewManagerBase::UpdatePrintSettingsReply(
+    mojom::PrintManagerHost::UpdatePrintSettingsCallback callback,
+    mojom::PrintPagesParamsPtr params,
+    bool canceled) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  set_cookie(params->params->document_cookie);
+  std::move(callback).Run(std::move(params), canceled);
+}
+
 #endif  // BUILDFLAG(ENABLE_PRINT_PREVIEW)
 
+void PrintViewManagerBase::GetDefaultPrintSettingsReply(
+    GetDefaultPrintSettingsCallback callback,
+    mojom::PrintParamsPtr params) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  set_cookie(params->document_cookie);
+  std::move(callback).Run(std::move(params));
+}
+
+void PrintViewManagerBase::ScriptedPrintReply(
+    ScriptedPrintCallback callback,
+    int process_id,
+    mojom::PrintPagesParamsPtr params) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  if (!content::RenderProcessHost::FromID(process_id)) {
+    // Early return if the renderer is not alive.
+    return;
+  }
+
+  set_cookie(params->params->document_cookie);
+  std::move(callback).Run(std::move(params));
+}
+
 void PrintViewManagerBase::UpdatePrintingEnabled() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   // The Unretained() is safe because ForEachFrame() is synchronous.
@@ -644,10 +638,13 @@ void PrintViewManagerBase::GetDefaultPrintSettings(
   }
 
   content::RenderFrameHost* render_frame_host = GetCurrentTargetFrame();
+  auto callback_wrapper =
+      base::BindOnce(&PrintViewManagerBase::GetDefaultPrintSettingsReply,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(callback));
   content::GetIOThreadTaskRunner({})->PostTask(
       FROM_HERE,
-      base::BindOnce(&GetDefaultPrintSettingsOnIO, std::move(callback), queue_,
-                     render_frame_host->GetProcess()->GetID(),
+      base::BindOnce(&GetDefaultPrintSettingsOnIO, std::move(callback_wrapper),
+                     queue_, render_frame_host->GetProcess()->GetID(),
                      render_frame_host->GetRoutingID()));
 }
 
@@ -670,12 +667,15 @@ void PrintViewManagerBase::UpdatePrintSettings(
   }
 
   content::RenderFrameHost* render_frame_host = GetCurrentTargetFrame();
+  auto callback_wrapper =
+      base::BindOnce(&PrintViewManagerBase::UpdatePrintSettingsReply,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(callback));
   content::GetIOThreadTaskRunner({})->PostTask(
-      FROM_HERE,
-      base::BindOnce(&UpdatePrintSettingsOnIO, cookie, std::move(callback),
-                     queue_, std::move(job_settings),
-                     render_frame_host->GetProcess()->GetID(),
-                     render_frame_host->GetRoutingID()));
+      FROM_HERE, base::BindOnce(&UpdatePrintSettingsOnIO, cookie,
+                                std::move(callback_wrapper), queue_,
+                                std::move(job_settings),
+                                render_frame_host->GetProcess()->GetID(),
+                                render_frame_host->GetRoutingID()));
 }
 #endif  // BUILDFLAG(ENABLE_PRINT_PREVIEW)
 
@@ -684,11 +684,15 @@ void PrintViewManagerBase::ScriptedPrint(mojom::ScriptedPrintParamsPtr params,
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   content::RenderFrameHost* render_frame_host = GetCurrentTargetFrame();
+  int process_id = render_frame_host->GetProcess()->GetID();
+  int routing_id = render_frame_host->GetRoutingID();
+  auto callback_wrapper = base::BindOnce(
+      &PrintViewManagerBase::ScriptedPrintReply, weak_ptr_factory_.GetWeakPtr(),
+      std::move(callback), process_id);
   content::GetIOThreadTaskRunner({})->PostTask(
-      FROM_HERE,
-      base::BindOnce(&ScriptedPrintOnIO, std::move(params), std::move(callback),
-                     queue_, render_frame_host->GetProcess()->GetID(),
-                     render_frame_host->GetRoutingID()));
+      FROM_HERE, base::BindOnce(&ScriptedPrintOnIO, std::move(params),
+                                std::move(callback_wrapper), queue_, process_id,
+                                routing_id));
 }
 
 void PrintViewManagerBase::PrintingFailed(int32_t cookie) {

diff --git a/chrome/browser/printing/print_view_manager_base.h b/chrome/browser/printing/print_view_manager_base.h
@@ -157,8 +157,23 @@ class PrintViewManagerBase : public content::NotificationObserver,
                           uint32_t page_count,
                           int cookie,
                           PrinterHandler::PrintCallback callback);
+
+  // Runs `callback` with `params` to reply to UpdatePrintSettings().
+  void UpdatePrintSettingsReply(
+      mojom::PrintManagerHost::UpdatePrintSettingsCallback callback,
+      mojom::PrintPagesParamsPtr params,
+      bool canceled);
 #endif  // BUILDFLAG(ENABLE_PRINT_PREVIEW)
 
+  // Runs `callback` with `params` to reply to GetDefaultPrintSettings().
+  void GetDefaultPrintSettingsReply(GetDefaultPrintSettingsCallback callback,
+                                    mojom::PrintParamsPtr params);
+
+  // Runs `callback` with `params` to reply to ScriptedPrint().
+  void ScriptedPrintReply(ScriptedPrintCallback callback,
+                          int process_id,
+                          mojom::PrintPagesParamsPtr params);
+
   // Processes a NOTIFY_PRINT_JOB_EVENT notification.
   void OnNotifyPrintJobEvent(const JobEventDetails& event_details);
 

diff --git a/components/printing/browser/print_manager.cc b/components/printing/browser/print_manager.cc
@@ -29,10 +29,6 @@ void PrintManager::DidGetPrintedPagesCount(int32_t cookie,
   number_pages_ = number_pages;
 }
 
-void PrintManager::DidGetDocumentCookie(int32_t cookie) {
-  cookie_ = cookie;
-}
-
 void PrintManager::DidShowPrintDialog() {}
 
 void PrintManager::DidPrintDocument(mojom::DidPrintDocumentParamsPtr params,

diff --git a/components/printing/browser/print_manager.h b/components/printing/browser/print_manager.h
@@ -40,7 +40,6 @@ class PrintManager : public content::WebContentsObserver,
 
   // printing::mojom::PrintManagerHost:
   void DidGetPrintedPagesCount(int32_t cookie, uint32_t number_pages) override;
-  void DidGetDocumentCookie(int32_t cookie) override;
   void DidPrintDocument(mojom::DidPrintDocumentParamsPtr params,
                         DidPrintDocumentCallback callback) override;
   void DidShowPrintDialog() override;

diff --git a/components/printing/common/print.mojom b/components/printing/common/print.mojom
@@ -322,9 +322,6 @@ interface PrintManagerHost {
   // rendered pages according to the specified settings.
   DidGetPrintedPagesCount(int32 cookie, uint32 number_pages);
 
-  // Sends the document cookie of the current printer query to the browser.
-  DidGetDocumentCookie(int32 cookie);
-
   // Request the default print settings.
   [Sync]
   GetDefaultPrintSettings() => (PrintParams default_settings);

diff --git a/components/printing/renderer/print_render_frame_helper.cc b/components/printing/renderer/print_render_frame_helper.cc
@@ -2884,7 +2884,6 @@ void PrintRenderFrameHelper::PrintPreviewContext::CalculatePluginAttributes() {
 void PrintRenderFrameHelper::SetPrintPagesParams(
     const mojom::PrintPagesParams& settings) {
   print_pages_params_ = settings.Clone();
-  GetPrintManagerHost()->DidGetDocumentCookie(settings.params->document_cookie);
 }
 
 void PrintRenderFrameHelper::QuitActiveRunLoops() {

diff --git a/components/printing/test/print_render_frame_helper_browsertest.cc b/components/printing/test/print_render_frame_helper_browsertest.cc
@@ -297,7 +297,6 @@ class TestPrintManagerHost
       EXPECT_EQ(number_pages, number_pages_);
     printer_->SetPrintedPagesCount(cookie, number_pages);
   }
-  void DidGetDocumentCookie(int32_t cookie) override {}
   void DidPrintDocument(mojom::DidPrintDocumentParamsPtr params,
                         DidPrintDocumentCallback callback) override {
     base::RunLoop().RunUntilIdle();

diff --git a/headless/lib/browser/headless_print_manager.cc b/headless/lib/browser/headless_print_manager.cc
@@ -166,6 +166,7 @@ void HeadlessPrintManager::GetPDFContents(content::RenderFrameHost* rfh,
   print_params_ = GetPrintParamsFromSettings(settings);
   page_ranges_text_ = settings.page_ranges;
   ignore_invalid_page_ranges_ = settings.ignore_invalid_page_ranges;
+  set_cookie(print_params_->params->document_cookie);
   GetPrintRenderFrame(rfh)->PrintRequestedPages();
 }