Bring RFH/RVH unit tests closer to reality of how RF/RV are initialized

BUG=357747
TEST=all unit tests continue passing
TBR=sky@chromium.org

Review URL: https://codereview.chromium.org/1151973005

Cr-Commit-Position: refs/heads/master@{#331894}

11 files changed, 44 insertions, 28 deletions

diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc b/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc
index cdc185a..0730b18 100644
--- a/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page_unittest.cc
@@ -548,7 +548,7 @@ TEST_F(SafeBrowsingBlockingPageTest, NavigatingBackAndForth) {
 
   // Proceed, then navigate back.
   ProceedThroughInterstitial(sb_interstitial);
-  Navigate(kBadURL, 2, pending_id, true);  // Commit the navigation.
+  NavigateCrossSite(kBadURL, 2, pending_id, true);  // Commit the navigation.
   GoBack(true);
 
   // We are back on the good page.
diff --git a/components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page_unittest.cc b/components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page_unittest.cc
index 64d5076..5f66af3 100644
--- a/components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page_unittest.cc
+++ b/components/data_reduction_proxy/content/browser/data_reduction_proxy_debug_blocking_page_unittest.cc
@@ -412,7 +412,7 @@ TEST_F(DataReductionProxyDebugBlockingPageTest, NavigatingBackAndForth) {
 
   // Proceed through the 1st interstitial.
   ProceedThroughInterstitial(interstitial);
-  Navigate(kBypassURL, 2, pending_id, true);  // Commit navigation.
+  NavigateCrossSite(kBypassURL, 2, pending_id, true);  // Commit navigation.
   GoBack(true);
 
   // We are back on the first page.
diff --git a/content/browser/frame_host/frame_tree_unittest.cc b/content/browser/frame_host/frame_tree_unittest.cc
index e22ca30..5a60ec8 100644
--- a/content/browser/frame_host/frame_tree_unittest.cc
+++ b/content/browser/frame_host/frame_tree_unittest.cc
@@ -309,7 +309,7 @@ TEST_F(FrameTreeTest, PreviousSibling) {
 TEST_F(FrameTreeTest, ObserverWalksTreeDuringFrameCreation) {
   TreeWalkingWebContentsLogger activity(contents());
   contents()->NavigateAndCommit(GURL("http://www.google.com"));
-  EXPECT_EQ("", activity.GetLog());
+  EXPECT_EQ("RenderFrameCreated(1) -> 1: []", activity.GetLog());
 
   FrameTree* frame_tree = contents()->GetFrameTree();
   FrameTreeNode* root = frame_tree->root();
@@ -340,7 +340,7 @@ TEST_F(FrameTreeTest, ObserverWalksTreeDuringFrameCreation) {
 TEST_F(FrameTreeTest, ObserverWalksTreeAfterCrash) {
   TreeWalkingWebContentsLogger activity(contents());
   contents()->NavigateAndCommit(GURL("http://www.google.com"));
-  EXPECT_EQ("", activity.GetLog());
+  EXPECT_EQ("RenderFrameCreated(1) -> 1: []", activity.GetLog());
 
   main_test_rfh()->OnCreateChildFrame(22, blink::WebTreeScopeType::Document,
                                       std::string(),
@@ -362,7 +362,7 @@ TEST_F(FrameTreeTest, ObserverWalksTreeAfterCrash) {
   EXPECT_EQ(
       "RenderFrameDeleted(23) -> 1: [22: [], 23*: []]\n"
       "RenderFrameDeleted(22) -> 1: [22*: [], 23*: []]\n"
-      "RenderFrameDeleted(1) -> 1: []\n"  // TODO(nick): Should be "1*:"
+      "RenderFrameDeleted(1) -> 1*: []\n"
       "RenderProcessGone -> 1*: []",
       activity.GetLog());
 }
diff --git a/content/browser/frame_host/navigation_controller_impl_unittest.cc b/content/browser/frame_host/navigation_controller_impl_unittest.cc
index a8a1f6c..baf63d6 100644
--- a/content/browser/frame_host/navigation_controller_impl_unittest.cc
+++ b/content/browser/frame_host/navigation_controller_impl_unittest.cc
@@ -2824,6 +2824,10 @@ TEST_F(NavigationControllerTest, RestoreNavigateAfterFailure) {
       0, NavigationController::RESTORE_LAST_SESSION_EXITED_CLEANLY, &entries);
   ASSERT_EQ(0u, entries.size());
 
+  // Ensure the RenderFrame is initialized before simulating events coming from
+  // it.
+  main_test_rfh()->InitializeRenderFrameIfNeeded();
+
   // Before navigating to the restored entry, it should have a restore_type
   // and no SiteInstance.
   entry = our_controller.GetEntryAtIndex(0);
diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc
index cc0ad50..d6abcbb 100644
--- a/content/browser/frame_host/render_frame_host_impl.cc
+++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -645,12 +645,13 @@ bool RenderFrameHostImpl::CreateRenderFrame(int parent_routing_id,
 }
 
 bool RenderFrameHostImpl::IsRenderFrameLive() {
-  // RenderFrames are created for main frames at the same time as RenderViews,
-  // so we rely on IsRenderViewLive.  For subframes, we keep track of each
-  // RenderFrame individually with render_frame_created_.
-  bool is_live = !GetParent() ?
-      render_view_host_->IsRenderViewLive() :
-      GetProcess()->HasConnection() && render_frame_created_;
+  bool is_live = GetProcess()->HasConnection() && render_frame_created_;
+
+  // If the process is for an isolated guest (e.g. <webview>), rely on the
+  // RenderViewHost liveness check. Once https://crbug.com/492830 is fixed,
+  // this can be removed.
+  if (GetProcess()->IsIsolatedGuest())
+    is_live = render_view_host_->IsRenderViewLive();
 
   // Sanity check: the RenderView should always be live if the RenderFrame is.
   DCHECK(!is_live || render_view_host_->IsRenderViewLive());
diff --git a/content/browser/web_contents/aura/overscroll_navigation_overlay_unittest.cc b/content/browser/web_contents/aura/overscroll_navigation_overlay_unittest.cc
index f4f6947..1067d15 100644
--- a/content/browser/web_contents/aura/overscroll_navigation_overlay_unittest.cc
+++ b/content/browser/web_contents/aura/overscroll_navigation_overlay_unittest.cc
@@ -39,7 +39,6 @@ class OverscrollTestWebContents : public TestWebContents {
     OverscrollTestWebContents* web_contents = new OverscrollTestWebContents(
         browser_context, fake_native_view.Pass(), fake_contents_window.Pass());
     web_contents->Init(WebContents::CreateParams(browser_context, instance));
-    web_contents->RenderFrameCreated(web_contents->GetMainFrame());
     return web_contents;
   }
 
diff --git a/content/browser/web_contents/web_contents_impl_unittest.cc b/content/browser/web_contents/web_contents_impl_unittest.cc
index 7829b5a..c2d8189 100644
--- a/content/browser/web_contents/web_contents_impl_unittest.cc
+++ b/content/browser/web_contents/web_contents_impl_unittest.cc
@@ -1681,6 +1681,7 @@ TEST_F(WebContentsImplTest,
   // Simulate the navigation to the page, that's when the interstitial gets
   // hidden.
   GURL url3("http://www.thepage.com");
+  contents()->GetMainFrame()->PrepareForCommit();
   contents()->GetMainFrame()->SendNavigate(2, 0, true, url3);
 
   EXPECT_FALSE(contents()->ShowingInterstitialPage());
@@ -3015,6 +3016,9 @@ TEST_F(WebContentsImplTest, MediaPowerSaveBlocking) {
   TestRenderFrameHost* rfh = contents()->GetMainFrame();
   AudioStateProvider* audio_state = contents()->audio_state_provider();
 
+  // Ensure RenderFrame is initialized before simulating events coming from it.
+  main_test_rfh()->InitializeRenderFrameIfNeeded();
+
   // The audio power save blocker should not be based on having a media player
   // when audio stream monitoring is available.
   if (audio_state->IsAudioStateAvailable()) {
diff --git a/content/test/test_render_frame_host.cc b/content/test/test_render_frame_host.cc
index 5b7aa53..de63d46 100644
--- a/content/test/test_render_frame_host.cc
+++ b/content/test/test_render_frame_host.cc
@@ -56,12 +56,9 @@ TestRenderFrameHost::TestRenderFrameHost(SiteInstance* site_instance,
       child_creation_observer_(delegate ? delegate->GetAsWebContents() : NULL),
       contents_mime_type_("text/html"),
       simulate_history_list_was_cleared_(false) {
-  if (frame_tree_node_->IsMainFrame())
-    SetRenderFrameCreated(true);
 }
 
 TestRenderFrameHost::~TestRenderFrameHost() {
-  SetRenderFrameCreated(false);
 }
 
 TestRenderViewHost* TestRenderFrameHost::GetRenderViewHost() {
@@ -132,11 +129,6 @@ void TestRenderFrameHost::SendNavigateWithTransitionAndResponseCode(
   // DidStartProvisionalLoad may delete the pending entry that holds |url|,
   // so we keep a copy of it to use in SendNavigateWithParameters.
   GURL url_copy(url);
-
-  // Ensure that the RenderFrameCreated notification has been sent to observers
-  // before navigating the frame.
-  SetRenderFrameCreated(true);
-
   OnDidStartProvisionalLoadForFrame(url_copy);
   SendNavigateWithParameters(page_id, nav_entry_id, did_create_new_entry,
                              url_copy, transition, url_copy, response_code, 0,
@@ -149,10 +141,6 @@ void TestRenderFrameHost::SendNavigateWithOriginalRequestURL(
     bool did_create_new_entry,
     const GURL& url,
     const GURL& original_request_url) {
-  // Ensure that the RenderFrameCreated notification has been sent to observers
-  // before navigating the frame.
-  SetRenderFrameCreated(true);
-
   OnDidStartProvisionalLoadForFrame(url);
   SendNavigateWithParameters(page_id, nav_entry_id, did_create_new_entry, url,
                              ui::PAGE_TRANSITION_LINK, original_request_url,
@@ -246,6 +234,10 @@ void TestRenderFrameHost::NavigateAndCommitRendererInitiated(
 void TestRenderFrameHost::SendRendererInitiatedNavigationRequest(
     const GURL& url,
     bool has_user_gesture) {
+  // Since this is renderer-initiated navigation, the RenderFrame must be
+  // initialized. Do it if it hasn't happened yet.
+  InitializeRenderFrameIfNeeded();
+
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableBrowserSideNavigation)) {
     BeginNavigationParams begin_params("GET", std::string(), net::LOAD_NORMAL,
@@ -263,6 +255,13 @@ void TestRenderFrameHost::DidDisownOpener() {
   OnDidDisownOpener();
 }
 
+void TestRenderFrameHost::InitializeRenderFrameIfNeeded() {
+  if (!render_view_host()->IsRenderViewLive()) {
+    RenderViewHostTester::For(render_view_host())->CreateRenderView(
+        base::string16(), MSG_ROUTING_NONE, MSG_ROUTING_NONE, -1, false);
+  }
+}
+
 void TestRenderFrameHost::PrepareForCommit() {
   PrepareForCommitWithServerRedirect(GURL());
 }
diff --git a/content/test/test_render_frame_host.h b/content/test/test_render_frame_host.h
index 3b0c9ae..1b28496 100644
--- a/content/test/test_render_frame_host.h
+++ b/content/test/test_render_frame_host.h
@@ -123,6 +123,11 @@ class TestRenderFrameHost : public RenderFrameHostImpl,
     simulate_history_list_was_cleared_ = cleared;
   }
 
+  // Simulates initialization of the RenderFrame object in the renderer process
+  // and ensures internal state of RenderFrameHost is ready for simulating
+  // RenderFrame originated IPCs.
+  void InitializeRenderFrameIfNeeded();
+
   // Advances the RenderFrameHost (and through it the RenderFrameHostManager) to
   // a state where a new navigation can be committed by a renderer. Currently,
   // this simulates a BeforeUnload ACK from the renderer.
diff --git a/content/test/test_render_view_host.cc b/content/test/test_render_view_host.cc
index 0b7b8c1..d729fb6 100644
--- a/content/test/test_render_view_host.cc
+++ b/content/test/test_render_view_host.cc
@@ -246,6 +246,10 @@ bool TestRenderViewHost::CreateRenderView(
   set_renderer_initialized(true);
   DCHECK(IsRenderViewLive());
   opener_route_id_ = opener_route_id;
+  RenderFrameHost* main_frame = GetMainFrame();
+  if (main_frame)
+    static_cast<RenderFrameHostImpl*>(main_frame)->SetRenderFrameCreated(true);
+
   return true;
 }
 
diff --git a/content/test/test_web_contents.cc b/content/test/test_web_contents.cc
index 2643132..2f35a8b 100644
--- a/content/test/test_web_contents.cc
+++ b/content/test/test_web_contents.cc
@@ -38,7 +38,6 @@ TestWebContents* TestWebContents::Create(BrowserContext* browser_context,
                                          SiteInstance* instance) {
   TestWebContents* test_web_contents = new TestWebContents(browser_context);
   test_web_contents->Init(WebContents::CreateParams(browser_context, instance));
-  test_web_contents->RenderFrameCreated(test_web_contents->GetMainFrame());
   return test_web_contents;
 }
 
@@ -106,9 +105,10 @@ void TestWebContents::TestDidNavigateWithReferrer(
   params.is_post = false;
   params.page_state = PageState::CreateFromURL(url);
 
-  RenderFrameHostImpl* rfhi =
-      static_cast<RenderFrameHostImpl*>(render_frame_host);
-  rfhi->frame_tree_node()->navigator()->DidNavigate(rfhi, params);
+  TestRenderFrameHost* rfh =
+      static_cast<TestRenderFrameHost*>(render_frame_host);
+  rfh->InitializeRenderFrameIfNeeded();
+  rfh->frame_tree_node()->navigator()->DidNavigate(rfh, params);
 }
 
 const std::string& TestWebContents::GetSaveFrameHeaders() {