content/browser/frame_host/navigator_impl_unittest.cc - Issue 946543003: PlzNavigate: have renderer-initiated navigations be same-process

Unified Diff: content/browser/frame_host/navigator_impl_unittest.cc

Issue 946543003: PlzNavigate: have renderer-initiated navigations be same-process (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Created 5 years, 10 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

« content/browser/frame_host/navigator_impl.cc ('K') | « content/browser/frame_host/navigator_impl.cc ('k') | content/browser/frame_host/render_frame_host_manager.cc » ('j') | content/browser/frame_host/render_frame_host_manager.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: content/browser/frame_host/navigator_impl_unittest.cc

diff --git a/content/browser/frame_host/navigator_impl_unittest.cc b/content/browser/frame_host/navigator_impl_unittest.cc

index 82a49874ec9561ca5d6d17ec8a0aeb1c70cb8cce..783707a3bae1b9490853eff6fb81847539e7786f 100644

--- a/content/browser/frame_host/navigator_impl_unittest.cc

+++ b/content/browser/frame_host/navigator_impl_unittest.cc

@@ -421,21 +421,14 @@ TEST_F(NavigatorTestWithBrowserSideNavigation, RedirectCrossSite) {

EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));

// It then redirects to another site.

- net::RedirectInfo redirect_info;

- redirect_info.status_code = 302;

- redirect_info.new_method = "GET";

- redirect_info.new_url = kUrl2;

- redirect_info.new_first_party_for_cookies = kUrl2;

- scoped_refptr<ResourceResponse> response(new ResourceResponse);

- GetLoaderForNavigationRequest(main_request)->CallOnRequestRedirected(

- redirect_info, response);

+ GetLoaderForNavigationRequest(main_request)->SimulateServerRedirect(kUrl2);

// The redirect should have been followed.

EXPECT_EQ(1, GetLoaderForNavigationRequest(main_request)->redirect_count());

EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));

// Have the RenderFrameHost commit the navigation.

- response = new ResourceResponse;

+ scoped_refptr<ResourceResponse> response(new ResourceResponse);

GetLoaderForNavigationRequest(main_request)->CallOnResponseStarted(

response, MakeEmptyStream());

TestRenderFrameHost* final_speculative_rfh =

@@ -550,9 +543,7 @@ TEST_F(NavigatorTestWithBrowserSideNavigation,

EXPECT_TRUE(loader1);

// Confirm a speculative RenderFrameHost was created.

- TestRenderFrameHost* speculative_rfh = GetSpeculativeRenderFrameHost(node);

- ASSERT_TRUE(speculative_rfh);

- int32 site_instance_id_1 = speculative_rfh->GetSiteInstance()->GetId();

+ ASSERT_TRUE(GetSpeculativeRenderFrameHost(node));

// Now receive a renderer-initiated user-initiated request. It should replace

// the current NavigationRequest.

@@ -566,28 +557,23 @@ TEST_F(NavigatorTestWithBrowserSideNavigation,

// Confirm that the first loader got destroyed.

EXPECT_FALSE(loader1);

- // Confirm that a new speculative RenderFrameHost was created.

- speculative_rfh = GetSpeculativeRenderFrameHost(node);

- ASSERT_TRUE(speculative_rfh);

- int32 site_instance_id_2 = speculative_rfh->GetSiteInstance()->GetId();

- EXPECT_NE(site_instance_id_1, site_instance_id_2);

+ // Confirm that a the speculative RenderFrameHost was destroyed.

+ // TODO(carlosk): Once OOPIF adds support for renderer-initiated cross-process

clamy 2015/02/23 10:52:34 I think the TODO is not needed here. The test will

carlosk 2015/03/04 19:42:40 Done.

+ // navigations, re-add existence and update checks for the speculative RFH.

+ ASSERT_FALSE(GetSpeculativeRenderFrameHost(node));

clamy 2015/02/23 10:52:34 I would use an EXPECT instead of an ASSERT here.

carlosk 2015/03/04 19:42:40 Done here and in 2 other places below.

// Have the RenderFrameHost commit the navigation.

scoped_refptr<ResourceResponse> response(new ResourceResponse);

GetLoaderForNavigationRequest(request2)

->CallOnResponseStarted(response, MakeEmptyStream());

- EXPECT_TRUE(DidRenderFrameHostRequestCommit(speculative_rfh));

- EXPECT_FALSE(DidRenderFrameHostRequestCommit(main_test_rfh()));

+ EXPECT_TRUE(DidRenderFrameHostRequestCommit(main_test_rfh()));

// Commit the navigation.

- speculative_rfh->SendNavigate(0, kUrl2);

+ main_test_rfh()->SendNavigate(0, kUrl2);

// Confirm that the commit corresponds to the new request.

ASSERT_TRUE(main_test_rfh());

EXPECT_EQ(kUrl2, contents()->GetLastCommittedURL());

- // Confirm that the committed RenderFrameHost is the latest speculative one.

- EXPECT_EQ(site_instance_id_2, main_test_rfh()->GetSiteInstance()->GetId());

}

// PlzNavigate: Test that a renderer-initiated user-initiated navigation is NOT

@@ -611,7 +597,9 @@ TEST_F(NavigatorTestWithBrowserSideNavigation,

EXPECT_EQ(kUrl1, request1->common_params().url);

EXPECT_FALSE(request1->browser_initiated());

EXPECT_TRUE(request1->begin_params().has_user_gesture);

- EXPECT_TRUE(GetSpeculativeRenderFrameHost(node));

+ // TODO(carlosk): Once OOPIF adds support for renderer-initiated cross-process

+ // navigations, re-add existence and update checks for the speculative RFH.

clamy 2015/02/23 10:52:34 Same here, no need for the TODO here and below.

carlosk 2015/03/04 19:42:40 Done.

+ EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));

// Now receive a renderer-initiated non-user-initiated request. Nothing should

// change.

@@ -622,18 +610,16 @@ TEST_F(NavigatorTestWithBrowserSideNavigation,

EXPECT_EQ(kUrl1, request2->common_params().url);

EXPECT_FALSE(request2->browser_initiated());

EXPECT_TRUE(request2->begin_params().has_user_gesture);

- TestRenderFrameHost* speculative_rfh = GetSpeculativeRenderFrameHost(node);

- ASSERT_TRUE(speculative_rfh);

+ ASSERT_FALSE(GetSpeculativeRenderFrameHost(node));

clamy 2015/02/23 10:52:34 I would use an EXPECT here and below.

carlosk 2015/03/04 19:42:40 Done.

// Have the RenderFrameHost commit the navigation.

scoped_refptr<ResourceResponse> response(new ResourceResponse);

GetLoaderForNavigationRequest(request2)

->CallOnResponseStarted(response, MakeEmptyStream());

- EXPECT_TRUE(DidRenderFrameHostRequestCommit(speculative_rfh));

- EXPECT_FALSE(DidRenderFrameHostRequestCommit(main_test_rfh()));

+ EXPECT_TRUE(DidRenderFrameHostRequestCommit(main_test_rfh()));

// Commit the navigation.

- speculative_rfh->SendNavigate(0, kUrl1);

+ main_test_rfh()->SendNavigate(0, kUrl1);

EXPECT_EQ(kUrl1, contents()->GetLastCommittedURL());

}

@@ -705,7 +691,9 @@ TEST_F(NavigatorTestWithBrowserSideNavigation,

EXPECT_EQ(kUrl1, request1->common_params().url);

EXPECT_FALSE(request1->browser_initiated());

EXPECT_FALSE(request1->begin_params().has_user_gesture);

- EXPECT_TRUE(GetSpeculativeRenderFrameHost(node));

+ // TODO(carlosk): Once OOPIF adds support for renderer-initiated cross-process

+ // navigations, re-add existence and update checks for the speculative RFH.

+ EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));

base::WeakPtr<TestNavigationURLLoader> loader1 =

GetLoaderForNavigationRequest(request1)->AsWeakPtr();

EXPECT_TRUE(loader1);

@@ -716,8 +704,7 @@ TEST_F(NavigatorTestWithBrowserSideNavigation,

EXPECT_EQ(kUrl2, request2->common_params().url);

EXPECT_FALSE(request2->browser_initiated());

EXPECT_FALSE(request2->begin_params().has_user_gesture);

- TestRenderFrameHost* speculative_rfh = GetSpeculativeRenderFrameHost(node);

- ASSERT_TRUE(speculative_rfh);

+ ASSERT_FALSE(GetSpeculativeRenderFrameHost(node));

// Confirm that the first loader got destroyed.

EXPECT_FALSE(loader1);

@@ -726,11 +713,10 @@ TEST_F(NavigatorTestWithBrowserSideNavigation,

scoped_refptr<ResourceResponse> response(new ResourceResponse);

GetLoaderForNavigationRequest(request2)

->CallOnResponseStarted(response, MakeEmptyStream());

- EXPECT_TRUE(DidRenderFrameHostRequestCommit(speculative_rfh));

- EXPECT_FALSE(DidRenderFrameHostRequestCommit(main_test_rfh()));

+ EXPECT_TRUE(DidRenderFrameHostRequestCommit(main_test_rfh()));

// Commit the navigation.

- speculative_rfh->SendNavigate(0, kUrl2);

+ main_test_rfh()->SendNavigate(0, kUrl2);

EXPECT_EQ(kUrl2, contents()->GetLastCommittedURL());

}

@@ -843,14 +829,8 @@ TEST_F(NavigatorTestWithBrowserSideNavigation,

NavigationRequest* main_request = GetNavigationRequestForFrameTreeNode(node);

ASSERT_TRUE(main_request);

const GURL kUrlRedirect("https://www.google.com/");

- net::RedirectInfo redirect_info;

- redirect_info.status_code = 302;

- redirect_info.new_method = "GET";

- redirect_info.new_url = kUrlRedirect;

- redirect_info.new_first_party_for_cookies = kUrlRedirect;

- scoped_refptr<ResourceResponse> response(new ResourceResponse);

GetLoaderForNavigationRequest(main_request)

- ->CallOnRequestRedirected(redirect_info, response);

+ ->SimulateServerRedirect(kUrlRedirect);

EXPECT_EQ(init_site_instance_id, main_test_rfh()->GetSiteInstance()->GetId());

speculative_rfh = GetSpeculativeRenderFrameHost(node);

ASSERT_TRUE(speculative_rfh);

@@ -863,7 +843,7 @@ TEST_F(NavigatorTestWithBrowserSideNavigation,

// Commit the navigation with Navigator by simulating the call to

// OnResponseStarted.

- response = new ResourceResponse;

+ scoped_refptr<ResourceResponse> response(new ResourceResponse);

GetLoaderForNavigationRequest(main_request)

->CallOnResponseStarted(response, MakeEmptyStream());

speculative_rfh = GetSpeculativeRenderFrameHost(node);