Thumbnail capturing: Try to deflake and reenable ExplicitWait test

This CL introduces a NavigationAndFirstPaintObserver and uses it in
place of content::TestNavigationObserver.

Bug: 778248
Change-Id: I9c42e384c48a4f5b2b85ca02ed931adf0178e086
Reviewed-on: https://chromium-review.googlesource.com/767388
Commit-Queue: Marc Treib <treib@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516323}

chrome/browser/thumbnails/thumbnail_browsertest.cc

@@ -23,9 +23,10 @@
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
+#include "content/public/browser/navigation_handle.h"
+#include "content/public/browser/web_contents_observer.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/test/controllable_http_response.h"
-#include "content/public/test/test_navigation_observer.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -115,6 +116,42 @@ class MockThumbnailService : public ThumbnailService {
   ~MockThumbnailService() override = default;
 };
 
+// A helper class to wait until a navigation finishes and completes the first
+// paint (as per WebContentsObserver::DidFirstVisuallyNonEmptyPaint). Similar
+// to content::TestNavigationObserver, but waits for the paint in addition to
+// the navigation (and is otherwise simplified).
+// Note: This is a single-use class, supporting only a single navigation. Create
+// a new instance for each navigation.
+class NavigationAndFirstPaintWaiter : public content::WebContentsObserver {
+ public:
+  explicit NavigationAndFirstPaintWaiter(content::WebContents* web_contents)
+      : content::WebContentsObserver(web_contents) {}
+  ~NavigationAndFirstPaintWaiter() override {}
+
+  void Wait() { run_loop_.Run(); }
+
+ private:
+  void DidFinishNavigation(
+      content::NavigationHandle* navigation_handle) override {
+    if (!navigation_handle->HasCommitted() ||
+        !navigation_handle->IsInMainFrame() ||
+        navigation_handle->IsSameDocument()) {
+      return;
+    }
+    CHECK(!did_finish_navigation_) << "Only a single navigation is supported";
+    did_finish_navigation_ = true;
+  }
+
+  void DidFirstVisuallyNonEmptyPaint() override {
+    if (did_finish_navigation_) {
+      run_loop_.Quit();
+    }
+  }
+
+  bool did_finish_navigation_ = false;
+  base::RunLoop run_loop_;
+};
+
 class ThumbnailTest : public InProcessBrowserTest {
  public:
   ThumbnailTest() {}
@@ -164,9 +201,8 @@ class ThumbnailTest : public InProcessBrowserTest {
       will_create_browser_context_services_subscription_;
 };
 
-// Flaky on all bots, see crbug.com/778248.
 IN_PROC_BROWSER_TEST_F(ThumbnailTest,
-                       DISABLED_ShouldCaptureOnNavigatingAwayExplicitWait) {
+                       ShouldCaptureOnNavigatingAwayExplicitWait) {
   content::ControllableHttpResponse response_red(embedded_test_server(),
                                                  "/red.html");
   content::ControllableHttpResponse response_yellow(embedded_test_server(),
@@ -203,8 +239,8 @@ IN_PROC_BROWSER_TEST_F(ThumbnailTest,
       .Times(0);
 
   {
-    // Navigate to some page.
-    content::TestNavigationObserver nav_observer(active_tab);
+    // Navigate to the red page.
+    NavigationAndFirstPaintWaiter waiter(active_tab);
     browser()->OpenURL(content::OpenURLParams(
         red_url, content::Referrer(), WindowOpenDisposition::CURRENT_TAB,
         ui::PAGE_TRANSITION_TYPED, false));
@@ -212,7 +248,7 @@ IN_PROC_BROWSER_TEST_F(ThumbnailTest,
     response_red.Send(kHttpResponseHeader);
     response_red.Send(MakeHtmlDocument("red"));
     response_red.Done();
-    nav_observer.Wait();
+    waiter.Wait();
   }
 
   {
@@ -223,7 +259,7 @@ IN_PROC_BROWSER_TEST_F(ThumbnailTest,
                                  ImageColorIs(SK_ColorRED)))
         .WillOnce(DoAll(RunClosure(run_loop.QuitClosure()), Return(true)));
 
-    content::TestNavigationObserver nav_observer(active_tab);
+    NavigationAndFirstPaintWaiter waiter(active_tab);
     browser()->OpenURL(content::OpenURLParams(
         yellow_url, content::Referrer(), WindowOpenDisposition::CURRENT_TAB,
         ui::PAGE_TRANSITION_TYPED, false));
@@ -236,7 +272,7 @@ IN_PROC_BROWSER_TEST_F(ThumbnailTest,
     response_yellow.Send(kHttpResponseHeader);
     response_yellow.Send(MakeHtmlDocument("yellow"));
     response_yellow.Done();
-    nav_observer.Wait();
+    waiter.Wait();
   }
 
   {
@@ -247,7 +283,7 @@ IN_PROC_BROWSER_TEST_F(ThumbnailTest,
                                  ImageColorIs(SK_ColorYELLOW)))
         .WillOnce(DoAll(RunClosure(run_loop.QuitClosure()), Return(true)));
 
-    content::TestNavigationObserver nav_observer(active_tab);
+    NavigationAndFirstPaintWaiter waiter(active_tab);
     browser()->OpenURL(content::OpenURLParams(
         green_url, content::Referrer(), WindowOpenDisposition::CURRENT_TAB,
         ui::PAGE_TRANSITION_TYPED, false));
@@ -259,7 +295,7 @@ IN_PROC_BROWSER_TEST_F(ThumbnailTest,
     run_loop.Run();
     response_green.Send(MakeHtmlDocument("green"));
     response_green.Done();
-    nav_observer.Wait();
+    waiter.Wait();
   }
 }