Consolidate two WebContents destruction watchers.

There were two different watchers for WebContents destruction in two
different test helper files, in addition to lots of tests rolling their
own. Consolidate all of them.

Bug: none
Change-Id: I9b5723758aa997467689cce8767a3def9e8c13e0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2153747
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Auto-Submit: Avi Drissman <avi@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760054}

chrome/browser/printing/print_preview_dialog_controller_browsertest.cc

@@ -36,6 +36,7 @@
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/test/browser_test_utils.h"
+#include "content/public/test/test_utils.h"
 #include "ipc/ipc_message_macros.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/ui_base_switches.h"
@@ -103,23 +104,6 @@ class PrintPreviewDialogClonedObserver : public WebContentsObserver {
   DISALLOW_COPY_AND_ASSIGN(PrintPreviewDialogClonedObserver);
 };
 
-class PrintPreviewDialogDestroyedObserver : public WebContentsObserver {
- public:
-  explicit PrintPreviewDialogDestroyedObserver(WebContents* dialog)
-      : WebContentsObserver(dialog) {}
-  ~PrintPreviewDialogDestroyedObserver() override = default;
-
-  bool dialog_destroyed() const { return dialog_destroyed_; }
-
- private:
-  // content::WebContentsObserver implementation.
-  void WebContentsDestroyed() override { dialog_destroyed_ = true; }
-
-  bool dialog_destroyed_ = false;
-
-  DISALLOW_COPY_AND_ASSIGN(PrintPreviewDialogDestroyedObserver);
-};
-
 void PluginsLoadedCallback(
     base::OnceClosure quit_closure,
     const std::vector<content::WebPluginInfo>& /* info */) {
@@ -243,9 +227,9 @@ IN_PROC_BROWSER_TEST_F(PrintPreviewDialogControllerBrowserTest,
 
   // Navigate in the initiator tab. Make sure navigating destroys the print
   // preview dialog.
-  PrintPreviewDialogDestroyedObserver dialog_destroyed_observer(preview_dialog);
+  content::WebContentsDestroyedWatcher watcher(preview_dialog);
   ui_test_utils::NavigateToURL(browser(), GURL(chrome::kChromeUINewTabURL));
-  ASSERT_TRUE(dialog_destroyed_observer.dialog_destroyed());
+  ASSERT_TRUE(watcher.IsDestroyed());
 
   // Try printing again.
   PrintPreview();
@@ -278,7 +262,7 @@ IN_PROC_BROWSER_TEST_F(PrintPreviewDialogControllerBrowserTest,
 
   // Reload the initiator. Make sure reloading destroys the print preview
   // dialog.
-  PrintPreviewDialogDestroyedObserver dialog_destroyed_observer(preview_dialog);
+  content::WebContentsDestroyedWatcher watcher(preview_dialog);
   chrome::Reload(browser(), WindowOpenDisposition::CURRENT_TAB);
   content::WaitForLoadStop(
       browser()->tab_strip_model()->GetActiveWebContents());
@@ -287,7 +271,7 @@ IN_PROC_BROWSER_TEST_F(PrintPreviewDialogControllerBrowserTest,
   // may occur right after the commit, before the widget is destroyed.
   // Execute pending tasks to account for this.
   base::RunLoop().RunUntilIdle();
-  ASSERT_TRUE(dialog_destroyed_observer.dialog_destroyed());
+  ASSERT_TRUE(watcher.IsDestroyed());
 
   // Try printing again.
   PrintPreview();

chrome/browser/printing/print_preview_dialog_controller_unittest.cc

@@ -22,32 +22,17 @@
 #include "content/public/common/url_constants.h"
 #include "content/public/test/mock_render_process_host.h"
 #include "content/public/test/navigation_simulator.h"
+#include "content/public/test/test_utils.h"
 #include "content/public/test/web_contents_tester.h"
 
 using content::WebContents;
 using content::WebContentsObserver;
 
 namespace {
+
 // content::WebContentsDelegate destructor is protected: subclass for testing.
 class TestWebContentsDelegate : public content::WebContentsDelegate {};
 
-class PrintPreviewDialogDestroyedObserver : public WebContentsObserver {
- public:
-  explicit PrintPreviewDialogDestroyedObserver(WebContents* dialog)
-      : WebContentsObserver(dialog) {}
-  ~PrintPreviewDialogDestroyedObserver() override = default;
-
-  bool dialog_destroyed() const { return dialog_destroyed_; }
-
- private:
-  // content::WebContentsObserver implementation.
-  void WebContentsDestroyed() override { dialog_destroyed_ = true; }
-
-  bool dialog_destroyed_ = false;
-
-  DISALLOW_COPY_AND_ASSIGN(PrintPreviewDialogDestroyedObserver);
-};
-
 }  // namespace
 
 namespace printing {
@@ -243,7 +228,7 @@ TEST_F(PrintPreviewDialogControllerUnitTest, CloseDialogOnNavigation) {
   // still 1.
   EXPECT_EQ(1, browser()->tab_strip_model()->count());
   EXPECT_NE(web_contents, tiger_preview_dialog);
-  PrintPreviewDialogDestroyedObserver tiger_destroyed(tiger_preview_dialog);
+  content::WebContentsDestroyedWatcher tiger_destroyed(tiger_preview_dialog);
 
   // Navigate via link to a similar page.
   content::NavigationSimulator::NavigateAndCommitFromBrowser(web_contents,
@@ -261,10 +246,10 @@ TEST_F(PrintPreviewDialogControllerUnitTest, CloseDialogOnNavigation) {
 
   // Check a new dialog was created - either the pointers should be different or
   // the previous web contents must have been destroyed.
-  EXPECT_TRUE(tiger_destroyed.dialog_destroyed() ||
+  EXPECT_TRUE(tiger_destroyed.IsDestroyed() ||
               tiger_barb_preview_dialog != tiger_preview_dialog);
   EXPECT_NE(tiger_barb_preview_dialog, web_contents);
-  PrintPreviewDialogDestroyedObserver tiger_barb_destroyed(
+  content::WebContentsDestroyedWatcher tiger_barb_destroyed(
       tiger_barb_preview_dialog);
 
   // Now this returns false as |tiger_barb_preview_dialog| is open.
@@ -281,10 +266,10 @@ TEST_F(PrintPreviewDialogControllerUnitTest, CloseDialogOnNavigation) {
   ASSERT_TRUE(tiger_preview_dialog_2);
 
   // Verify this is a new dialog.
-  EXPECT_TRUE(tiger_barb_destroyed.dialog_destroyed() ||
+  EXPECT_TRUE(tiger_barb_destroyed.IsDestroyed() ||
               tiger_barb_preview_dialog != tiger_preview_dialog_2);
   EXPECT_NE(tiger_preview_dialog_2, web_contents);
-  PrintPreviewDialogDestroyedObserver tiger_2_destroyed(
+  content::WebContentsDestroyedWatcher tiger_2_destroyed(
       tiger_preview_dialog_2);
 
   // Try to simulate Gmail navigation: Navigate to an existing page (via
@@ -304,7 +289,7 @@ TEST_F(PrintPreviewDialogControllerUnitTest, CloseDialogOnNavigation) {
   // returned by GetOrCreatePreviewDialog should be the same as the earlier
   // dialog.
   EXPECT_FALSE(manager->PrintPreviewNow(web_contents->GetMainFrame(), false));
-  EXPECT_FALSE(tiger_2_destroyed.dialog_destroyed());
+  EXPECT_FALSE(tiger_2_destroyed.IsDestroyed());
   WebContents* tiger_preview_dialog_2b =
       dialog_controller->GetOrCreatePreviewDialog(web_contents);
   ASSERT_TRUE(tiger_preview_dialog_2b);

chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc

@@ -1044,29 +1044,6 @@ class ChromeLauncherControllerExtendedShelfTest
   DISALLOW_COPY_AND_ASSIGN(ChromeLauncherControllerExtendedShelfTest);
 };
 
-// Watches WebContents and blocks until it is destroyed. This is needed for
-// the destruction of a V2 application.
-class WebContentsDestroyedWatcher : public content::WebContentsObserver {
- public:
-  explicit WebContentsDestroyedWatcher(content::WebContents* web_contents)
-      : content::WebContentsObserver(web_contents),
-        message_loop_runner_(new content::MessageLoopRunner) {
-    EXPECT_TRUE(web_contents != nullptr);
-  }
-  ~WebContentsDestroyedWatcher() override {}
-
-  // Waits until the WebContents is destroyed.
-  void Wait() { message_loop_runner_->Run(); }
-
- private:
-  // Overridden WebContentsObserver methods.
-  void WebContentsDestroyed() override { message_loop_runner_->Quit(); }
-
-  scoped_refptr<content::MessageLoopRunner> message_loop_runner_;
-
-  DISALLOW_COPY_AND_ASSIGN(WebContentsDestroyedWatcher);
-};
-
 // A V1 windowed application.
 class V1App : public TestBrowserWindow {
  public:
@@ -1118,7 +1095,8 @@ class V2App {
   }
 
   virtual ~V2App() {
-    WebContentsDestroyedWatcher destroyed_watcher(window_->web_contents());
+    content::WebContentsDestroyedWatcher destroyed_watcher(
+        window_->web_contents());
     window_->GetBaseWindow()->Close();
     destroyed_watcher.Wait();
   }

chrome/browser/ui/media_router/presentation_receiver_window_controller_browsertest.cc

@@ -26,6 +26,7 @@
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/test/browser_test_utils.h"
+#include "content/public/test/test_utils.h"
 #include "extensions/browser/script_executor.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
@@ -66,24 +67,6 @@ base::FilePath GetResourceFile(base::FilePath::StringPieceType relative_path) {
   return full_path;
 }
 
-// This class waits for a WebContents it is assigned via Observe to be destroyed
-// and then quits a RunLoop it is given.  This is used in tests to wait for the
-// receiver page to be torn down in the presentation window.
-class CloseObserver final : public content::WebContentsObserver {
- public:
-  explicit CloseObserver(base::RunLoop* run_loop) : run_loop_(run_loop) {}
-
-  // content::WebContentsObserver overrides.
-  void WebContentsDestroyed() override { run_loop_->Quit(); }
-
-  using content::WebContentsObserver::Observe;
-
- private:
-  base::RunLoop* const run_loop_;
-
-  DISALLOW_COPY_AND_ASSIGN(CloseObserver);
-};
-
 // This class imitates a presentation controller page from a messaging
 // standpoint.  It is registered as a controller connection for the appropriate
 // presentation ID with the LocalPresentationManager to facilitate a
@@ -262,13 +245,11 @@ IN_PROC_BROWSER_TEST_F(PresentationReceiverWindowControllerBrowserTest,
   receiver_window->Start(kPresentationId, presentation_url);
   ASSERT_TRUE(content::WaitForLoadStop(receiver_window->web_contents()));
 
-  base::RunLoop run_loop;
-  CloseObserver close_observer(&run_loop);
-  close_observer.Observe(receiver_window->web_contents());
-
+  content::WebContentsDestroyedWatcher destroyed_watcher(
+      receiver_window->web_contents());
   ASSERT_TRUE(content::ExecuteScript(receiver_window->web_contents(),
                                      "window.location = 'about:blank'"));
-  run_loop.Run();
+  destroyed_watcher.Wait();
 
   destroyer.AwaitTerminate(std::move(receiver_window));
 }
@@ -339,12 +320,10 @@ IN_PROC_BROWSER_TEST_F(PresentationReceiverWindowControllerBrowserTest,
   receiver_window->Start(kPresentationId, GURL("about:blank"));
   ASSERT_TRUE(content::WaitForLoadStop(receiver_window->web_contents()));
 
-  base::RunLoop run_loop;
-  CloseObserver close_observer(&run_loop);
-  close_observer.Observe(receiver_window->web_contents());
-
+  content::WebContentsDestroyedWatcher destroyed_watcher(
+      receiver_window->web_contents());
   CloseWindow(receiver_window.get());
-  run_loop.Run();
+  destroyed_watcher.Wait();
 
   destroyer.AwaitTerminate(std::move(receiver_window));
 }

chrome/browser/ui/webui/constrained_web_dialog_ui_browsertest.cc

@@ -22,6 +22,7 @@
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_navigation_observer.h"
+#include "content/public/test/test_utils.h"
 #include "ui/web_dialogs/test/test_web_dialog_delegate.h"
 
 using content::WebContents;
@@ -46,23 +47,6 @@ std::string GetChangeDimensionsScript(int dimension) {
       "window.document.body.style.height = %d + 'px';", dimension, dimension);
 }
 
-class ConstrainedWebDialogBrowserTestObserver
-    : public content::WebContentsObserver {
- public:
-  explicit ConstrainedWebDialogBrowserTestObserver(WebContents* contents)
-      : content::WebContentsObserver(contents),
-        contents_destroyed_(false) {
-  }
-  ~ConstrainedWebDialogBrowserTestObserver() override {}
-
-  bool contents_destroyed() { return contents_destroyed_; }
-
- private:
-  void WebContentsDestroyed() override { contents_destroyed_ = true; }
-
-  bool contents_destroyed_;
-};
-
 class AutoResizingTestWebDialogDelegate
     : public ui::test::TestWebDialogDelegate {
  public:
@@ -150,15 +134,15 @@ IN_PROC_BROWSER_TEST_F(ConstrainedWebDialogBrowserTest,
   ASSERT_TRUE(dialog_contents);
   ASSERT_TRUE(IsShowingWebContentsModalDialog(web_contents));
 
-  ConstrainedWebDialogBrowserTestObserver observer(dialog_contents);
+  content::WebContentsDestroyedWatcher watcher(dialog_contents);
   std::unique_ptr<WebContents> dialog_contents_holder =
       dialog_delegate->ReleaseWebContents();
   dialog_delegate->OnDialogCloseFromWebUI();
 
-  ASSERT_FALSE(observer.contents_destroyed());
+  ASSERT_FALSE(watcher.IsDestroyed());
   EXPECT_FALSE(IsShowingWebContentsModalDialog(web_contents));
   dialog_contents_holder.reset();
-  EXPECT_TRUE(observer.contents_destroyed());
+  EXPECT_TRUE(watcher.IsDestroyed());
 }
 
 // Tests that dialog autoresizes based on web contents when autoresizing

content/public/test/browser_test_utils.cc

@@ -84,6 +84,7 @@
 #include "content/public/test/test_fileapi_operation_waiter.h"
 #include "content/public/test/test_launcher.h"
 #include "content/public/test/test_navigation_observer.h"
+#include "content/public/test/test_utils.h"
 #include "content/test/did_commit_navigation_interceptor.h"
 #include "ipc/ipc_security_test_util.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
@@ -730,9 +731,9 @@ void WaitForLoadStopWithoutSuccessCheck(WebContents* web_contents) {
 }
 
 bool WaitForLoadStop(WebContents* web_contents) {
-  WebContentsDestroyedObserver observer(web_contents);
+  WebContentsDestroyedWatcher watcher(web_contents);
   WaitForLoadStopWithoutSuccessCheck(web_contents);
-  if (observer.IsDestroyed()) {
+  if (watcher.IsDestroyed()) {
     LOG(ERROR) << "WebContents was destroyed during waiting for load stop.";
     return false;
   }
@@ -2649,18 +2650,6 @@ bool WebContentsAddedObserver::RenderViewCreatedCalled() {
   return false;
 }
 
-WebContentsDestroyedObserver::WebContentsDestroyedObserver(
-    WebContents* web_contents)
-    : WebContentsObserver(web_contents) {
-  DCHECK(web_contents);
-}
-
-WebContentsDestroyedObserver::~WebContentsDestroyedObserver() {}
-
-void WebContentsDestroyedObserver::WebContentsDestroyed() {
-  destroyed_ = true;
-}
-
 bool RequestFrame(WebContents* web_contents) {
   DCHECK(web_contents);
   return RenderWidgetHostImpl::From(

content/public/test/browser_test_utils.h

@@ -1244,22 +1244,6 @@ class WebContentsAddedObserver {
   DISALLOW_COPY_AND_ASSIGN(WebContentsAddedObserver);
 };
 
-// Watches a WebContents to check if it was destroyed.
-class WebContentsDestroyedObserver : public WebContentsObserver {
- public:
-  explicit WebContentsDestroyedObserver(WebContents* web_contents);
-  ~WebContentsDestroyedObserver() override;
-  bool IsDestroyed() { return destroyed_; }
-
- private:
-  // Overridden WebContentsObserver methods.
-  void WebContentsDestroyed() override;
-
-  bool destroyed_ = false;
-
-  DISALLOW_COPY_AND_ASSIGN(WebContentsDestroyedObserver);
-};
-
 // Request a new frame be drawn, returns false if request fails.
 bool RequestFrame(WebContents* web_contents);
 

content/public/test/test_utils.cc

@@ -448,6 +448,7 @@ void WebContentsDestroyedWatcher::Wait() {
 }
 
 void WebContentsDestroyedWatcher::WebContentsDestroyed() {
+  destroyed_ = true;
   run_loop_.Quit();
 }
 

content/public/test/test_utils.h

@@ -316,7 +316,8 @@ class RenderFrameDeletedObserver : public WebContentsObserver {
   DISALLOW_COPY_AND_ASSIGN(RenderFrameDeletedObserver);
 };
 
-// Watches a WebContents and blocks until it is destroyed.
+// Watches a WebContents. Can be used to block until it is destroyed or just
+// merely report if it was destroyed.
 class WebContentsDestroyedWatcher : public WebContentsObserver {
  public:
   explicit WebContentsDestroyedWatcher(WebContents* web_contents);
@@ -325,12 +326,17 @@ class WebContentsDestroyedWatcher : public WebContentsObserver {
   // Waits until the WebContents is destroyed.
   void Wait();
 
+  // Returns whether the WebContents was destroyed.
+  bool IsDestroyed() { return destroyed_; }
+
  private:
   // Overridden WebContentsObserver methods.
   void WebContentsDestroyed() override;
 
   base::RunLoop run_loop_;
 
+  bool destroyed_ = false;
+
   DISALLOW_COPY_AND_ASSIGN(WebContentsDestroyedWatcher);
 };