Fix WindowdotPrintShouldBringUpPrintPreview

chrome/browser/apps/app_browsertest.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <stddef.h>
 
 #include <memory>
 
 #include "apps/launcher.h"
+#include "base/auto_reset.h"
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/macros.h"
 #include "base/stl_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/apps/app_browsertest_util.h"
@@ skipping 15 matching lines @@
 #include "chrome/test/base/test_switches.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/prefs/pref_service.h"
 #include "components/web_modal/web_contents_modal_dialog_manager.h"
 #include "content/public/browser/devtools_agent_host.h"
 #include "content/public/browser/host_zoom_map.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_widget_host_view.h"
 #include "content/public/test/browser_test_utils.h"
-#include "content/public/test/test_utils.h"
 #include "extensions/browser/app_window/app_window.h"
 #include "extensions/browser/app_window/app_window_registry.h"
 #include "extensions/browser/app_window/native_app_window.h"
 #include "extensions/browser/event_router.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_registry_observer.h"
 #include "extensions/browser/pref_names.h"
 #include "extensions/common/api/app_runtime.h"
 #include "extensions/common/constants.h"
@@ skipping 60 matching lines @@
  private:
   size_t observations_;
   std::vector<content::WebContents*> observed_tabs_;
 
   DISALLOW_COPY_AND_ASSIGN(TabsAddedNotificationObserver);
 };
 
 #if BUILDFLAG(ENABLE_PRINT_PREVIEW)
 class ScopedPreviewTestingDelegate : PrintPreviewUI::TestingDelegate {
  public:
-  explicit ScopedPreviewTestingDelegate(bool auto_cancel)
-      : auto_cancel_(auto_cancel),
-        total_page_count_(1),
+  ScopedPreviewTestingDelegate()
+      : total_page_count_(1),
         rendered_page_count_(0) {
     PrintPreviewUI::SetDelegateForTesting(this);
   }
 
   ~ScopedPreviewTestingDelegate() {
     PrintPreviewUI::SetDelegateForTesting(NULL);
   }
 
   // PrintPreviewUI::TestingDelegate implementation.
-  bool IsAutoCancelEnabled() override { return auto_cancel_; }
-
-  // PrintPreviewUI::TestingDelegate implementation.
   void DidGetPreviewPageCount(int page_count) override {
     total_page_count_ = page_count;
   }
 
   // PrintPreviewUI::TestingDelegate implementation.
   void DidRenderPreviewPage(content::WebContents* preview_dialog) override {
     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());
-    if (rendered_page_count_ < total_page_count_) {
-      waiting_runner_ = new content::MessageLoopRunner;
-      waiting_runner_->Run();
-      waiting_runner_ = NULL;
-    }
+    if (rendered_page_count_ >= total_page_count_)
+      return;
+
+    base::RunLoop run_loop;
+    base::AutoReset<base::RunLoop*> auto_reset(&run_loop_, &run_loop);
+    run_loop.Run();
   }
 
   gfx::Size dialog_size() {
     return dialog_size_;
   }
 
  private:
-  bool auto_cancel_;
   int total_page_count_;

[tapted, 2017/02/09 02:03:02]

nit: move the = 1 down here (and = 0 below) rather than doing some stuff in the
constructor

[rbpotter, 2017/02/09 20:11:46]

Done.

   int rendered_page_count_;
-  scoped_refptr<content::MessageLoopRunner> waiting_runner_;
+  base::RunLoop* run_loop_ = nullptr;
   gfx::Size dialog_size_;
 };
 
 #endif  // ENABLE_PRINT_PREVIEW
 
 #if !defined(OS_CHROMEOS) && !defined(OS_WIN)
 bool CopyTestDataAndGetTestFilePath(
     const base::FilePath& test_data_file,
     const base::FilePath& temp_dir,
     const char* filename,
@@ skipping 948 matching lines @@
 
   EXPECT_EQ(1LU, GetAppWindowCount());
   EXPECT_TRUE(GetFirstAppWindow()
                   ->web_contents()
                   ->GetRenderWidgetHostView()
                   ->HasFocus());
 }
 
 #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;
   ASSERT_TRUE(RunPlatformAppTest("platform_apps/print_api")) << message_;
   preview_delegate.WaitUntilPreviewIsReady();
 }
 
 // This test verifies that http://crbug.com/297179 is fixed.
 IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest,
                        DISABLED_ClosingWindowWhilePrintingShouldNotCrash) {
-  ScopedPreviewTestingDelegate preview_delegate(false);
+  ScopedPreviewTestingDelegate preview_delegate;
   ASSERT_TRUE(RunPlatformAppTest("platform_apps/print_api")) << message_;
   preview_delegate.WaitUntilPreviewIsReady();
   GetFirstAppWindow()->GetBaseWindow()->Close();
 }
 
 // This test currently only passes on OS X (on other platforms the print preview
 // dialog's size is limited by the size of the window being printed).
 #if !defined(OS_MACOSX)
 #define MAYBE_PrintPreviewShouldNotBeTooSmall \
     DISABLED_PrintPreviewShouldNotBeTooSmall
 #else
 #define MAYBE_PrintPreviewShouldNotBeTooSmall \
     PrintPreviewShouldNotBeTooSmall
 #endif
 
 IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest,
                        MAYBE_PrintPreviewShouldNotBeTooSmall) {
   // Print preview dialogs with widths less than 410 pixels will have preview
   // areas that are too small, and ones with heights less than 191 pixels will
   // have vertical scrollers for their controls that are too small.
   gfx::Size minimum_dialog_size(410, 191);
-  ScopedPreviewTestingDelegate preview_delegate(false);
+  ScopedPreviewTestingDelegate preview_delegate;
   ASSERT_TRUE(RunPlatformAppTest("platform_apps/print_api")) << message_;
   preview_delegate.WaitUntilPreviewIsReady();
   EXPECT_GE(preview_delegate.dialog_size().width(),
             minimum_dialog_size.width());
   EXPECT_GE(preview_delegate.dialog_size().height(),
             minimum_dialog_size.height());
   GetFirstAppWindow()->GetBaseWindow()->Close();
 }
 #endif  // ENABLE_PRINT_PREVIEW
 
@@ skipping 187 matching lines @@
 // https://crbug.com/620194.
 #define MAYBE_AppWindowIframe DISABLED_AppWindowIframe
 // Sends chrome.test.sendMessage from chrome.app.window.create's callback.
 // The app window also adds an <iframe> to the page during window.onload.
 IN_PROC_BROWSER_TEST_F(PlatformAppBrowserTest, MAYBE_AppWindowIframe) {
   LoadAndLaunchPlatformApp("app_window_send_message",
                            "APP_WINDOW_CREATE_CALLBACK");
 }
 
 }  // namespace extensions