diff options
author | nasko <nasko@chromium.org> | 2015-06-02 06:44:48 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-06-02 13:45:25 +0000 |
commit | 8c95dcf5164de33fd42d29567adb24655b4c2ab0 (patch) | |
tree | 6f1a41dad24d4444eb70c076d8c4890171cd6e5b | |
parent | 857d18c87d6cae264e7fe5c771fd5306a90b13d1 (diff) | |
download | chromium_src-8c95dcf5164de33fd42d29567adb24655b4c2ab0.zip chromium_src-8c95dcf5164de33fd42d29567adb24655b4c2ab0.tar.gz chromium_src-8c95dcf5164de33fd42d29567adb24655b4c2ab0.tar.bz2 |
Bring RFH/RVH unit tests closer to reality of how RF/RV are initialized
This is an attempt at relanding https://codereview.chromium.org/1151973005,
which broke official builds due to RlzLibTest unit tests failing.
The initial patchset is identical to the previously committed CL and
subsequent uploads are changes made to fix the failures.
BUG=357747
TEST=all unit tests continue passing
Review URL: https://codereview.chromium.org/1156023006
Cr-Commit-Position: refs/heads/master@{#332380}
13 files changed, 49 insertions, 28 deletions
diff --git a/chrome/browser/rlz/rlz_unittest.cc b/chrome/browser/rlz/rlz_unittest.cc index 5dac40b..a1fe65a 100644 --- a/chrome/browser/rlz/rlz_unittest.cc +++ b/chrome/browser/rlz/rlz_unittest.cc @@ -271,6 +271,10 @@ void RlzLibTest::SimulateHomepageUsage() { content::RenderFrameHostTester* rfht = content::RenderFrameHostTester::For(main_rfh()); + // Ensure the RenderFrame is initialized before simulating events coming from + // it. + rfht->InitializeRenderFrameIfNeeded(); + // Simulate a navigation to homepage first. rfht->SendNavigateWithTransition( 0, 0, true, home_url, ui::PAGE_TRANSITION_HOME_PAGE); 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 ff73239..13d5bf4 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 35c21b0..18bf1e4 100644 --- a/content/browser/frame_host/render_frame_host_impl.cc +++ b/content/browser/frame_host/render_frame_host_impl.cc @@ -650,12 +650,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 b4747a5..fdcbdec 100644 --- a/content/browser/web_contents/web_contents_impl_unittest.cc +++ b/content/browser/web_contents/web_contents_impl_unittest.cc @@ -1689,6 +1689,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()); @@ -3023,6 +3024,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/public/test/test_renderer_host.h b/content/public/test/test_renderer_host.h index 001a48e..9945d87 100644 --- a/content/public/test/test_renderer_host.h +++ b/content/public/test/test_renderer_host.h @@ -66,6 +66,11 @@ class RenderFrameHostTester { virtual ~RenderFrameHostTester() {} + // Simulates initialization of the RenderFrame object in the renderer process + // and ensures internal state of RenderFrameHost is ready for simulating + // RenderFrame originated IPCs. + virtual void InitializeRenderFrameIfNeeded() = 0; + // Gives tests access to RenderFrameHostImpl::OnCreateChild. The returned // RenderFrameHost is owned by the parent RenderFrameHost. virtual RenderFrameHost* AppendChild(const std::string& frame_name) = 0; diff --git a/content/test/test_render_frame_host.cc b/content/test/test_render_frame_host.cc index 5b7aa53..6badf47 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() { @@ -73,6 +70,13 @@ MockRenderProcessHost* TestRenderFrameHost::GetProcess() { return static_cast<MockRenderProcessHost*>(RenderFrameHostImpl::GetProcess()); } +void TestRenderFrameHost::InitializeRenderFrameIfNeeded() { + if (!render_view_host()->IsRenderViewLive()) { + RenderViewHostTester::For(render_view_host())->CreateRenderView( + base::string16(), MSG_ROUTING_NONE, MSG_ROUTING_NONE, -1, false); + } +} + TestRenderFrameHost* TestRenderFrameHost::AppendChild( const std::string& frame_name) { OnCreateChildFrame(GetProcess()->GetNextRoutingID(), @@ -132,11 +136,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 +148,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 +241,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, diff --git a/content/test/test_render_frame_host.h b/content/test/test_render_frame_host.h index 3b0c9ae..233d501 100644 --- a/content/test/test_render_frame_host.h +++ b/content/test/test_render_frame_host.h @@ -51,6 +51,7 @@ class TestRenderFrameHost : public RenderFrameHostImpl, MockRenderProcessHost* GetProcess() override; // RenderFrameHostTester implementation. + void InitializeRenderFrameIfNeeded() override; TestRenderFrameHost* AppendChild(const std::string& frame_name) override; void SendNavigate(int page_id, int nav_entry_id, 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() { |