Fix NavigationHandleImplBrowserTest flakiness.

content/browser/frame_host/navigation_handle_impl_browsertest.cc

 // Copyright 2016 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 "base/memory/weak_ptr.h"
 #include "content/browser/frame_host/navigation_handle_impl.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/common/request_context_type.h"
@@ skipping 12 matching lines @@
 
 namespace {
 
 // Gathers data from the NavigationHandle assigned to navigations that start
 // with the expected URL.
 class NavigationHandleObserver : public WebContentsObserver {
  public:
   NavigationHandleObserver(WebContents* web_contents,
                            const GURL& expected_start_url)
       : WebContentsObserver(web_contents),
-        handle_(nullptr),
-        has_committed_(false),
-        is_error_(false),
-        is_main_frame_(false),
-        is_parent_main_frame_(false),
-        is_renderer_initiated_(true),
-        is_same_page_(false),
-        was_redirected_(false),
-        frame_tree_node_id_(-1),
         page_transition_(ui::PAGE_TRANSITION_LINK),
-        expected_start_url_(expected_start_url),
-        net_error_code_(net::OK) {}
+        expected_start_url_(expected_start_url) {}
 
   void DidStartNavigation(NavigationHandle* navigation_handle) override {
     if (handle_ || navigation_handle->GetURL() != expected_start_url_)
       return;
 
     handle_ = navigation_handle;
     has_committed_ = false;
     is_error_ = false;
     page_transition_ = ui::PAGE_TRANSITION_LINK;
     last_committed_url_ = GURL();
@@ skipping 47 matching lines @@
   const GURL& last_committed_url() { return last_committed_url_; }
 
   ui::PageTransition page_transition() { return page_transition_; }
 
   net::Error net_error_code() { return net_error_code_; }
 
  private:
   // A reference to the NavigationHandle so this class will track only
   // one navigation at a time. It is set at DidStartNavigation and cleared
   // at DidFinishNavigation before the NavigationHandle is destroyed.
-  NavigationHandle* handle_;
-  bool has_committed_;
-  bool is_error_;
-  bool is_main_frame_;
-  bool is_parent_main_frame_;
-  bool is_renderer_initiated_;
-  bool is_same_page_;
-  bool was_redirected_;
-  int frame_tree_node_id_;
-  ui::PageTransition page_transition_;
+  NavigationHandle* handle_ = nullptr;
+  bool has_committed_ = false;
+  bool is_error_ = false;
+  bool is_main_frame_ = false;
+  bool is_parent_main_frame_ = false;
+  bool is_renderer_initiated_ = true;
+  bool is_same_page_ = false;
+  bool was_redirected_ = false;
+  int frame_tree_node_id_ = -1;
+  ui::PageTransition page_transition_ = ui::PAGE_TRANSITION_LINK;
   GURL expected_start_url_;
   GURL last_committed_url_;
-  net::Error net_error_code_;
+  net::Error net_error_code_ = net::OK;
 };
 
 // A test NavigationThrottle that will return pre-determined checks and run
 // callbacks when the various NavigationThrottle methods are called. It is
 // not instantiated directly but through a TestNavigationThrottleInstaller.
 class TestNavigationThrottle : public NavigationThrottle {
  public:
   TestNavigationThrottle(
       NavigationHandle* handle,
       NavigationThrottle::ThrottleCheckResult will_start_result,
@@ skipping 49 matching lines @@
                             did_call_will_process_);
     return will_process_result_;
   }
 
   NavigationThrottle::ThrottleCheckResult will_start_result_;
   NavigationThrottle::ThrottleCheckResult will_redirect_result_;
   NavigationThrottle::ThrottleCheckResult will_process_result_;
   base::Closure did_call_will_start_;
   base::Closure did_call_will_redirect_;
   base::Closure did_call_will_process_;
-  RequestContextType request_context_type_;
+  RequestContextType request_context_type_ = REQUEST_CONTEXT_TYPE_UNSPECIFIED;
 };
 
 // Install a TestNavigationThrottle on all following requests and allows waiting
 // for various NavigationThrottle related events. Waiting works only for the
 // immediately next navigation. New instances are needed to wait for further
 // navigations.
 class TestNavigationThrottleInstaller : public WebContentsObserver {
  public:
   TestNavigationThrottleInstaller(
       WebContents* web_contents,
       NavigationThrottle::ThrottleCheckResult will_start_result,
       NavigationThrottle::ThrottleCheckResult will_redirect_result,
       NavigationThrottle::ThrottleCheckResult will_process_result)
       : WebContentsObserver(web_contents),
         will_start_result_(will_start_result),
         will_redirect_result_(will_redirect_result),
         will_process_result_(will_process_result),
-        will_start_called_(0),
-        will_redirect_called_(0),
-        will_process_called_(0),
-        navigation_throttle_(nullptr),
         weak_factory_(this) {}
   ~TestNavigationThrottleInstaller() override {}
 
   TestNavigationThrottle* navigation_throttle() { return navigation_throttle_; }
 
   void WaitForThrottleWillStart() {
     if (will_start_called_)
       return;
     will_start_loop_runner_ = new MessageLoopRunner();
     will_start_loop_runner_->Run();
@@ skipping 13 matching lines @@
       return;
     will_process_loop_runner_ = new MessageLoopRunner();
     will_process_loop_runner_->Run();
     will_process_loop_runner_ = nullptr;
   }
 
   int will_start_called() { return will_start_called_; }
   int will_redirect_called() { return will_redirect_called_; }
   int will_process_called() { return will_process_called_; }
 
+  int install_count() { return install_count_; }
+
  private:
   void DidStartNavigation(NavigationHandle* handle) override {
     std::unique_ptr<NavigationThrottle> throttle(new TestNavigationThrottle(
         handle, will_start_result_, will_redirect_result_, will_process_result_,
         base::Bind(&TestNavigationThrottleInstaller::DidCallWillStartRequest,
                    weak_factory_.GetWeakPtr()),
         base::Bind(&TestNavigationThrottleInstaller::DidCallWillRedirectRequest,
                    weak_factory_.GetWeakPtr()),
         base::Bind(&TestNavigationThrottleInstaller::DidCallWillProcessResponse,
                    weak_factory_.GetWeakPtr())));
     navigation_throttle_ = static_cast<TestNavigationThrottle*>(throttle.get());
     handle->RegisterThrottleForTesting(std::move(throttle));
+    ++install_count_;
   }
 
   void DidFinishNavigation(NavigationHandle* handle) override {
     if (!navigation_throttle_)
       return;
 
     if (handle == navigation_throttle_->navigation_handle())
       navigation_throttle_ = nullptr;
   }
 
@@ skipping 11 matching lines @@
 
   void DidCallWillProcessResponse() {
     will_process_called_++;
     if (will_process_loop_runner_)
       will_process_loop_runner_->Quit();
   }
 
   NavigationThrottle::ThrottleCheckResult will_start_result_;
   NavigationThrottle::ThrottleCheckResult will_redirect_result_;
   NavigationThrottle::ThrottleCheckResult will_process_result_;
-  int will_start_called_;
-  int will_redirect_called_;
-  int will_process_called_;
-  TestNavigationThrottle* navigation_throttle_;
+  int will_start_called_ = 0;
+  int will_redirect_called_ = 0;
+  int will_process_called_ = 0;
+  TestNavigationThrottle* navigation_throttle_ = nullptr;
+  int install_count_ = 0;
   scoped_refptr<MessageLoopRunner> will_start_loop_runner_;
   scoped_refptr<MessageLoopRunner> will_redirect_loop_runner_;
   scoped_refptr<MessageLoopRunner> will_process_loop_runner_;
 
   // The throttle installer can be deleted before all tasks posted by its
   // throttles are run, so it must be referenced via weak pointers.
   base::WeakPtrFactory<TestNavigationThrottleInstaller> weak_factory_;
 };
 
 // Records all navigation start URLs from the WebContents.
@@ skipping 412 matching lines @@
   // Wait for the end of the navigation.
   navigation_observer.Wait();
 
   EXPECT_TRUE(observer.has_committed());
   EXPECT_TRUE(observer.was_redirected());
   EXPECT_FALSE(observer.is_error());
   EXPECT_EQ(shell()->web_contents()->GetLastCommittedURL(),
             GURL(embedded_test_server()->GetURL("bar.com", "/title2.html")));
 }
 
-#if defined(OS_WIN)
-#define MAYBE_VerifyRequestContextTypeForFrameTree \
-  DISABLED_VerifyRequestContextTypeForFrameTree
-#else
-#define MAYBE_VerifyRequestContextTypeForFrameTree \
-  VerifyRequestContextTypeForFrameTree
-#endif
-
 // Checks that the RequestContextType value is properly set.
 IN_PROC_BROWSER_TEST_F(NavigationHandleImplBrowserTest,
-                       MAYBE_VerifyRequestContextTypeForFrameTree) {
+                       VerifyRequestContextTypeForFrameTree) {
   GURL main_url(embedded_test_server()->GetURL(
       "a.com", "/cross_site_iframe_factory.html?a(b(c))"));
   GURL b_url(embedded_test_server()->GetURL(
       "b.com", "/cross_site_iframe_factory.html?b(c())"));
   GURL c_url(embedded_test_server()->GetURL(
       "c.com", "/cross_site_iframe_factory.html?c()"));
 
   TestNavigationThrottleInstaller installer(
       shell()->web_contents(), NavigationThrottle::PROCEED,
       NavigationThrottle::PROCEED, NavigationThrottle::PROCEED);
   TestNavigationManager main_manager(shell()->web_contents(), main_url);
   TestNavigationManager b_manager(shell()->web_contents(), b_url);
   TestNavigationManager c_manager(shell()->web_contents(), c_url);
   NavigationStartUrlRecorder url_recorder(shell()->web_contents());
-  TestNavigationThrottle* previous_throttle = nullptr;
 
   // Starts and verifies the main frame navigation.
   shell()->LoadURL(main_url);
   EXPECT_TRUE(main_manager.WaitForRequestStart());
-  // The throttle should not be null.
-  EXPECT_NE(previous_throttle, installer.navigation_throttle());
+  // For each navigation a new throttle should have been installed.
+  EXPECT_EQ(1, installer.install_count());
   // Checks the only URL recorded so far is the one expected for the main frame.
   EXPECT_EQ(main_url, url_recorder.urls().back());
   EXPECT_EQ(1ul, url_recorder.urls().size());
   // Checks the main frame RequestContextType.
   EXPECT_EQ(REQUEST_CONTEXT_TYPE_LOCATION,
             installer.navigation_throttle()->request_context_type());
-  // For each navigations the throttle should be a different instance.
-  previous_throttle = installer.navigation_throttle();
 
   // Ditto for frame b navigation.
   main_manager.WaitForNavigationFinished();
   EXPECT_TRUE(b_manager.WaitForRequestStart());
-  EXPECT_NE(previous_throttle, installer.navigation_throttle());
+  EXPECT_EQ(2, installer.install_count());
   EXPECT_EQ(b_url, url_recorder.urls().back());
   EXPECT_EQ(2ul, url_recorder.urls().size());
   EXPECT_EQ(REQUEST_CONTEXT_TYPE_LOCATION,
             installer.navigation_throttle()->request_context_type());
-  previous_throttle = installer.navigation_throttle();
 
   // Ditto for frame c navigation.
   b_manager.WaitForNavigationFinished();
   EXPECT_TRUE(c_manager.WaitForRequestStart());
-  EXPECT_NE(previous_throttle, installer.navigation_throttle());
+  EXPECT_EQ(3, installer.install_count());
   EXPECT_EQ(c_url, url_recorder.urls().back());
   EXPECT_EQ(3ul, url_recorder.urls().size());
   EXPECT_EQ(REQUEST_CONTEXT_TYPE_LOCATION,
             installer.navigation_throttle()->request_context_type());
 
   // Lets the final navigation finish so that we conclude running the
   // RequestContextType checks that happen in TestNavigationThrottle.
   c_manager.WaitForNavigationFinished();
   // Confirms the last navigation did finish.
   EXPECT_FALSE(installer.navigation_throttle());
@@ skipping 258 matching lines @@
         NavigationThrottle::PROCEED, NavigationThrottle::PROCEED);
     NavigationHandleObserver observer(shell()->web_contents(), url);
     EXPECT_TRUE(NavigateToURL(shell(), url));
     EXPECT_EQ(1, installer.will_start_called());
     EXPECT_EQ(1, installer.will_process_called());
     EXPECT_FALSE(observer.is_same_page());
   }
 }
 
 }  // namespace content