Introduce NavigationSimulator to use in unit tests

components/subresource_filter/content/browser/content_subresource_filter_driver_factory_unittest.cc

Index: components/subresource_filter/content/browser/content_subresource_filter_driver_factory_unittest.cc
diff --git a/components/subresource_filter/content/browser/content_subresource_filter_driver_factory_unittest.cc b/components/subresource_filter/content/browser/content_subresource_filter_driver_factory_unittest.cc
index 231c7aef89b188ee5bcf78a5d9daea94192f204d..3f72a0b03e442a809067b54c1153c172ab240b4b 100644
--- a/components/subresource_filter/content/browser/content_subresource_filter_driver_factory_unittest.cc
+++ b/components/subresource_filter/content/browser/content_subresource_filter_driver_factory_unittest.cc
@@ -13,11 +13,14 @@
 #include "components/subresource_filter/core/browser/subresource_filter_client.h"
 #include "components/subresource_filter/core/browser/subresource_filter_features.h"
 #include "components/subresource_filter/core/browser/subresource_filter_features_test_support.h"
+#include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/browser_side_navigation_policy.h"

[engedy, 2017/03/02 10:38:10]

nit: No longer needed.

[clamy, 2017/03/02 13:11:17]

Done.

 #include "content/public/test/mock_render_process_host.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 "net/base/net_errors.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
@@ -40,6 +43,7 @@ const char kUrlA[] = "https://example_a.com";
 const char kUrlB[] = "https://example_b.com";
 const char kUrlC[] = "https://example_c.com";
 const char kUrlD[] = "https://example_d.com";
+const char kSubframeName[] = "Child";
 
 // Human readable representation of expected redirect chain match patterns.
 // The explanations for the buckets given for the following redirect chain:
@@ -185,7 +189,7 @@ class ContentSubresourceFilterDriverFactoryTest
     content::RenderFrameHostTester* rfh_tester =
         content::RenderFrameHostTester::For(main_rfh());
     rfh_tester->InitializeRenderFrameIfNeeded();
-    subframe_rfh_ = rfh_tester->AppendChild("Child");
+    subframe_rfh_ = rfh_tester->AppendChild(kSubframeName);
   }
 
   ContentSubresourceFilterDriverFactory* factory() {
@@ -212,19 +216,6 @@ class ContentSubresourceFilterDriverFactoryTest
     render_process_host->sink().ClearMessages();
   }
 
-  void SimulateNavigationCommit(content::RenderFrameHost* rfh,
-                                const GURL& url,
-                                const content::Referrer& referrer,
-                                const ui::PageTransition transition) {
-    // TODO(crbug.com/688393): Once WCO::ReadyToCommitNavigation is invoked
-    // consistently for tests in PlzNavigate and non-PlzNavigate, remove this.
-    if (!content::IsBrowserSideNavigationEnabled()) {
-      factory()->ReadyToCommitNavigationInternal(rfh, url, referrer,
-                                                 transition);
-    }
-    content::RenderFrameHostTester::For(rfh)->SimulateNavigationCommit(url);
-  }
-
   void BlacklistURLWithRedirectsNavigateAndCommit(
       const std::vector<bool>& blacklisted_urls,
       const std::vector<GURL>& navigation_chain,
@@ -238,10 +229,14 @@ class ContentSubresourceFilterDriverFactoryTest
         expected_activation_decision == ActivationDecision::ACTIVATED;
     base::HistogramTester tester;
     EXPECT_CALL(*client(), ToggleNotificationVisibility(false)).Times(1);
-    content::RenderFrameHostTester* rfh_tester =
-        content::RenderFrameHostTester::For(main_rfh());
 
-    rfh_tester->SimulateNavigationStart(navigation_chain.front());
+    std::unique_ptr<content::NavigationSimulator> navigation_simulator =
+        content::NavigationSimulator::CreateRendererInitiated(
+            navigation_chain.front(), main_rfh());
+    navigation_simulator->SetReferrer(referrer);
+    navigation_simulator->SetTransition(transition);
+    navigation_simulator->Start();
+
     if (blacklisted_urls.front()) {
       factory()->OnMainResourceMatchedSafeBrowsingBlacklist(
           navigation_chain.front(), navigation_chain, threat_type,
@@ -255,15 +250,19 @@ class ContentSubresourceFilterDriverFactoryTest
         factory()->OnMainResourceMatchedSafeBrowsingBlacklist(
             url, navigation_chain, threat_type, threat_type_metadata);
       }
-      rfh_tester->SimulateRedirect(url);
+      navigation_simulator->Redirect(url);
     }
 
-    SimulateNavigationCommit(main_rfh(), navigation_chain.back(), referrer,
-                             transition);
+    navigation_simulator->Commit();
     ExpectActivationSignalForFrame(main_rfh(), expected_activation);
     EXPECT_EQ(expected_activation_decision,
               factory()->GetActivationDecisionForLastCommittedPageLoad());
 
+    // Re-create a subframe now that the frame has navigated.
+    content::RenderFrameHostTester* rfh_tester =
+        content::RenderFrameHostTester::For(main_rfh());
+    subframe_rfh_ = rfh_tester->AppendChild(kSubframeName);
+
     if (expected_pattern != EMPTY) {
       EXPECT_THAT(tester.GetAllSamples(kMatchesPatternHistogramName),
                   ::testing::ElementsAre(base::Bucket(expected_pattern, 1)));
@@ -282,10 +281,17 @@ class ContentSubresourceFilterDriverFactoryTest
   void NavigateAndCommitSubframe(const GURL& url, bool expected_activation) {
     EXPECT_CALL(*client(), ToggleNotificationVisibility(::testing::_)).Times(0);
 
-    content::RenderFrameHostTester::For(subframe_rfh())
-        ->SimulateNavigationStart(url);
-    SimulateNavigationCommit(subframe_rfh(), url, content::Referrer(),
-                             ui::PAGE_TRANSITION_LINK);
+    content::NavigationSimulator::NavigateAndCommitFromDocument(url,
+                                                                subframe_rfh());
+
+    // Update the subframe RFH as the navigation may have lead to a process

[engedy, 2017/03/02 10:38:10]

nit: Given that we need to go hunting for this subframe here anyway, how about
getting rid of |subframe_rfh_| entirely, and moving this block to become the
implementation of subframe_rfh()?

(Alternatively, NavigationSimulator could expose the RFH the simulated
navigation landed in.)

[clamy, 2017/03/02 13:11:17]

I'll go with moving this block. We could have NavigationSimulator expose it
eventually. I'd like to land this ASAP so I'll avoid any modification to the
NavigationSimulator code right now.

+    // swap.
+    for (content::RenderFrameHost* rfh : web_contents()->GetAllFrames()) {
+      if (rfh->GetFrameName() == kSubframeName) {
+        subframe_rfh_ = rfh;
+        break;
+      }
+    }
     ExpectActivationSignalForFrame(subframe_rfh(), expected_activation);
     ::testing::Mock::VerifyAndClearExpectations(client());
   }
@@ -335,10 +341,8 @@ class ContentSubresourceFilterDriverFactoryTest
     // called in production code for failed navigations (e.g. network errors).
     // It is called with browser-side navigation enabled, in which case
     // RenderFrameHostTester already calls it, no need to call it manually.
-    content::RenderFrameHostTester* rfh_tester =
-        content::RenderFrameHostTester::For(main_rfh());
-    rfh_tester->SimulateNavigationStart(url);
-    rfh_tester->SimulateNavigationError(url, 403);
+    content::NavigationSimulator::NavigateAndFailFromDocument(
+        url, net::ERR_TIMED_OUT, main_rfh());
     ExpectActivationSignalForFrame(main_rfh(), false);
     ::testing::Mock::VerifyAndClearExpectations(client());
   }
@@ -358,8 +362,10 @@ class ContentSubresourceFilterDriverFactoryTest
     NavigateAndExpectActivation(blacklisted_urls, {GURL(kExampleUrl)},
                                 expected_pattern, expected_activation_decision);
     EXPECT_CALL(*client(), ToggleNotificationVisibility(::testing::_)).Times(0);
-    content::RenderFrameHostTester::For(main_rfh())
-        ->SimulateNavigationCommit(GURL(kExampleUrl));
+    std::unique_ptr<content::NavigationSimulator> navigation_simulator =
+        content::NavigationSimulator::CreateRendererInitiated(GURL(kExampleUrl),
+                                                              main_rfh());
+    navigation_simulator->CommitSamePage();
     ExpectActivationSignalForFrame(main_rfh(), false);
     ::testing::Mock::VerifyAndClearExpectations(client());
   }
@@ -602,12 +608,6 @@ TEST_F(ContentSubresourceFilterDriverFactoryTest,
 }
 
 TEST_F(ContentSubresourceFilterDriverFactoryTest, WhitelistSiteOnReload) {
-  // TODO(crbug.com/688393): enable this test for PlzNavigate once
-  // WCO::ReadyToCommitNavigation is invoked consistently for tests in
-  // PlzNavigate and non-PlzNavigate.
-  if (content::IsBrowserSideNavigationEnabled())
-    return;
-
   const struct {
     content::Referrer referrer;
     ui::PageTransition transition;