Narrow the scope of page pause when printing.

In the current implementation, sometimes the scope of a page's "pause"
is too wide compared to the definition in the spec [1], and sometimes it
does not pause at all. So narrow the scope to better conform to the
spec, and apply it consistently to all possible ways users can print.

This is based on an earlier CL [2] which only did this for printing with
the system dialog, but not for Print Preview. Some of the trickier cases
handled, which require unpausing, are:
- Navigations while previewing.
- Printing selections, which require a navigation.

[1] https://html.spec.whatwg.org/C/#printing-steps
[2] https://crrev.com/c/1669250

Bug: 956832
Change-Id: I9bf274fcfb70eefe896fbbae9ce13a88b6f4dfc0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2338819Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797120}

components/printing/renderer/print_render_frame_helper.cc

@@ -47,6 +47,7 @@
 #include "third_party/blink/public/platform/platform.h"
 #include "third_party/blink/public/platform/web_data.h"
 #include "third_party/blink/public/platform/web_double_size.h"
+#include "third_party/blink/public/platform/web_scoped_page_pauser.h"
 #include "third_party/blink/public/platform/web_size.h"
 #include "third_party/blink/public/platform/web_url.h"
 #include "third_party/blink/public/platform/web_url_request.h"
@@ -1112,6 +1113,11 @@ void PrintRenderFrameHelper::DidStartNavigation(
     const GURL& url,
     base::Optional<blink::WebNavigationType> navigation_type) {
   is_loading_ = true;
+
+  // If the renderer navigates while paused, unpause to let the navigation
+  // proceed.
+  if (print_preview_context_.IsPaused())
+    auto pauser = print_preview_context_.TakePauser();
 }
 
 void PrintRenderFrameHelper::DidFailProvisionalLoad() {
@@ -1146,7 +1152,12 @@ void PrintRenderFrameHelper::ScriptedPrint(bool user_initiated) {
     web_frame->DispatchBeforePrintEvent();
     if (!weak_this)
       return;
-    Print(web_frame, blink::WebNode(), PrintRequestType::kScripted);
+
+    // Pause between onbeforeprint and onafterprint events.
+    // https://html.spec.whatwg.org/C/#printing-steps
+    Print(web_frame, blink::WebNode(), PrintRequestType::kScripted,
+          blink::WebScopedPagePauser::Create());
+
     if (weak_this)
       web_frame->DispatchAfterPrintEvent();
   }
@@ -1181,7 +1192,12 @@ void PrintRenderFrameHelper::PrintRequestedPages() {
   // If we are printing a PDF extension frame, find the plugin node and print
   // that instead.
   auto plugin = delegate_->GetPdfElement(frame);
-  Print(frame, plugin, PrintRequestType::kRegular);
+
+  // Pause between onbeforeprint and onafterprint events.
+  // https://html.spec.whatwg.org/C/#printing-steps
+  Print(frame, plugin, PrintRequestType::kRegular,
+        blink::WebScopedPagePauser::Create());
+
   if (!render_frame_gone_)
     frame->DispatchAfterPrintEvent();
   // WARNING: |this| may be gone at this point. Do not do any more work here and
@@ -1197,10 +1213,13 @@ void PrintRenderFrameHelper::PrintForSystemDialog() {
     NOTREACHED();
     return;
   }
-  Print(frame, print_preview_context_.source_node(),
-        PrintRequestType::kRegular);
+
+  // Hand over control, including Print Preview's WebScopedPagePauser, to the
+  // system print dialog.
+  Print(frame, print_preview_context_.source_node(), PrintRequestType::kRegular,
+        print_preview_context_.TakePauser());
   if (!render_frame_gone_)
-    frame->DispatchAfterPrintEvent();
+    print_preview_context_.DispatchAfterPrintEvent();
   // WARNING: |this| may be gone at this point. Do not do any more work here and
   // just return.
 }
@@ -1295,7 +1314,7 @@ void PrintRenderFrameHelper::PrintPreview(base::Value settings) {
 
 void PrintRenderFrameHelper::OnPrintPreviewDialogClosed() {
   ScopedIPC scoped_ipc(weak_ptr_factory_.GetWeakPtr());
-  print_preview_context_.source_frame()->DispatchAfterPrintEvent();
+  print_preview_context_.DispatchAfterPrintEvent();
 }
 #endif  // BUILDFLAG(ENABLE_PRINT_PREVIEW)
 
@@ -1345,6 +1364,9 @@ void PrintRenderFrameHelper::PrintFrameContent(
   // try to handle pdf plugin element until that bug is fixed.
   {
     TRACE_EVENT0("print", "PrintRenderFrameHelper::PrintFrameContent");
+    // Pause between onbeforeprint and onafterprint events.
+    // https://html.spec.whatwg.org/C/#printing-steps
+    auto page_pauser = blink::WebScopedPagePauser::Create();
     if (frame->PrintBegin(web_print_params,
                           /*constrain_to_node=*/blink::WebElement())) {
       frame->PrintPage(0, canvas);
@@ -1442,6 +1464,12 @@ void PrintRenderFrameHelper::PrepareFrameForPreviewDocument() {
   prep_frame_view_ = std::make_unique<PrepareFrameAndViewForPrint>(
       print_params, print_preview_context_.source_frame(),
       print_preview_context_.source_node(), ignore_css_margins_);
+
+  // The renderer needs to load a new page to print the selection. Must unpause
+  // to do that.
+  if (print_params.selection_only)
+    auto pauser_to_destroy = print_preview_context_.TakePauser();
+
   prep_frame_view_->CopySelectionIfNeeded(
       render_frame()->GetWebkitPreferences(),
       base::BindOnce(&PrintRenderFrameHelper::OnFramePreparedForPreviewDocument,
@@ -1453,7 +1481,16 @@ void PrintRenderFrameHelper::OnFramePreparedForPreviewDocument() {
     PrepareFrameForPreviewDocument();
     return;
   }
+
   CreatePreviewDocumentResult result = CreatePreviewDocument();
+
+  // Now that the renderer has finished generating the print preview for a
+  // selection, pause again. The system print dialog path in Print() does not
+  // need to do this, since that dialog is done, whereas the Print Preview
+  // dialog will continue to generate more previews.
+  if (print_pages_params_->params.selection_only)
+    print_preview_context_.Pause();
+
   if (result != CREATE_IN_PROGRESS)
     DidFinishPrinting(result == CREATE_SUCCESS ? OK : FAIL_PREVIEW);
 }
@@ -1779,7 +1816,7 @@ void PrintRenderFrameHelper::PrintNode(const blink::WebNode& node) {
       return;
 
     Print(duplicate_node.GetDocument().GetFrame(), duplicate_node,
-          PrintRequestType::kRegular);
+          PrintRequestType::kRegular, blink::WebScopedPagePauser::Create());
     // Check if |this| is still valid.
     if (!weak_this)
       return;
@@ -1792,9 +1829,11 @@ void PrintRenderFrameHelper::PrintNode(const blink::WebNode& node) {
   print_node_in_progress_ = false;
 }
 
-void PrintRenderFrameHelper::Print(blink::WebLocalFrame* frame,
-                                   const blink::WebNode& node,
-                                   PrintRequestType print_request_type) {
+void PrintRenderFrameHelper::Print(
+    blink::WebLocalFrame* frame,
+    const blink::WebNode& node,
+    PrintRequestType print_request_type,
+    std::unique_ptr<blink::WebScopedPagePauser> pauser) {
   // If still not finished with earlier print request simply ignore.
   if (prep_frame_view_)
     return;
@@ -1840,6 +1879,11 @@ void PrintRenderFrameHelper::Print(blink::WebLocalFrame* frame,
     }
   }
 
+  // The renderer needs to load a new page to print the selection. Must unpause
+  // to do that.
+  if (print_pages_params_->params.selection_only)
+    pauser.reset();
+
   // Render Pages for printing.
   if (!RenderPagesForPrint(frame_ref.GetFrame(), node)) {
     LOG(ERROR) << "RenderPagesForPrint failed";
@@ -2331,9 +2375,10 @@ void PrintRenderFrameHelper::ShowScriptedPrintPreview() {
 
 void PrintRenderFrameHelper::RequestPrintPreview(PrintPreviewRequestType type) {
   auto weak_this = weak_ptr_factory_.GetWeakPtr();
-  print_preview_context_.source_frame()->DispatchBeforePrintEvent();
+  print_preview_context_.DispatchBeforePrintEvent(weak_this);
   if (!weak_this)
     return;
+
   const bool is_from_arc = print_preview_context_.IsForArc();
   const bool is_modifiable = print_preview_context_.IsModifiable();
   const bool is_pdf = print_preview_context_.IsPdf();
@@ -2491,7 +2536,12 @@ void PrintRenderFrameHelper::OnPreviewDisconnect() {
 
 PrintRenderFrameHelper::PrintPreviewContext::PrintPreviewContext() = default;
 
-PrintRenderFrameHelper::PrintPreviewContext::~PrintPreviewContext() = default;
+PrintRenderFrameHelper::PrintPreviewContext::~PrintPreviewContext() {
+  // Make sure |pauser_| is null. If |pauser_| still exists, it will try to
+  // unpause during teardown, and that is too late.
+  // DispatchAfterPrintEvent() or TakePauser() should have destroyed it already.
+  DCHECK(!pauser_);
+}
 
 void PrintRenderFrameHelper::PrintPreviewContext::InitWithFrame(
     blink::WebLocalFrame* web_frame) {
@@ -2514,6 +2564,38 @@ void PrintRenderFrameHelper::PrintPreviewContext::InitWithNode(
   CalculatePluginAttributes();
 }
 
+bool PrintRenderFrameHelper::PrintPreviewContext::IsPaused() const {
+  return !!pauser_;
+}
+
+void PrintRenderFrameHelper::PrintPreviewContext::Pause() {
+  DCHECK(!pauser_);
+  pauser_ = blink::WebScopedPagePauser::Create();
+}
+
+std::unique_ptr<blink::WebScopedPagePauser>
+PrintRenderFrameHelper::PrintPreviewContext::TakePauser() {
+  DCHECK(pauser_);
+  return std::move(pauser_);
+}
+
+void PrintRenderFrameHelper::PrintPreviewContext::DispatchBeforePrintEvent(
+    base::WeakPtr<PrintRenderFrameHelper> weak_this) {
+  source_frame()->DispatchBeforePrintEvent();
+  if (!weak_this)
+    return;
+
+  DCHECK(!pauser_);
+  pauser_ = blink::WebScopedPagePauser::Create();
+}
+
+void PrintRenderFrameHelper::PrintPreviewContext::DispatchAfterPrintEvent() {
+  // No DCHECK(pauser_), as |pauser_| may have been reset by TakePauser().
+  pauser_.reset();
+
+  source_frame()->DispatchAfterPrintEvent();
+}
+
 void PrintRenderFrameHelper::PrintPreviewContext::OnPrintPreview() {
   DCHECK_EQ(INITIALIZED, state_);
   ClearContext();

components/printing/renderer/print_render_frame_helper.h

@@ -50,8 +50,9 @@ class DictionaryValue;
 
 namespace blink {
 class WebLocalFrame;
+class WebScopedPagePauser;
 class WebView;
-}
+}  // namespace blink
 
 namespace content {
 class AXTreeSnapshotter;
@@ -291,7 +292,8 @@ class PrintRenderFrameHelper
   // WARNING: |this| may be gone after this method returns.
   void Print(blink::WebLocalFrame* frame,
              const blink::WebNode& node,
-             PrintRequestType print_request_type);
+             PrintRequestType print_request_type,
+             std::unique_ptr<blink::WebScopedPagePauser> pauser);
 
   // Notification when printing is done - signal tear-down/free resources.
   void DidFinishPrinting(PrintingResult result);
@@ -473,6 +475,17 @@ class PrintRenderFrameHelper
     void InitWithFrame(blink::WebLocalFrame* web_frame);
     void InitWithNode(const blink::WebNode& web_node);
 
+    // Manual control of pausing/unpausing for special situations.
+    bool IsPaused() const;
+    void Pause();
+    std::unique_ptr<blink::WebScopedPagePauser> TakePauser();
+
+    // Dispatchs onbeforeprint/onafterprint events. Use these instead of calling
+    // the WebLocalFrame version on source_frame().
+    void DispatchBeforePrintEvent(
+        base::WeakPtr<PrintRenderFrameHelper> weak_this);
+    void DispatchAfterPrintEvent();
+
     // Does bookkeeping at the beginning of print preview.
     void OnPrintPreview();
 
@@ -558,6 +571,10 @@ class PrintRenderFrameHelper
 
     std::unique_ptr<PrepareFrameAndViewForPrint> prep_frame_view_;
 
+    // Manages when to pause between onbeforeprint and onafterprint events.
+    // https://html.spec.whatwg.org/C/#printing-steps
+    std::unique_ptr<blink::WebScopedPagePauser> pauser_;
+
     // The typefaces encountered in the content during document serialization.
     ContentProxySet typeface_content_info_;
 

third_party/blink/renderer/core/page/chrome_client.cc

@@ -292,11 +292,6 @@ bool ChromeClient::Print(LocalFrame* frame) {
     return false;
   }
 
-  // Suspend pages in case the client method runs a new event loop that would
-  // otherwise cause the load to continue while we're in the middle of
-  // executing JavaScript.
-  ScopedPagePauser pauser;
-
   PrintDelegate(frame);
   return true;
 }