Commit d6297f27 authored by Arthur Hemery's avatar Arthur Hemery Committed by Commit Bot

[navigation] Removing IsPerNavigationMojoInterface in individual tests.

NavigationClient is now on by default, so removing an unused branch in tests.

Also removing a test that does not make sense anymore.

Bug: 784904
Change-Id: Ibf91fdfbdc23d03a9f5a966ca02c82314fcdca97
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1821558
Commit-Queue: Arthur Hemery <ahemery@chromium.org>
Reviewed-by: default avatarNasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#708177}
parent ea7a9bad
...@@ -1602,17 +1602,11 @@ IN_PROC_BROWSER_TEST_F(SecurityExploitViaDisabledWebSecurityTest, ...@@ -1602,17 +1602,11 @@ IN_PROC_BROWSER_TEST_F(SecurityExploitViaDisabledWebSecurityTest,
RenderProcessHostKillWaiter process_kill_waiter(rfh->GetProcess()); RenderProcessHostKillWaiter process_kill_waiter(rfh->GetProcess());
mojo::PendingAssociatedRemote<mojom::NavigationClient> navigation_client; mojo::PendingAssociatedRemote<mojom::NavigationClient> navigation_client;
if (IsPerNavigationMojoInterfaceEnabled()) { auto navigation_client_receiver =
auto navigation_client_receiver = navigation_client.InitWithNewEndpointAndPassReceiver();
navigation_client.InitWithNewEndpointAndPassReceiver(); rfh->frame_host_receiver_for_testing().impl()->BeginNavigation(
rfh->frame_host_receiver_for_testing().impl()->BeginNavigation( std::move(common_params), std::move(begin_params), mojo::NullRemote(),
std::move(common_params), std::move(begin_params), mojo::NullRemote(), std::move(navigation_client), mojo::NullRemote());
std::move(navigation_client), mojo::NullRemote());
} else {
rfh->frame_host_receiver_for_testing().impl()->BeginNavigation(
std::move(common_params), std::move(begin_params), mojo::NullRemote(),
mojo::NullAssociatedRemote(), mojo::NullRemote());
}
EXPECT_EQ(bad_message::RFH_BASE_URL_FOR_DATA_URL_SPECIFIED, EXPECT_EQ(bad_message::RFH_BASE_URL_FOR_DATA_URL_SPECIFIED,
process_kill_waiter.Wait()); process_kill_waiter.Wait());
......
...@@ -11843,109 +11843,6 @@ class CommitMessageOrderReverser : public DidCommitNavigationInterceptor { ...@@ -11843,109 +11843,6 @@ class CommitMessageOrderReverser : public DidCommitNavigationInterceptor {
} // namespace } // namespace
// Regression test for https://crbug.com/877239, simulating the following
// scenario:
//
// 1) http://a.com/empty.html is loaded in a main frame.
// 2) Dynamically by JS, a same-site child frame is added:
// <iframe 'src=http://a.com/title1.html'/>.
// 3) The initial byte of the response for `title1.html` arrives, causing
// FrameMsg_CommitNavigation to be sent to the same renderer.
// 4) Just before processing this message, however, `main.html` navigates
// the iframe to http://baz.com/title2.html, which results in mojom::Frame::
// BeginNavigation being called on the RenderFrameHost.
// 5) Suppose that immediately afterwards, `main.html` enters a busy-loop.
// 6) The cross site navigation in the child frame starts, the first response
// byte arrives quickly, and thus the navigation commits quickly.
// 6.1) FrameTreeNode::has_committed_real_load is set to true for the child.
// 6.2) The same-site RenderFrame in the child FrameTreeNode is swapped out,
// i.e. UnfreezableFrameMsg_SwapOut is sent.
// 7) The renderer for site instance `a.com` exits from the busy loop,
// and starts processing messages in order:
// 7.1) The first being processed is FrameMsg_CommitNavigation, so a
// provisional load is created and immediately committed to
// http://a.com/title1.html.
// 7.2) Because at the time the same-site child RenderFrame was created,
// there had been no real load committed in the child frame, and because
// the navigation from the initial empty document to the first real
// document was same-origin, the global object is reused and the
// RemoteInterfaceProvider of the RenderFrame is not rebound.
// 7.3) The obsoleted load in the same-site child frame commits, calling
// mojom::Frame::DidCommitProvisionalLoad, however, with
// |interface_provider_request| being null.
// 8) RenderFrameHostImpl::DidCommitProvisionalLoad sees that a real load was
// already committed in the frame, but |interface_provider_request| is
// missing. However, it also sees that the frame was waiting for a swap-out
// ACK, so ignores the commit, and does not kill the renderer process.
//
// In the simulation of this scenario, we simulate (5) not by delaying
// renderer-side processing of the CommmitNavigation message, but by delaying
// browser-side processing of the response to it, of DidCommitProvisionalLoad.
IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
InterfaceProviderRequestIsOptionalForRaceyFirstCommits) {
// This test does not make sense anymore for the new NavigationClient way
// of doing navigations. The first iframe navigation will get instantly
// destroyed by the NavigationClient disconnection handle
// NavigationRequest::OnRendererAbortedNavigation.
// The pipe gets closed in the renderer when the javascript starts the
// second navigation.
// TODO(ahemery): Remove this test when IsPerNavigationMojoInterfaceEnabled
// is on by default.
if (IsPerNavigationMojoInterfaceEnabled())
return;
const GURL kMainFrameUrl(
embedded_test_server()->GetURL("a.com", "/empty.html"));
const GURL kSubframeSameSiteUrl(
embedded_test_server()->GetURL("a.com", "/title1.html"));
const GURL kCrossSiteSubframeUrl(
embedded_test_server()->GetURL("baz.com", "/title2.html"));
const auto kAddSameSiteDynamicSubframe = JsReplace(
"var f = document.createElement(\"iframe\");"
"f.src=$1;"
"document.body.append(f);",
kSubframeSameSiteUrl);
const auto kNavigateSubframeCrossSite =
JsReplace("f.src = $1;", kCrossSiteSubframeUrl);
const std::string kExtractSubframeUrl =
"window.domAutomationController.send(f.src);";
ASSERT_TRUE(NavigateToURL(shell(), kMainFrameUrl));
const auto* main_rfh_site_instance =
shell()->web_contents()->GetMainFrame()->GetSiteInstance();
auto did_start_deferring_commit_callback =
base::BindLambdaForTesting([&](RenderFrameHost* subframe_rfh) {
// Verify that the subframe starts out as same-process with its parent.
ASSERT_EQ(main_rfh_site_instance, subframe_rfh->GetSiteInstance());
// Trigger the second commit now that we are deferring the first one.
ASSERT_TRUE(ExecuteScript(shell(), kNavigateSubframeCrossSite));
});
CommitMessageOrderReverser commit_order_reverser(
shell()->web_contents(), kSubframeSameSiteUrl /* deferred_url */,
std::move(did_start_deferring_commit_callback));
ASSERT_TRUE(ExecuteScript(shell(), kAddSameSiteDynamicSubframe));
commit_order_reverser.WaitForBothCommits();
// Verify that:
// - The cross-site navigation in the sub-frame was committed and the
// same-site navigation was ignored.
// - The parent frame thinks so, too.
// - The renderer process corresponding to the sub-frame with the ignored
// commit was not killed. This is verified implicitly: this is the same
// renderer process where the parent RenderFrame lives, so if the call to
// ExecuteScriptAndExtractString succeeds here, the process is still alive.
std::string actual_subframe_url;
ASSERT_TRUE(ExecuteScriptAndExtractString(shell(), kExtractSubframeUrl,
&actual_subframe_url));
EXPECT_EQ(kCrossSiteSubframeUrl.spec(), actual_subframe_url);
}
// Create an out-of-process iframe that causes itself to be detached during // Create an out-of-process iframe that causes itself to be detached during
// its layout/animate phase. See https://crbug.com/802932. // its layout/animate phase. See https://crbug.com/802932.
// Disabled on Android due to flakiness, https://crbug.com/809580. // Disabled on Android due to flakiness, https://crbug.com/809580.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment