Fix PreviewModeClient::ProposeDocumentLayout() DCHECK

crrev.com/700518 incorrectly assumed
PreviewModeClient::ProposeDocumentLayout() could not be reached. In
fact, the PreviewModeClient is used only when previewing non-PDF
documents with more than 1 page.

This change also adds a browser test to check the behavior of the print
preview dialog when printing multiple pages: Current print preview tests
only generate a single page, which fails to exercise this issue.

The crash occurs after the print preview is "ready" (all pages have been
rendered), but before the print preview dialog has "loaded" completely.
Current tests only wait for the "ready" condition, but the new test must
wait for the "loaded" condition. Since fully loading is slow, most tests
continue waiting only for the "ready" condition.

Bug: 885110
Change-Id: I20316fd658d5a125a13f08d07ced4dd79884fac7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1835211
Auto-Submit: K Moon <kmoon@chromium.org>
Commit-Queue: K Moon <kmoon@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#702190}

chrome/browser/printing/print_browsertest.cc

@@ -7,6 +7,7 @@
 #include "base/auto_reset.h"
 #include "base/bind.h"
 #include "base/files/file_path.h"
+#include "base/optional.h"
 #include "base/path_service.h"
 #include "base/run_loop.h"
 #include "base/strings/stringprintf.h"
@@ -46,7 +47,12 @@ constexpr int kDefaultDocumentCookie = 1234;
 
 class PrintPreviewObserver : PrintPreviewUI::TestDelegate {
  public:
-  PrintPreviewObserver() { PrintPreviewUI::SetDelegateForTesting(this); }
+  explicit PrintPreviewObserver(bool wait_for_loaded) {
+    if (wait_for_loaded)
+      queue_.emplace();  // DOMMessageQueue doesn't allow assignment
+    PrintPreviewUI::SetDelegateForTesting(this);
+  }
+
   ~PrintPreviewObserver() override {
     PrintPreviewUI::SetDelegateForTesting(nullptr);
   }
@@ -58,6 +64,12 @@ class PrintPreviewObserver : PrintPreviewUI::TestDelegate {
     base::RunLoop run_loop;
     base::AutoReset<base::RunLoop*> auto_reset(&run_loop_, &run_loop);
     run_loop.Run();
+
+    if (queue_.has_value()) {
+      std::string message;
+      EXPECT_TRUE(queue_->WaitForMessage(&message));
+      EXPECT_EQ("\"success\"", message);
+    }
   }
 
  private:
@@ -72,9 +84,20 @@ class PrintPreviewObserver : PrintPreviewUI::TestDelegate {
     CHECK(rendered_page_count_ <= total_page_count_);
     if (rendered_page_count_ == total_page_count_ && run_loop_) {
       run_loop_->Quit();
+
+      if (queue_.has_value()) {
+        content::ExecuteScriptAsync(
+            preview_dialog,
+            "window.addEventListener('message', event => {"
+            "  if (event.data.type === 'documentLoaded') {"
+            "    domAutomationController.send(event.data.load_state);"
+            "  }"
+            "});");
+      }
     }
   }
 
+  base::Optional<content::DOMMessageQueue> queue_;
   int total_page_count_ = 1;
   int rendered_page_count_ = 0;
   base::RunLoop* run_loop_ = nullptr;
@@ -192,7 +215,17 @@ class PrintBrowserTest : public InProcessBrowserTest {
   }
 
   void PrintAndWaitUntilPreviewIsReady(bool print_only_selection) {
-    PrintPreviewObserver print_preview_observer;
+    PrintPreviewObserver print_preview_observer(/*wait_for_loaded=*/false);
+
+    StartPrint(browser()->tab_strip_model()->GetActiveWebContents(),
+               /*print_renderer=*/nullptr,
+               /*print_preview_disabled=*/false, print_only_selection);
+
+    print_preview_observer.WaitUntilPreviewIsReady();
+  }
+
+  void PrintAndWaitUntilPreviewIsReadyAndLoaded(bool print_only_selection) {
+    PrintPreviewObserver print_preview_observer(/*wait_for_loaded=*/true);
 
     StartPrint(browser()->tab_strip_model()->GetActiveWebContents(),
                /*print_renderer=*/nullptr,
@@ -279,7 +312,7 @@ class PrintExtensionBrowserTest : public extensions::ExtensionBrowserTest {
   ~PrintExtensionBrowserTest() override = default;
 
   void PrintAndWaitUntilPreviewIsReady(bool print_only_selection) {
-    PrintPreviewObserver print_preview_observer;
+    PrintPreviewObserver print_preview_observer(/*wait_for_loaded=*/false);
 
     StartPrint(browser()->tab_strip_model()->GetActiveWebContents(),
                /*print_renderer=*/nullptr,
@@ -611,4 +644,20 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessPrintBrowserTest, PrintNup) {
   PrintAndWaitUntilPreviewIsReady(/*print_only_selection=*/false);
 }
 
+IN_PROC_BROWSER_TEST_F(PrintBrowserTest, MultipagePrint) {
+  ASSERT_TRUE(embedded_test_server()->Started());
+  GURL url(embedded_test_server()->GetURL("/printing/multipage.html"));
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  PrintAndWaitUntilPreviewIsReadyAndLoaded(/*print_only_selection=*/false);
+}
+
+IN_PROC_BROWSER_TEST_F(SitePerProcessPrintBrowserTest, MultipagePrint) {
+  ASSERT_TRUE(embedded_test_server()->Started());
+  GURL url(embedded_test_server()->GetURL("/printing/multipage.html"));
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  PrintAndWaitUntilPreviewIsReadyAndLoaded(/*print_only_selection=*/false);
+}
+
 }  // namespace printing

chrome/test/data/printing/multipage.html

+<!doctype html>
+<meta charset="utf-8">
+<title>Print multiple pages</title>
+<style>
+@media print {
+  p {
+    break-before: page;
+  }
+}
+</style>
+<p>Page 1
+<p>Page 2
+<p>Page 3

pdf/preview_mode_client.cc

@@ -14,7 +14,8 @@ namespace chrome_pdf {
 PreviewModeClient::PreviewModeClient(Client* client) : client_(client) {}
 
 void PreviewModeClient::ProposeDocumentLayout(const DocumentLayout& layout) {
-  NOTREACHED();
+  // This will be invoked if the PreviewModeClient is used, which currently
+  // occurs if and only if loading a non-PDF document with more than 1 page.
 }
 
 void PreviewModeClient::Invalidate(const pp::Rect& rect) {