Commit 8e0e1ee3 authored by Nasko Oskov's avatar Nasko Oskov Committed by Commit Bot

Enable SecurityExploitBrowserTest.DidCommitInvalidURL test.

The test was flaky because it uses DidCommitUrlReplacer which expects
all RenderFrameHosts it sees being deleted to have been created. The
main navigation to the starting test page causes a cross-process
navigation and if the test system is slow, the UnloadACK IPC can be
delayed until the pointe after which the DidCommitUrlReplacer is
created. If that happens, the DCHECK in the RenderFrameDeleted
implementation is hit.

The fix for the test is to wait for the frame to be deleted, which will
guarantee that the DidCommitUrlReplacer will not see the frame deletion
regardless of how slow the machine runs.

Bug: 1106893
Change-Id: I3174f8c2fc11bc69db226c59065e214dfc58705c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2304554Reviewed-by: default avatarAlex Moshchuk <alexmos@chromium.org>
Commit-Queue: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#790182}
parent 858fdd8f
......@@ -1178,17 +1178,26 @@ class DidCommitUrlReplacer : public DidCommitNavigationInterceptor {
// Test which verifies that when an exploited renderer process sends a commit
// message with URL that the process is not allowed to commit.
// Disabled as per crbug.com/1106893.
IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest,
DISABLED_DidCommitInvalidURL) {
IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest, DidCommitInvalidURL) {
// Explicitly isolating foo.com helps ensure that this test is applicable on
// platforms without site-per-process.
IsolateOrigin("foo.com");
RenderFrameDeletedObserver initial_frame_deleted_observer(
shell()->web_contents()->GetMainFrame());
// Navigate to foo.com initially.
GURL foo_url(embedded_test_server()->GetURL("foo.com", "/title1.html"));
EXPECT_TRUE(NavigateToURL(shell(), foo_url));
// Wait for the RenderFrameHost which was current before the navigation to
// foo.com to be deleted. This is necessary, since on a slow system the
// UnloadACK event can arrive after the DidCommitUrlReplacer instance below
// is created. The replacer code has checks to ensure that all frames being
// deleted it has seen being created, which with delayed UnloadACK is
// violated.
initial_frame_deleted_observer.WaitUntilDeleted();
// Create the interceptor object which will replace the URL of the subsequent
// navigation with bar.com based URL.
GURL bar_url(embedded_test_server()->GetURL("bar.com", "/title3.html"));
......
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