Assertion removed in print_preview_pdf_generated.cc.

chrome/browser/printing/print_preview_pdf_generated_browsertest.cc

Index: chrome/browser/printing/print_preview_pdf_generated_browsertest.cc
diff --git a/chrome/browser/printing/print_preview_pdf_generated_browsertest.cc b/chrome/browser/printing/print_preview_pdf_generated_browsertest.cc
index 8977fc240107fb38536856cfa489998ec16d083d..553edabcce3504c7a1c63eacc2670c3038980d6f 100644
--- a/chrome/browser/printing/print_preview_pdf_generated_browsertest.cc
+++ b/chrome/browser/printing/print_preview_pdf_generated_browsertest.cc
@@ -11,6 +11,8 @@
 #include <utility>
 #include <vector>
 
+#include "base/bind.h"
+#include "base/callback.h"
 #include "base/file_util.h"
 #include "base/files/file.h"
 #include "base/files/file_path.h"
@@ -107,11 +109,14 @@ struct PrintPreviewSettings {
 // change the settings of the preview dialog.
 class PrintPreviewObserver : public WebContentsObserver {
  public:
-  PrintPreviewObserver(Browser* browser, WebContents* dialog)
+  PrintPreviewObserver(Browser* browser,
+                       WebContents* dialog,
+                       const base::FilePath& pdf_file_save_path)
       : WebContentsObserver(dialog),
         browser_(browser),
         state_(kWaitingToSendSaveAsPdf),
-        failed_setting_("None") {}
+        failed_setting_("None"),
+        pdf_file_save_path_(pdf_file_save_path) {}
 
   virtual ~PrintPreviewObserver() {}
 
@@ -184,7 +189,12 @@ class PrintPreviewObserver : public WebContentsObserver {
       state_ = kWaitingForFinalMessage;
       failed_setting_ = "Margins";
     } else if (state_ == kWaitingForFinalMessage) {
-      EndLoop();
+      // Called by PrintPreviewMessageHandler when the PDF is saved. This ends
+      // the message loop for |this|.
+      base::Closure end_loop_callback =
+          base::Bind(&PrintPreviewObserver::EndLoop, base::Unretained(this));
+      GetUI()->SetPdfSavedCallbackForTesting(end_loop_callback);
+      GetUI()->SetSelectedFileForTesting(pdf_file_save_path_);
       return;
     }
 
@@ -272,10 +282,6 @@ class PrintPreviewObserver : public WebContentsObserver {
     Observe(new_web_contents);
   }
 
-  virtual void WebContentsDestroyed() OVERRIDE {
-    EndLoop();
-  }
-
   Browser* browser_;
   base::Closure quit_closure_;
   scoped_ptr<PrintPreviewSettings> settings_;
@@ -285,6 +291,7 @@ class PrintPreviewObserver : public WebContentsObserver {
   // ManipulatePreviewSettings() on the observer.
   State state_;
   std::string failed_setting_;
+  base::FilePath pdf_file_save_path_;
 
   DISALLOW_COPY_AND_ASSIGN(PrintPreviewObserver);
 };
@@ -295,8 +302,8 @@ class PrintPreviewPdfGeneratedBrowserTest : public InProcessBrowserTest {
   virtual ~PrintPreviewPdfGeneratedBrowserTest() {}
 
   // Navigates to the given web page, then initiates print preview and waits
-  // for all the settings to be set.
-  void NavigateAndPreview(const base::FilePath::StringType& file_name,
+  // for all the settings to be set, then the webpage is printed to a PDF.

[Lei Zhang, 2014/07/22 03:04:39]

... then save the preview to PDF.

[ivandavid, 2014/07/22 19:02:00]

Done.

+  void NavigateAndPrint(const base::FilePath::StringType& file_name,
                           const PrintPreviewSettings& settings) {
     print_preview_observer_->SetPrintPreviewSettings(settings);
     base::FilePath path(file_name);
@@ -310,31 +317,6 @@ class PrintPreviewPdfGeneratedBrowserTest : public InProcessBrowserTest {
     loop.Run();
   }
 
-  // Prints the web page to a PDF. NavigateAndPreview must be called first.
-  void Print() {
-    ASSERT_FALSE(pdf_file_save_path_.empty());
-
-    base::RunLoop loop;
-    print_preview_observer_->set_quit_closure(loop.QuitClosure());
-    print_preview_observer_->GetUI()->SetSelectedFileForTesting(
-        pdf_file_save_path_);
-    loop.Run();
-
-    // Checks to see if the file exists and is readable. If the file doesn't
-    // exist, the test will fail. If it exists, but isn't readable, the test
-    // will keep polling until the file becomes readable. This is due to a
-    // problem on Windows where the file exists, but isn't readable, causing
-    // other ASSERT statements in this test to fail.
-    // TODO(ivandavid): Come up with a better way to do this.
-    ASSERT_TRUE(base::PathExists(pdf_file_save_path_));
-    while (true) {
-      base::File pdf_file(
-          pdf_file_save_path_, base::File::FLAG_OPEN | base::File::FLAG_READ);
-      if (pdf_file.IsValid())
-        break;
-    }
-  }
-
   // Initializes function pointers from the PDF library. Called once when the
   // test starts. The library is closed when the browser test ends.
   void InitPdfFunctions() {
@@ -495,7 +477,8 @@ class PrintPreviewPdfGeneratedBrowserTest : public InProcessBrowserTest {
         browser()->tab_strip_model()->GetActiveWebContents();
     ASSERT_TRUE(tab);
 
-    print_preview_observer_.reset(new PrintPreviewObserver(browser(), tab));
+    print_preview_observer_.reset(
+        new PrintPreviewObserver(browser(), tab, pdf_file_save_path_));
     chrome::DuplicateTab(browser());
 
     WebContents* initiator =
@@ -544,6 +527,7 @@ class PrintPreviewPdfGeneratedBrowserTest : public InProcessBrowserTest {
 
     pdf_file_save_path_ =
         tmp_dir_.path().Append(FILE_PATH_LITERAL("dummy.pdf"));
+    ASSERT_FALSE(pdf_file_save_path_.empty());

[Lei Zhang, 2014/07/22 03:04:39]

This can't be false, you just appended to it one line above. Rather,
PrintPreviewObserver(), which is the consumer of |pdf_file_save_path_| should
sanity check this.

[ivandavid, 2014/07/22 19:02:00]

Done.

[ivandavid, 2014/07/22 19:02:00]

I moved this to ManipulatePreviewSettings() when |observer_| is in the
kWaitingForFinalMessage state.

 
     // Send the file path to the layout test framework so that it can
     // communicate with this browser test.
@@ -698,8 +682,7 @@ IN_PROC_BROWSER_TEST_F(PrintPreviewPdfGeneratedBrowserTest,
 
     ASSERT_GE(cmd_arguments.size(), 1U);
     base::FilePath::StringType test_name(cmd_arguments[0]);
-    NavigateAndPreview(test_name, settings);
-    Print();
+    NavigateAndPrint(test_name, settings);
     PdfToPng();
 
     // Message to the layout test framework indicating that it should start