Commit 9cd6ac25 authored by Fergal Daly's avatar Fergal Daly Committed by Commit Bot

Enable RenderDocument for crashed frames by default.

Analysis of 50/50 experiment is in https://crbug.com/1102233.

Fix 2 tests.

This reenables
CrossOriginOpenerPolicyBrowserTest.CoopPageCrashIntoNonCoop.

This won't be submitted until it's clear that https://crbug.com/1139104
is caused by early commit.

Bug: 1102233,936696,1066376
Change-Id: Id1f96e53f163d50d873819bbf2bd0fd826b7332e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2483730Reviewed-by: default avatarAlex Moshchuk <alexmos@chromium.org>
Reviewed-by: default avatarAlexander Timin <altimin@chromium.org>
Commit-Queue: Fergal Daly <fergal@chromium.org>
Auto-Submit: Fergal Daly <fergal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#820664}
parent 791b3e31
...@@ -526,9 +526,6 @@ IN_PROC_BROWSER_TEST_P(CrossOriginOpenerPolicyBrowserTest, ...@@ -526,9 +526,6 @@ IN_PROC_BROWSER_TEST_P(CrossOriginOpenerPolicyBrowserTest,
IN_PROC_BROWSER_TEST_P(CrossOriginOpenerPolicyBrowserTest, IN_PROC_BROWSER_TEST_P(CrossOriginOpenerPolicyBrowserTest,
CoopPageCrashIntoNonCoop) { CoopPageCrashIntoNonCoop) {
// TODO(http://crbug.com/1066376): Remove this when the test case passes.
if (ShouldCreateNewHostForCrashedFrame())
return;
IsolateAllSitesForTesting(base::CommandLine::ForCurrentProcess()); IsolateAllSitesForTesting(base::CommandLine::ForCurrentProcess());
GURL non_coop_page(https_server()->GetURL("a.com", "/title1.html")); GURL non_coop_page(https_server()->GetURL("a.com", "/title1.html"));
GURL coop_page = https_server()->GetURL( GURL coop_page = https_server()->GetURL(
......
...@@ -3813,6 +3813,8 @@ IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest, ...@@ -3813,6 +3813,8 @@ IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest,
// 5) Let the navigation finish and make sure it is succeeded. // 5) Let the navigation finish and make sure it is succeeded.
manager.WaitForNavigationFinished(); manager.WaitForNavigationFinished();
EXPECT_EQ(url_b, web_contents()->GetMainFrame()->GetLastCommittedURL()); EXPECT_EQ(url_b, web_contents()->GetMainFrame()->GetLastCommittedURL());
// The RenderFrameHost has been replaced after the crash, so get it again.
current_rfh = root->render_manager()->current_frame_host();
EXPECT_EQ(LifecycleState::kActive, current_rfh->lifecycle_state()); EXPECT_EQ(LifecycleState::kActive, current_rfh->lifecycle_state());
} }
......
...@@ -285,11 +285,16 @@ class RenderDocumentFeatureTest : public testing::Test { ...@@ -285,11 +285,16 @@ class RenderDocumentFeatureTest : public testing::Test {
GetRenderDocumentLevelName(level)); GetRenderDocumentLevelName(level));
} }
void DisableRenderDocument() {
feature_list_.InitAndDisableFeature(features::kRenderDocument);
}
private: private:
base::test::ScopedFeatureList feature_list_; base::test::ScopedFeatureList feature_list_;
}; };
TEST_F(RenderDocumentFeatureTest, FeatureDisabled) { TEST_F(RenderDocumentFeatureTest, FeatureDisabled) {
DisableRenderDocument();
EXPECT_FALSE(ShouldCreateNewHostForCrashedFrame()); EXPECT_FALSE(ShouldCreateNewHostForCrashedFrame());
EXPECT_FALSE(ShouldCreateNewHostForSameSiteSubframe()); EXPECT_FALSE(ShouldCreateNewHostForSameSiteSubframe());
} }
......
...@@ -404,6 +404,9 @@ IN_PROC_BROWSER_TEST_F(WebUIMojoTest, ChromeSendAvailable_AfterCrash) { ...@@ -404,6 +404,9 @@ IN_PROC_BROWSER_TEST_F(WebUIMojoTest, ChromeSendAvailable_AfterCrash) {
// available. // available.
EXPECT_TRUE(NavigateToURL(shell(), test_url)); EXPECT_TRUE(NavigateToURL(shell(), test_url));
EXPECT_TRUE(EvalJs(shell(), "isChromeSendAvailable()").ExtractBool()); EXPECT_TRUE(EvalJs(shell(), "isChromeSendAvailable()").ExtractBool());
// The RenderFrameHost has been replaced after the crash, so get web_ui again.
web_ui = static_cast<WebUIImpl*>(
shell()->web_contents()->GetMainFrame()->GetWebUI());
EXPECT_TRUE(web_ui->GetRemoteForTest().is_bound()); EXPECT_TRUE(web_ui->GetRemoteForTest().is_bound());
} }
......
...@@ -140,7 +140,7 @@ constexpr base::FeatureParam<RenderDocumentLevel>::Option ...@@ -140,7 +140,7 @@ constexpr base::FeatureParam<RenderDocumentLevel>::Option
{RenderDocumentLevel::kSubframe, "subframe"}}; {RenderDocumentLevel::kSubframe, "subframe"}};
const base::FeatureParam<RenderDocumentLevel> render_document_level{ const base::FeatureParam<RenderDocumentLevel> render_document_level{
&features::kRenderDocument, kRenderDocumentLevelParameterName, &features::kRenderDocument, kRenderDocumentLevelParameterName,
RenderDocumentLevel::kDisabled, &render_document_levels}; RenderDocumentLevel::kCrashedFrame, &render_document_levels};
RenderDocumentLevel GetRenderDocumentLevel() { RenderDocumentLevel GetRenderDocumentLevel() {
if (base::FeatureList::IsEnabled(features::kRenderDocument)) if (base::FeatureList::IsEnabled(features::kRenderDocument))
......
...@@ -535,7 +535,7 @@ const base::Feature kReloadHiddenTabsWithCrashedSubframes { ...@@ -535,7 +535,7 @@ const base::Feature kReloadHiddenTabsWithCrashedSubframes {
// Enable using the RenderDocument. // Enable using the RenderDocument.
const base::Feature kRenderDocument{"RenderDocument", const base::Feature kRenderDocument{"RenderDocument",
base::FEATURE_DISABLED_BY_DEFAULT}; base::FEATURE_ENABLED_BY_DEFAULT};
// Enables skipping the early call to CommitPending when navigating away from a // Enables skipping the early call to CommitPending when navigating away from a
// crashed frame. // crashed frame.
const base::Feature kSkipEarlyCommitPendingForCrashedFrame{ const base::Feature kSkipEarlyCommitPendingForCrashedFrame{
......
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