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

Issue 725265 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocked on:
issue 690229



Sign in to add a comment

Remove AreCrossProcessFramesPossible

Project Member Reported by alex...@chromium.org, May 22 2017

Issue description

Now that we've shipped --isolate-extensions, AreCrossProcessFramesPossible returns true on all platforms except for Android, where it's still gated on one of our other OOPIF modes being enabled.  We should try to enable it on Android and then rip it out altogether, as it's complicating feature work for sign-in isolation and safe browsing (see https://codereview.chromium.org/2831683002/diff/80001/content/common/site_isolation_policy.cc#newcode22), and possibly leading to bugs on Android if we inadvertently start relying on the AreCrossProcessFramesPossible paths (e.g., DidNavigateFrame being called for all subframes from DidNavigate --
 see https://codereview.chromium.org/2883213002/#msg11).

The main uses of AreCrossProcessFramesPossible are currently:
1. Enable the event router in the browser process.
2. Skip certain proxy creation and replication work when OOPIFs are off.
3. Skip calling RFHM::DidNavigateFrame for subframes if OOPIFs are off.
4. Some sanity CHECKs to verify that certain things can only happen when OOPIFs are possible.
5. Decide whether to checks for transfers with IsRendererTransferNeededForNavigation in NavigationHandleImpl::MaybeTransferAndProceedInternal.
6. Some other shortcuts in process model decisions, such as in ShouldTransitionCrossSite and CanSubframeSwapProcess.

One blocker we're currently aware of is the perf regression in  issue 690229 .  Android hit testing used to be another blocker, but kenrb@ has since fixed that in r455559. We should investigate/fix  issue 690229  and then clean up the checks above to just assume that OOPIFs are always possible.
 
Project Member

Comment 1 by bugdroid1@chromium.org, May 23 2017

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

commit d340a11e259f0837320cdfd4e95a2c6b9efed721
Author: alexmos <alexmos@chromium.org>
Date: Tue May 23 01:06:25 2017

Remove AreCrossProcessFramesPossible from NavigatorImpl::DidNavigate.

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

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

[modify] https://crrev.com/d340a11e259f0837320cdfd4e95a2c6b9efed721/content/browser/frame_host/navigator_impl.cc

Cc: -kenrb@chromium.org
Owner: kenrb@chromium.org
I think Ken was planning to work on this.

Comment 3 by kenrb@chromium.org, Aug 22 2017

Status: Fixed (was: Available)
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 23 2017

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

commit 077f5031e693e9c495203dc5c3360a7f0d3c0c39
Author: Ken Buchanan <kenrb@chromium.org>
Date: Wed Aug 23 15:34:47 2017

Remove AreCrossProcessFramesPossible

SiteIsolationPolicy::AreCrossProcessFramesPossible no longer performs
any purpose, since it always returns true. This patch removes the
function, all call sites, and any code that is no longer reachable as a
consequence.

Bug:  725265 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: Ifed5ab20831947c4d219d0a17af2ccc984641108
Reviewed-on: https://chromium-review.googlesource.com/615147
Reviewed-by: Jay Civelli <jcivelli@chromium.org>
Reviewed-by: Stephen Lanham <slan@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Nick Carter <nick@chromium.org>
Commit-Queue: Ken Buchanan <kenrb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496691}
[modify] https://crrev.com/077f5031e693e9c495203dc5c3360a7f0d3c0c39/android_webview/common/crash_reporter/crash_keys.cc
[modify] https://crrev.com/077f5031e693e9c495203dc5c3360a7f0d3c0c39/chrome/app/chrome_crash_reporter_client_win.cc
[modify] https://crrev.com/077f5031e693e9c495203dc5c3360a7f0d3c0c39/chrome/common/crash_keys.cc
[modify] https://crrev.com/077f5031e693e9c495203dc5c3360a7f0d3c0c39/chromecast/crash/cast_crash_keys.cc
[modify] https://crrev.com/077f5031e693e9c495203dc5c3360a7f0d3c0c39/content/browser/frame_host/frame_tree.cc
[modify] https://crrev.com/077f5031e693e9c495203dc5c3360a7f0d3c0c39/content/browser/frame_host/frame_tree_node.cc
[modify] https://crrev.com/077f5031e693e9c495203dc5c3360a7f0d3c0c39/content/browser/frame_host/navigation_controller_impl_browsertest.cc
[modify] https://crrev.com/077f5031e693e9c495203dc5c3360a7f0d3c0c39/content/browser/frame_host/navigation_handle_impl.cc
[modify] https://crrev.com/077f5031e693e9c495203dc5c3360a7f0d3c0c39/content/browser/frame_host/navigator_impl.cc
[modify] https://crrev.com/077f5031e693e9c495203dc5c3360a7f0d3c0c39/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/077f5031e693e9c495203dc5c3360a7f0d3c0c39/content/browser/frame_host/render_frame_host_manager.cc
[modify] https://crrev.com/077f5031e693e9c495203dc5c3360a7f0d3c0c39/content/browser/frame_host/render_widget_host_view_guest.cc
[modify] https://crrev.com/077f5031e693e9c495203dc5c3360a7f0d3c0c39/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/077f5031e693e9c495203dc5c3360a7f0d3c0c39/content/browser/renderer_host/render_widget_host_view_event_handler.cc
[modify] https://crrev.com/077f5031e693e9c495203dc5c3360a7f0d3c0c39/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/077f5031e693e9c495203dc5c3360a7f0d3c0c39/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/077f5031e693e9c495203dc5c3360a7f0d3c0c39/content/common/site_isolation_policy.cc
[modify] https://crrev.com/077f5031e693e9c495203dc5c3360a7f0d3c0c39/content/common/site_isolation_policy.h
[modify] https://crrev.com/077f5031e693e9c495203dc5c3360a7f0d3c0c39/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/077f5031e693e9c495203dc5c3360a7f0d3c0c39/content/renderer/render_thread_impl.cc
[modify] https://crrev.com/077f5031e693e9c495203dc5c3360a7f0d3c0c39/content/renderer/render_view_browsertest.cc

Sign in to add a comment