btw I'm taking a look at NavigationControllerBrowserTest.SubframeForwardRedirect

I've sent https://codereview.chromium.org/2594263004/ for NavigationControllerBrowserTest.SubframeForwardRedirect



I investigated FindInPageControllerTest.SearchWithinSpecialURL a bit. One initial problem seems to be traced to the ViewHostMsg_WebUISend IPC being dropped in the renderer because the RenderView is swapped out. The RenderViewHost for the chrome://history-frame is created in RenderFrameHostManager::CreateRenderFrameProxy as swapped out. But nowhere is it marked as not swapped out later. It's a bit hard for me to follow this code as I'm not familiar with all the scenarios that cause swapping out with OOPIFs, so I'll stop investigating that test.

nasko: have you had a chance to check if these issues are blockers for extensions' use of oopif?

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

commit 015ba065ac2806f71b4c3b9e05b6d9013f473de1
Author: jam <jam@chromium.org>
Date: Fri Jan 06 21:17:00 2017

Fix NavigationControllerBrowserTest.SubframeForwardRedirect with PlzNavigate and site isolation enabled.

The problem is that NavigationRequest::dest_site_instance_ is set in the constructor of that class. If RenderFrameHostManager::CanSubframeSwapProcess determines that a cross process redirect should happen for the subframe (because of site isolation) then we might need to use a new site instance.

BUG=674734
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/015ba065ac2806f71b4c3b9e05b6d9013f473de1/content/browser/frame_host/navigation_controller_impl.cc
[modify] https://crrev.com/015ba065ac2806f71b4c3b9e05b6d9013f473de1/content/browser/frame_host/navigation_request.cc

I'm starting to look at these failures more proactively today and will continue next week.

I've investigated the failure for ExtensionWebUITest.CanEmbedExtensionOptions. The cause of the failure is that GuestView is not allowed to be instantiated inside an out-of-process iframe and the extension process embedding its option is being killed and it doesn't respond back to the browser.

The reason why there are out-of-process iframes is that chrome://settings uses multiple frames with different hosts - chrome://uber-frame, chrome://extensions-frame, etc. Those are treated as different sites and we have had to put in a specific check for those cases to allow them to continue working in --site-per-process mode. However, this check is in the transfer logic, which is not used in PlzNavigate, therefore we end up with out-of-process iframes where we shouldn't be.

I'll put together a CL to fix this and I think it will take care of more than just that test.

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

commit c6eede7d4c764d788ca02a02ab45c0f72309905b
Author: nasko <nasko@chromium.org>
Date: Mon Jan 23 20:07:55 2017

Don't swap processes for chrome:// subframes.

When running in --site-per-process mode, cross-site subframes run in
different process from the parent and there is an exception for chrome://
URLs to ensure chrome://settings continues working.

In PlzNavigate mode, the check for keeping chrome:// subframes in the
same process as the parent was missing, since it is added in the logic
for transferring navigations, which is not used with PlzNavigate.

BUG=674734
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/c6eede7d4c764d788ca02a02ab45c0f72309905b/content/browser/frame_host/render_frame_host_manager.cc
[modify] https://crrev.com/c6eede7d4c764d788ca02a02ab45c0f72309905b/content/browser/frame_host/render_frame_host_manager_browsertest.cc

The fix from #c8 has taken care of most failing browser_tests. rdevlin.cronin@ is working on the last remaining one - ExtensionBindingsApiTest.ModuleSystem and should have a fix soon.

There are also a few unit_tests cases that are failing with PlzNavigate and --site-per-process:

CaptivePortalTabHelperTest.HttpsSubframeParallelError
CorePageLoadMetricsObserverTest.FailedProvisionalLoad
CaptivePortalTabHelperTest.HttpToHttpsRedirectTimeout
SBNavigationObserverTest.ServerRedirect

I'll take a look at those next.

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

commit 78d31a9fd4f364abc61bf19cbd8e7f6d10209c2a
Author: rdevlin.cronin <rdevlin.cronin@chromium.org>
Date: Tue Jan 31 01:13:08 2017

[Extensions] Handle null ScriptContexts in GetModuleSystem()

GetModuleSystem() returns an object associated with a different JS
context for testing purposes (and is also one of the "why do we even
have that lever"-type things that we'd like to get rid of, but that's a
separate issue). Without site isolation, embedded frames (even those
from non-accessible contexts) are all hosted within the same process,
and we can retrieve a ScriptContext for an embedded frame. With site
isolation, this isn't necessarily true - there may be no corresponding
script context for an embedded frame (when the frame is hosted in
another process).

In both cases, we should never return an object that is inaccessible to
the current context, and so a null context is perfectly valid. Handle
this in GetModuleSystem() and add a general comment above the
GetContextByV8Object() method in ScriptContextSet.

Fixes ExtensionBindingsApiTest.ModuleSystem for PlzNavigate +
SitePerProcess

BUG=674734

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

[modify] https://crrev.com/78d31a9fd4f364abc61bf19cbd8e7f6d10209c2a/extensions/renderer/script_context_set.h
[modify] https://crrev.com/78d31a9fd4f364abc61bf19cbd8e7f6d10209c2a/extensions/renderer/v8_context_native_handler.cc

Looks like
SitePerProcessIgnoreCertErrorsBrowserTest.ActiveMixedContentInIframe
fails on 
mac& win with plznavigate enabled, i.e. 
https://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/387060
https://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_rel_ng/builds/365227
https://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/381393

@nasko: The remaining unit tests failures are due to the issue with error pages not being transferred when PlzNavigate isn't enabled, right?

If that's the case, I'm working on better unit test helpers in https://codereview.chromium.org/2682313002/. If we switch those tests to use them, this should fix the issue.

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

commit 255cffbde1d757fb114a07b648ac72837a9cd7ad
Author: jam <jam@chromium.org>
Date: Wed Mar 01 03:21:53 2017

Disable 4 unit tests that are failing with PlzNavigate and Site Isolation.

These are
CaptivePortalTabHelperTest.HttpsSubframeParallelError
CorePageLoadMetricsObserverTest.FailedProvisionalLoad
CaptivePortalTabHelperTest.HttpToHttpsRedirectTimeout
SBNavigationObserverTest.ServerRedirect

These are the last test failures on desktop and it's suspected they'll be fixed by Camille's upcoming NavigationSimulator.

BUG=674734

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

[modify] https://crrev.com/255cffbde1d757fb114a07b648ac72837a9cd7ad/chrome/browser/captive_portal/captive_portal_tab_helper_unittest.cc
[modify] https://crrev.com/255cffbde1d757fb114a07b648ac72837a9cd7ad/chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer_unittest.cc
[modify] https://crrev.com/255cffbde1d757fb114a07b648ac72837a9cd7ad/chrome/browser/safe_browsing/safe_browsing_navigation_observer_unittest.cc

#c13 - I think the issue is that we call the SimulateNavigationErrorPageCommit() method on the wrong RFH, which fails expecting to have a NavigationHandle associated with it. I'll wait for your CL to land and see how porting it over works out.

Given that there are only unit tests that remain failing, it is usually due to incorrect test harness. It will be good to fix them, but I don't think they should be blocking the PlzNavigate launch, so removing the blocking bit.

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

commit 5b85dadde9f62a7f2f5933174175612cc99958ee
Author: Charles Reis <creis@chromium.org>
Date: Sat Oct 14 00:31:21 2017

Remove exception for OOPIFs on chrome:// WebUI pages.

This was necessary for the old version of chrome://settings which had
frames on different hosts, but no WebUI pages depend on this anymore
now that the Material Design settings page has launched.  The ChromeOS
OOBE page does have a chrome://terms iframe, but it works when loaded
as an OOPIF in --site-per-process mode.

BUG=674734
TEST=No behavior change

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: I4ef0e386f1fe053eb809ff162b2695f59a313754
Reviewed-on: https://chromium-review.googlesource.com/685461
Commit-Queue: Charlie Reis <creis@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Dan Beam (no longer on Chrome) <dbeam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508891}
[modify] https://crrev.com/5b85dadde9f62a7f2f5933174175612cc99958ee/content/browser/frame_host/render_frame_host_manager.cc
[modify] https://crrev.com/5b85dadde9f62a7f2f5933174175612cc99958ee/content/browser/frame_host/render_frame_host_manager_browsertest.cc

Apologies, applied the wrong component in bulk.