Fix WindowdotPrintShouldBringUpPrintPreview

chrome/browser/apps/app_browsertest.cc

Index: chrome/browser/apps/app_browsertest.cc
diff --git a/chrome/browser/apps/app_browsertest.cc b/chrome/browser/apps/app_browsertest.cc
index 823407a55ff5a8ba33608c39f08053874a82dd43..8cacce4d6942d75caf9410e86254912ec5ca75ca 100644
--- a/chrome/browser/apps/app_browsertest.cc
+++ b/chrome/browser/apps/app_browsertest.cc
@@ -147,17 +147,16 @@ class ScopedPreviewTestingDelegate : PrintPreviewUI::TestingDelegate {
     dialog_size_ = preview_dialog->GetContainerBounds().size();
     ++rendered_page_count_;
     CHECK(rendered_page_count_ <= total_page_count_);
-    if (waiting_runner_.get() && rendered_page_count_ == total_page_count_) {
-      waiting_runner_->Quit();
+    if (rendered_page_count_ == total_page_count_ && run_loop_)  {
+      run_loop_->Quit();
     }
   }
 
   void WaitUntilPreviewIsReady() {
-    CHECK(!waiting_runner_.get());
+    CHECK(!run_loop_);

[tapted, 2017/02/09 01:10:02]

This is no longer being cleared after removing `waiting_runner_ = NULL;` - was
that intentional?. I think a neater way to do this is to declare run_loop_ as a
raw pointer.

base::RunLoop* run_loop_ = nullptr;

then this method is

void WaitUntilPreviewIsReady() {
  if (rendered_page_count_ >= total_page_count_)
    return;

  base::RunLoop run_loop;
  base::AutoReset<RunLoop> auto_reset(&run_loop_, &run_loop);
  run_loop.Run();
}


(this restructure is an optional nit though - I don't have strong feelings -
that's just how I'd write it ;)

[rbpotter, 2017/02/09 01:56:40]

Done. Thanks for the suggestion.

     if (rendered_page_count_ < total_page_count_) {
-      waiting_runner_ = new content::MessageLoopRunner;
-      waiting_runner_->Run();
-      waiting_runner_ = NULL;
+      run_loop_ = base::MakeUnique<base::RunLoop>();
+      run_loop_->Run();
     }
   }
 
@@ -169,7 +168,7 @@ class ScopedPreviewTestingDelegate : PrintPreviewUI::TestingDelegate {
   bool auto_cancel_;
   int total_page_count_;
   int rendered_page_count_;
-  scoped_refptr<content::MessageLoopRunner> waiting_runner_;
+  std::unique_ptr<base::RunLoop> run_loop_;
   gfx::Size dialog_size_;
 };
 
@@ -1138,17 +1137,9 @@ IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, DISABLED_WebContentsHasFocus) {
 
 #if BUILDFLAG(ENABLE_PRINT_PREVIEW)
 
-#if defined(OS_WIN) || defined(OS_LINUX) || defined(OS_MACOSX)
-#define MAYBE_WindowDotPrintShouldBringUpPrintPreview \
-    DISABLED_WindowDotPrintShouldBringUpPrintPreview
-#else
-#define MAYBE_WindowDotPrintShouldBringUpPrintPreview \
-    WindowDotPrintShouldBringUpPrintPreview
-#endif
-
 IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest,
-                       MAYBE_WindowDotPrintShouldBringUpPrintPreview) {
-  ScopedPreviewTestingDelegate preview_delegate(true);
+                       WindowDotPrintShouldBringUpPrintPreview) {
+  ScopedPreviewTestingDelegate preview_delegate(false);

[tapted, 2017/02/09 01:10:02]

in the bug you say this was the only test passing |true|, so can the bool
argument just be removed?

[rbpotter, 2017/02/09 01:56:40]

Done.

   ASSERT_TRUE(RunPlatformAppTest("platform_apps/print_api")) << message_;
   preview_delegate.WaitUntilPreviewIsReady();
 }