New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 590782 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows
Pri: 2
Type: Bug
Proj-Servicification

Blocking:
issue 417518
issue 236848



Sign in to add a comment

BrowserSideNavigationBrowserTest too eager to assert presence of OOPIFs?

Project Member Reported by lukasza@chromium.org, Feb 29 2016

Issue description

Site isolation FYI bot runs content_browsertests with --isolate-sites-for-testing=*.is flag.  nasko@ / alexmos@ / nick@ would have more context on why this particular flag was chosen.

On Friday, BrowserSideNavigationBrowserTest tests started to fail on the bot: https://build.chromium.org/p/chromium.fyi/builders/Site%20Isolation%20Win/builds/13330

Interestingly, the tests continue to pass with --site-per-process or --isolate-extensions flags.  The tests fail with --isolate-sites-for-testing=*.is

I suspect that the test assertions are too eagerly expecting an out-of-process iframe (when some navigations might happen in-process for some modes).

Test failures from https://build.chromium.org/p/chromium.fyi/builders/Site%20Isolation%20Win/builds/13330/steps/content_browsertests/logs/stdio:

[ RUN      ] BrowserSideNavigationBrowserTest.BrowserInitiatedNavigations
e:\b\build\slave\site_isolation_win\build\src\content\browser\browser_side_navigation_browsertest.cc(81): error: Expected: (initial_rfh) != (static_cast<WebContentsImpl*>(shell()->web_contents()) ->GetFrameTree()->root()->current_frame_host()), actual: 0000000004ED7FE0 vs 0000000004ED7FE0
[  FAILED  ] BrowserSideNavigationBrowserTest.BrowserInitiatedNavigations, where TypeParam =  and GetParam() =  (326 ms)

[ RUN      ] BrowserSideNavigationBrowserTest.RendererInitiatedCrossSiteNavigation
e:\b\build\slave\site_isolation_win\build\src\content\browser\browser_side_navigation_browsertest.cc(167): error: Expected: (initial_rfh) != (static_cast<WebContentsImpl*>(shell()->web_contents()) ->GetFrameTree() ->root() ->current_frame_host()), actual: 0000000005286530 vs 0000000005286530
[  FAILED  ] BrowserSideNavigationBrowserTest.RendererInitiatedCrossSiteNavigation, where TypeParam =  and GetParam() =  (326 ms)


 
Cc: nick@chromium.org alex...@chromium.org nasko@chromium.org
Owner: clamy@chromium.org
clamy@, could you please take a look?  The failing test assertions come from your crrev.com/1722623003.
Summary: BrowserSideNavigationBrowserTest too eager to assert presence of OOPIFs? (was: BrowserSideNavigationBrowserTest fails with --isolate-sites-for-testing=*.is on Windows)
Blocking: 417518
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 1 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1fe380c0e5012e03e5d1e5ad5d831fc65d30e0b6

commit 1fe380c0e5012e03e5d1e5ad5d831fc65d30e0b6
Author: lukasza <lukasza@chromium.org>
Date: Tue Mar 01 00:34:15 2016

Disabling 2 BrowserSideNavigationBrowserTest tests.

BUG= 590782 

Review URL: https://codereview.chromium.org/1747983002

Cr-Commit-Position: refs/heads/master@{#378342}

[modify] https://crrev.com/1fe380c0e5012e03e5d1e5ad5d831fc65d30e0b6/testing/buildbot/filters/isolate-extensions.content_browsertests.filter

Comment 5 by creis@chromium.org, May 4 2016

Blocking: 236848
Cc: clamy@chromium.org
Owner: creis@chromium.org
Status: Started (was: Untriaged)
I'll take this, since it's blocking  issue 236848 .

For reference, we use --isolate-sites-for-testing=*.is on content_browsertests to simulate --isolate-extensions, because --isolate-extensions does nothing within content/.  It only affects builds with extensions, and thus browser_tests.

RendererInitiatedCrossSiteNavigation was failing because it uses SiteIsolationPolicy::AreCrossProcessFramesPossible() instead of AreAllSitesIsolatedForTesting().  The former is true in modes that we don't use OOPIFs for all cross-site frames, so the test had the wrong expectation.

I'm not sure why BrowserInitiatedNavigations was failing.  That doesn't have the same issue, and it's passing for me locally with --isolate-sites-for-testing=*.is.  Looking at the test, I don't see anything it's doing wrong, since we should do a cross-process navigation in all modes for a cross-site browser-initiated main frame navigation.  I'll try turning it back on to see what happens, since the log files are gone.

I have a CL to fix the renderer test and re-enable both tests:
https://codereview.chromium.org/1948013002/
Project Member

Comment 6 by bugdroid1@chromium.org, May 4 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/53b4ec38aa64684d39d67bc5148427b26036a8bc

commit 53b4ec38aa64684d39d67bc5148427b26036a8bc
Author: creis <creis@chromium.org>
Date: Wed May 04 20:47:42 2016

Fix failing RendererInitiatedCrossSiteNavigation test.

This PlzNavigate test is failing in the equivalent of
--isolate-extensions mode (i.e., --isolate-sites-for-testing=*.is),
which creates OOPIFs in some but not all cases.

BUG= 590782 
TEST=Site Isolation Win FYI bot stays green.

Review-Url: https://codereview.chromium.org/1948013002
Cr-Commit-Position: refs/heads/master@{#391632}

[modify] https://crrev.com/53b4ec38aa64684d39d67bc5148427b26036a8bc/content/browser/browser_side_navigation_browsertest.cc
[modify] https://crrev.com/53b4ec38aa64684d39d67bc5148427b26036a8bc/testing/buildbot/filters/isolate-extensions.content_browsertests.filter

Comment 7 by creis@chromium.org, May 6 2016

Status: Fixed (was: Started)

Comment 8 by laforge@google.com, Nov 7 2017

Components: Internals>Network>Service

Comment 9 by laforge@google.com, Nov 7 2017

Components: -Internals>Network>Service Internals>Services>Network
Apologies, applied the wrong component in bulk.

Sign in to add a comment