Remove usage of deprecated navigation test method outside of content/

This CL removes most of the usages of
RenderFrameHostTester::SimulateNavigationStart outside of content/ in
favor of using NavigationSimulator. The goal is to have embedders of
content/ use the NavigationSimulator exclusively when writing unit tests
that involve navigation. This allows several tests to work with
PlzNavigate or OOPIF enabled.

BUG=728571

Change-Id: Iab0efe63d3c892da62ffe4e5cc2641e9af0f03a3
Reviewed-on: https://chromium-review.googlesource.com/582949
Commit-Queue: Camille Lamy <clamy@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Nathan Parker <nparker@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Emily Stark <estark@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491791}

chrome/browser/browser_commands_unittest.cc

@@ -21,6 +21,7 @@
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/test/navigation_simulator.h"
 #include "content/public/test/test_renderer_host.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -117,11 +118,8 @@ TEST_F(BrowserCommandsTest, ViewSource) {
       content::RenderFrameHostTester::For(
           browser()->tab_strip_model()->GetWebContentsAt(0)->GetMainFrame());
   content::RenderFrameHost* subframe = rfh_tester->AppendChild("subframe");
-  content::RenderFrameHostTester* subframe_tester =
-      content::RenderFrameHostTester::For(subframe);
-  subframe_tester->SimulateNavigationStart(GURL(url1_subframe));
-  subframe_tester->SimulateNavigationCommit(GURL(url1_subframe));
-  subframe_tester->SimulateNavigationStop();
+  content::NavigationSimulator::NavigateAndCommitFromDocument(
+      GURL(url1_subframe), subframe);
 
   // Now start a pending navigation that hasn't committed.
   content::NavigationController& orig_controller =

chrome/browser/extensions/extension_navigation_throttle_unittest.cc

@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "extensions/browser/extension_navigation_throttle.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/stringprintf.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
@@ -9,9 +10,9 @@
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/common/content_client.h"
+#include "content/public/test/navigation_simulator.h"
 #include "content/public/test/test_renderer_host.h"
 #include "content/public/test/web_contents_tester.h"
-#include "extensions/browser/extension_navigation_throttle.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_builder.h"
@@ -181,8 +182,8 @@ TEST_F(ExtensionNavigationThrottleUnitTest, WebPageAncestor) {
   content::RenderFrameHost* child =
       render_frame_host_tester(main_rfh())->AppendChild("subframe1");
   GURL url = extension()->GetResourceURL(kAccessible);
-  render_frame_host_tester(child)->SimulateNavigationStart(url);
-  render_frame_host_tester(child)->SimulateNavigationCommit(url);
+  child =
+      content::NavigationSimulator::NavigateAndCommitFromDocument(url, child);
   content::RenderFrameHost* grand_child =
       render_frame_host_tester(child)->AppendChild("grandchild");
 

chrome/browser/page_load_metrics/metrics_web_contents_observer_unittest.cc

@@ -371,10 +371,10 @@ TEST_F(MetricsWebContentsObserverTest, SubFrame) {
       base::TimeDelta::FromMilliseconds(30);
   subframe_timing.paint_timing->first_paint =
       base::TimeDelta::FromMilliseconds(40);
+  subframe = content::NavigationSimulator::NavigateAndCommitFromDocument(
+      GURL(kDefaultTestUrl2), subframe);
   content::RenderFrameHostTester* subframe_tester =
       content::RenderFrameHostTester::For(subframe);
-  subframe_tester->SimulateNavigationStart(GURL(kDefaultTestUrl2));
-  subframe_tester->SimulateNavigationCommit(GURL(kDefaultTestUrl2));
   SimulateTimingUpdate(subframe_timing, subframe);
   subframe_tester->SimulateNavigationStop();
 
@@ -793,12 +793,9 @@ TEST_F(MetricsWebContentsObserverTest, OutOfOrderCrossFrameTiming) {
   PopulatePageLoadTiming(&subframe_timing);
   subframe_timing.paint_timing->first_paint =
       base::TimeDelta::FromMilliseconds(40);
-  content::RenderFrameHostTester* subframe_tester =
-      content::RenderFrameHostTester::For(subframe);
-  subframe_tester->SimulateNavigationStart(GURL(kDefaultTestUrl2));
-  subframe_tester->SimulateNavigationCommit(GURL(kDefaultTestUrl2));
+  subframe = content::NavigationSimulator::NavigateAndCommitFromDocument(
+      GURL(kDefaultTestUrl2), subframe);
   SimulateTimingUpdate(subframe_timing, subframe);
-  subframe_tester->SimulateNavigationStop();
 
   // Though a first paint was dispatched in the child, it should not yet be
   // reflected as an updated timing in the main frame, since the main frame
@@ -871,10 +868,10 @@ TEST_F(MetricsWebContentsObserverTest, OutOfOrderCrossFrameTiming2) {
   subframe_timing.paint_timing->first_paint =
       base::TimeDelta::FromMilliseconds(500);
   content::RenderFrameHost* subframe = rfh_tester->AppendChild("subframe");
+  subframe = content::NavigationSimulator::NavigateAndCommitFromDocument(
+      GURL(kDefaultTestUrl2), subframe);
   content::RenderFrameHostTester* subframe_tester =
       content::RenderFrameHostTester::For(subframe);
-  subframe_tester->SimulateNavigationStart(GURL(kDefaultTestUrl2));
-  subframe_tester->SimulateNavigationCommit(GURL(kDefaultTestUrl2));
   SimulateTimingUpdateWithoutFiringDispatchTimer(subframe_timing, subframe);
   subframe_tester->SimulateNavigationStop();
 
@@ -919,10 +916,10 @@ TEST_F(MetricsWebContentsObserverTest, OutOfOrderCrossFrameTiming2) {
   subframe_timing.paint_timing->first_paint =
       base::TimeDelta::FromMilliseconds(50);
   content::RenderFrameHost* subframe2 = rfh_tester->AppendChild("subframe");
+  subframe2 = content::NavigationSimulator::NavigateAndCommitFromDocument(
+      GURL(kDefaultTestUrl2), subframe2);
   content::RenderFrameHostTester* subframe2_tester =
       content::RenderFrameHostTester::For(subframe2);
-  subframe2_tester->SimulateNavigationStart(GURL(kDefaultTestUrl2));
-  subframe2_tester->SimulateNavigationCommit(GURL(kDefaultTestUrl2));
   SimulateTimingUpdateWithoutFiringDispatchTimer(subframe_timing, subframe2);
   subframe2_tester->SimulateNavigationStop();
 

chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer_unittest.cc

@@ -14,6 +14,7 @@
 #include "components/rappor/test_rappor_service.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/browser_side_navigation_policy.h"
+#include "content/public/test/navigation_simulator.h"
 #include "content/public/test/test_utils.h"
 #include "third_party/WebKit/public/platform/WebMouseEvent.h"
 
@@ -319,17 +320,13 @@ TEST_F(CorePageLoadMetricsObserverTest, DontBackgroundQuickerLoad) {
 }
 
 TEST_F(CorePageLoadMetricsObserverTest, FailedProvisionalLoad) {
-  if (content::IsBrowserSideNavigationEnabled() &&
-      content::AreAllSitesIsolatedForTesting()) {
-    // http://crbug.com/674734 Fix this test with PlzNavigate and Site Isolation
-    return;
-  }
   GURL url(kDefaultTestUrl);
-  content::RenderFrameHostTester* rfh_tester =
-      content::RenderFrameHostTester::For(main_rfh());
-  rfh_tester->SimulateNavigationStart(url);
-  rfh_tester->SimulateNavigationError(url, net::ERR_TIMED_OUT);
-  rfh_tester->SimulateNavigationStop();
+  // The following tests a navigation that fails and should commit an error
+  // page, but finishes before the error page commit.
+  std::unique_ptr<content::NavigationSimulator> navigation =
+      content::NavigationSimulator::CreateRendererInitiated(url, main_rfh());
+  navigation->Fail(net::ERR_TIMED_OUT);
+  content::RenderFrameHostTester::For(main_rfh())->SimulateNavigationStop();
 
   histogram_tester().ExpectTotalCount(internal::kHistogramDomContentLoaded, 0);
   histogram_tester().ExpectTotalCount(internal::kHistogramLoad, 0);
@@ -349,11 +346,8 @@ TEST_F(CorePageLoadMetricsObserverTest, FailedBackgroundProvisionalLoad) {
   // histogram if it happened in the background
   GURL url(kDefaultTestUrl);
   web_contents()->WasHidden();
-  content::RenderFrameHostTester* rfh_tester =
-      content::RenderFrameHostTester::For(main_rfh());
-  rfh_tester->SimulateNavigationStart(url);
-  rfh_tester->SimulateNavigationError(url, net::ERR_TIMED_OUT);
-  rfh_tester->SimulateNavigationStop();
+  content::NavigationSimulator::NavigateAndFailFromDocument(
+      url, net::ERR_TIMED_OUT, main_rfh());
 
   histogram_tester().ExpectTotalCount(internal::kHistogramFailedProvisionalLoad,
                                       0);

chrome/browser/page_load_metrics/observers/ukm_page_load_metrics_observer_unittest.cc

@@ -13,6 +13,7 @@
 #include "chrome/common/page_load_metrics/test/page_load_metrics_test_util.h"
 #include "components/metrics/proto/system_profile.pb.h"
 #include "components/ukm/ukm_source.h"
+#include "content/public/test/navigation_simulator.h"
 #include "net/nqe/effective_connection_type.h"
 #include "net/nqe/network_quality_provider.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -127,12 +128,13 @@ TEST_F(UkmPageLoadMetricsObserverTest, FailedProvisionalLoad) {
   EXPECT_CALL(mock_network_quality_provider(), GetEffectiveConnectionType())
       .WillRepeatedly(Return(net::EFFECTIVE_CONNECTION_TYPE_2G));
 
+  // The following simulates a navigation that fails and should commit an error
+  // page, but finishes before the error page actually commits.
   GURL url(kTestUrl1);
-  content::RenderFrameHostTester* rfh_tester =
-      content::RenderFrameHostTester::For(main_rfh());
-  rfh_tester->SimulateNavigationStart(url);
-  rfh_tester->SimulateNavigationError(url, net::ERR_TIMED_OUT);
-  rfh_tester->SimulateNavigationStop();
+  std::unique_ptr<content::NavigationSimulator> navigation =
+      content::NavigationSimulator::CreateRendererInitiated(url, main_rfh());
+  navigation->Fail(net::ERR_TIMED_OUT);
+  content::RenderFrameHostTester::For(main_rfh())->SimulateNavigationStop();
 
   // Simulate closing the tab.
   DeleteContents();

chrome/browser/safe_browsing/safe_browsing_navigation_observer_unittest.cc

@@ -10,6 +10,7 @@
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/test/base/browser_with_test_window_test.h"
 #include "content/public/common/browser_side_navigation_policy.h"
+#include "content/public/test/navigation_simulator.h"
 #include "content/public/test/test_renderer_host.h"
 #include "content/public/test/test_utils.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -176,18 +177,12 @@ TEST_F(SBNavigationObserverTest, BasicNavigationAndCommit) {
 }
 
 TEST_F(SBNavigationObserverTest, ServerRedirect) {
-  if (content::IsBrowserSideNavigationEnabled() &&
-      content::AreAllSitesIsolatedForTesting()) {
-    // http://crbug.com/674734 Fix this test with PlzNavigate and Site Isolation
-    return;
-  }
-  content::RenderFrameHostTester* rfh_tester =
-      content::RenderFrameHostTester::For(
-          browser()->tab_strip_model()->GetActiveWebContents()->GetMainFrame());
-  rfh_tester->SimulateNavigationStart(GURL("http://foo/3"));
-  GURL redirect("http://redirect/1");
-  rfh_tester->SimulateRedirect(redirect);
-  rfh_tester->SimulateNavigationCommit(redirect);
+  auto navigation = content::NavigationSimulator::CreateRendererInitiated(
+      GURL("http://foo/3"),
+      browser()->tab_strip_model()->GetActiveWebContents()->GetMainFrame());
+  navigation->Start();
+  navigation->Redirect(GURL("http://redirect/1"));
+  navigation->Commit();
   int tab_id = SessionTabHelper::IdForTab(
       browser()->tab_strip_model()->GetWebContentsAt(0));
   auto* nav_list = navigation_event_list();

chrome/browser/ssl/insecure_sensitive_input_driver_unittest.cc

@@ -11,6 +11,7 @@
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/ssl_status.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/test/navigation_simulator.h"
 #include "content/public/test/web_contents_tester.h"
 
 namespace {
@@ -72,12 +73,10 @@ TEST_F(InsecureSensitiveInputDriverTest, PasswordVisibilityWithSubframe) {
   // notifications for it are handled properly.
   content::RenderFrameHost* subframe =
       content::RenderFrameHostTester::For(main_rfh())->AppendChild("child");
+  subframe = content::NavigationSimulator::NavigateAndCommitFromDocument(
+      GURL("http://example2.test"), subframe);
   auto subframe_driver =
       base::MakeUnique<InsecureSensitiveInputDriver>(subframe);
-  content::RenderFrameHostTester* subframe_tester =
-      content::RenderFrameHostTester::For(subframe);
-  subframe_tester->SimulateNavigationStart(GURL("http://example2.test"));
-  subframe_tester->SimulateNavigationCommit(GURL("http://example2.test"));
   subframe_driver->PasswordFieldVisibleInInsecureContext();
 
   content::NavigationEntry* entry =
@@ -115,12 +114,10 @@ TEST_F(InsecureSensitiveInputDriverTest,
   // notifications for it are handled properly.
   content::RenderFrameHost* subframe =
       content::RenderFrameHostTester::For(main_rfh())->AppendChild("child");
+  subframe = content::NavigationSimulator::NavigateAndCommitFromDocument(
+      GURL("http://example2.test"), subframe);
   auto subframe_driver =
       base::MakeUnique<InsecureSensitiveInputDriver>(subframe);
-  content::RenderFrameHostTester* subframe_tester =
-      content::RenderFrameHostTester::For(subframe);
-  subframe_tester->SimulateNavigationStart(GURL("http://example2.test"));
-  subframe_tester->SimulateNavigationCommit(GURL("http://example2.test"));
   subframe_driver->PasswordFieldVisibleInInsecureContext();
 
   entry = web_contents()->GetController().GetVisibleEntry();
@@ -150,12 +147,12 @@ TEST_F(InsecureSensitiveInputDriverTest,
   // Create a subframe with a password field.
   content::RenderFrameHost* subframe =
       content::RenderFrameHostTester::For(main_rfh())->AppendChild("child");
+  subframe = content::NavigationSimulator::NavigateAndCommitFromDocument(
+      GURL("http://example2.test"), subframe);
   auto subframe_driver =
       base::MakeUnique<InsecureSensitiveInputDriver>(subframe);
   content::RenderFrameHostTester* subframe_tester =
       content::RenderFrameHostTester::For(subframe);
-  subframe_tester->SimulateNavigationStart(GURL("http://example2.test"));
-  subframe_tester->SimulateNavigationCommit(GURL("http://example2.test"));
   subframe_driver->PasswordFieldVisibleInInsecureContext();
 
   content::NavigationEntry* entry =

components/subresource_filter/content/browser/subframe_navigation_filtering_throttle_unittest.cc

@@ -199,11 +199,11 @@ TEST_F(SubframeNavigationFilteringThrottleTest, FilterSubsubframe) {
       content::RenderFrameHostTester::For(main_rfh())
           ->AppendChild("parent-sub");
   GURL test_url = GURL("https://example.test");
-  content::RenderFrameHostTester::For(parent_subframe)
-      ->SimulateNavigationStart(test_url);
+  auto navigation = content::NavigationSimulator::CreateRendererInitiated(
+      test_url, parent_subframe);
+  navigation->Start();
   InitializeDocumentSubresourceFilter(GURL("https://example.test"));
-  content::RenderFrameHostTester::For(parent_subframe)
-      ->SimulateNavigationCommit(test_url);
+  navigation->Commit();
 
   CreateTestSubframeAndInitNavigation(
       GURL("https://example.test/disallowed.html"), parent_subframe);

content/public/test/navigation_simulator.cc

@@ -64,24 +64,27 @@ class NavigationThrottleCallbackRunner : public NavigationThrottle {
 }  // namespace
 
 // static
-void NavigationSimulator::NavigateAndCommitFromDocument(
+RenderFrameHost* NavigationSimulator::NavigateAndCommitFromDocument(
     const GURL& original_url,
     RenderFrameHost* render_frame_host) {
   NavigationSimulator simulator(
       original_url, static_cast<TestRenderFrameHost*>(render_frame_host));
   simulator.Commit();
+  return simulator.GetFinalRenderFrameHost();
 }
 
 // static
-void NavigationSimulator::NavigateAndFailFromDocument(
+RenderFrameHost* NavigationSimulator::NavigateAndFailFromDocument(
     const GURL& original_url,
     int net_error_code,
     RenderFrameHost* render_frame_host) {
   NavigationSimulator simulator(
       original_url, static_cast<TestRenderFrameHost*>(render_frame_host));
   simulator.Fail(net_error_code);
-  if (net_error_code != net::ERR_ABORTED)
-    simulator.CommitErrorPage();
+  if (net_error_code == net::ERR_ABORTED)
+    return nullptr;
+  simulator.CommitErrorPage();
+  return simulator.GetFinalRenderFrameHost();
 }
 
 // static
@@ -317,9 +320,8 @@ void NavigationSimulator::Commit() {
   params.origin = url::Origin(navigation_url_);
   params.transition = transition_;
   params.should_update_history = true;
-  params.did_create_new_entry =
-      !render_frame_host_->GetParent() ||
-      render_frame_host_->frame_tree_node()->has_committed_real_load();
+  params.did_create_new_entry = !ui::PageTransitionCoreTypeIs(
+      transition_, ui::PAGE_TRANSITION_AUTO_SUBFRAME);
   params.gesture = NavigationGestureUser;
   params.contents_mime_type = "text/html";
   params.method = "GET";
@@ -422,7 +424,8 @@ void NavigationSimulator::CommitErrorPage() {
       base::TimeTicks::Now()));
   FrameHostMsg_DidCommitProvisionalLoad_Params params;
   params.nav_entry_id = 0;
-  params.did_create_new_entry = true;
+  params.did_create_new_entry = !ui::PageTransitionCoreTypeIs(
+      transition_, ui::PAGE_TRANSITION_AUTO_SUBFRAME);
   params.url = navigation_url_;
   params.transition = transition_;
   params.was_within_same_document = false;

content/public/test/navigation_simulator.h

@@ -34,15 +34,20 @@ struct Referrer;
 class NavigationSimulator : public WebContentsObserver {
  public:
   // Simulates a renderer-initiated navigation to |url| started in
-  // |render_frame_host| from start to commit.
-  static void NavigateAndCommitFromDocument(const GURL& original_url,
-                                            RenderFrameHost* render_frame_host);
+  // |render_frame_host| from start to commit. Returns the RenderFramehost that
+  // committed the navigation.
+  static RenderFrameHost* NavigateAndCommitFromDocument(
+      const GURL& original_url,
+      RenderFrameHost* render_frame_host);
 
   // Simulates a failed renderer-initiated navigation to |url| started in
-  // |render_frame_host| from start to commit.
-  static void NavigateAndFailFromDocument(const GURL& original_url,
-                                          int net_error_code,
-                                          RenderFrameHost* render_frame_host);
+  // |render_frame_host| from start to commit. Returns the RenderFramehost that
+  // committed the error page for the navigation, or nullptr if the navigation
+  // error did not result in an error page.
+  static RenderFrameHost* NavigateAndFailFromDocument(
+      const GURL& original_url,
+      int net_error_code,
+      RenderFrameHost* render_frame_host);
 
   // ---------------------------------------------------------------------------