Remove GMail fork heuristics |
||||||
Issue descriptionLet's use this bug to track the attempt to remove the GMail fork heuristics: @@ -6069,42 +6069,6 @@ WebNavigationPolicy RenderFrameImpl::DecidePolicyForNavigation( // See http://crbug.com/93517 . GURL old_url(frame_->GetDocumentLoader()->GetRequest().Url()); - // Detect when a page is "forking" a new tab that can be safely rendered in - // its own process. This is done by sites like Gmail that try to open links - // in new windows without script connections back to the original page. We - // treat such cases as browser navigations (in which we will create a new - // renderer for a cross-site navigation), rather than WebKit navigations. - // - // We use the following heuristic to decide whether to fork a new page in its - // own process: - // The parent page must open a new tab to about:blank, set the new tab's - // window.opener to null, and then redirect the tab to a cross-site URL using - // JavaScript. - // - // TODO(creis): Deprecate this logic once we can rely on rel=noreferrer - // (see below). - bool is_fork = - // Must start from a tab showing about:blank, which is later redirected. - old_url == url::kAboutBlankURL && - // Must be the first real navigation of the tab. - render_view_->HistoryBackListCount() < 1 && - render_view_->HistoryForwardListCount() < 1 && - // The parent page must have set the child's window.opener to null before - // redirecting to the desired URL. - frame_->Opener() == nullptr && - // Must be a top-level frame. - frame_->Parent() == nullptr && - // Must be targeted at the current tab. - info.default_policy == blink::kWebNavigationPolicyCurrentTab && - // Must be a JavaScript navigation, which appears as "other". - info.navigation_type == blink::kWebNavigationTypeOther; - - if (is_fork) { - // Open the URL via the browser, not via WebKit. - OpenURL(info, /*is_history_navigation_in_new_child=*/false); - return blink::kWebNavigationPolicyIgnore; - } -
,
Oct 4
+alexmos@ who helped investigate WebViewNewWindowInteractiveTest.NewWindow_Redirect problems in https://chromium-review.googlesource.com/c/chromium/src/+/1188643#message-f4ae22af5776a24585b51fb353c18c3758a9daa0
,
Oct 4
+jochen@ and creis@ in case they want to comment on the desirability and/or potential gotchas related to removing of the "gmail fork heuristics".
,
Oct 4
,
Oct 5
Let me clarify some of the rationale for removing this. We added this heuristic in the early days of Chrome to make it possible for pages to open cross-site links in a new process if they hint to Chrome that they don't need to script them. Gmail happened to be one of the primary use cases. Any page could use it with a pattern like this: w = window.open(); w.opener = null; w.location.href = cross_site_url; Later, we added a more official / documented way to do it using rel=noreferrer (and later rel=noopener), which works for same-site links as well: https://blog.chromium.org/2009/12/links-that-open-in-new-processes.html We intended to remove the old approach but some sites (Gmail included) continued to use it. However, now that Site Isolation is enabled by default, all cross-site URLs are opened in a new process anyway, so this heuristic is now basically a no-op on desktop. (The heuristic was also used for referrer stripping for a while, but Jochen and I removed that in favor of Referrer Policy in r581502.) This means the heuristic only currently has an effect when Site Isolation is disabled (e.g., on Android). Note that Gmail was the primary user of it, and Gmail for the mobile web doesn't use the heuristic-- I've confirmed that cross-site links open in the same process there. Any other sites that want to open links in a new process without Site Isolation should really be using rel=noopener or rel=noreferrer (with target=_blank). At least from my perspective, I think we should be ok to remove it. I'll add some comments to the CL as well. (Also, lukasza@ also notes that Gmail doesn't seem to be relying on this heuristic on desktop anymore either (even when Site Isolation is disabled)-- I'm curious how it currently works.)
,
Oct 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0962f52f2b0d1627ee5b9446ec9ce79e275c428d commit 0962f52f2b0d1627ee5b9446ec9ce79e275c428d Author: Lukasz Anforowicz <lukasza@chromium.org> Date: Wed Oct 17 17:56:54 2018 Remove non-standard process fork heuristic used by desktop Gmail. Setting a new window's opener to null and navigating it cross-site used to be a hint to Chrome that it was safe to open the link in a new process. Now that Site Isolation is enabled by default on desktop, cross-site links will always open in a new process, making this unnecessary there. For cases where Site Isolation is not enabled, it is better to use target=_blank rel=noreferrer (or rel=noopener) than this old, non-standard heuristic. See: https://blog.chromium.org/2009/12/links-that-open-in-new-processes.html Bug: 892212, 892375 , 884383 Change-Id: I50422cb6bd97f8e0f1d4ef082551ecb355ae104b Reviewed-on: https://chromium-review.googlesource.com/c/1262089 Reviewed-by: James MacLean <wjmaclean@chromium.org> Reviewed-by: Charlie Reis <creis@chromium.org> Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org> Cr-Commit-Position: refs/heads/master@{#600469} [modify] https://crrev.com/0962f52f2b0d1627ee5b9446ec9ce79e275c428d/content/renderer/render_frame_impl.cc [delete] https://crrev.com/382ffc533cb64b0946adfbc617858a6203207d03/content/test/data/fork-popup.html [modify] https://crrev.com/0962f52f2b0d1627ee5b9446ec9ce79e275c428d/extensions/browser/guest_view/web_view/web_view_guest.cc [modify] https://crrev.com/0962f52f2b0d1627ee5b9446ec9ce79e275c428d/extensions/browser/guest_view/web_view/web_view_guest.h [modify] https://crrev.com/0962f52f2b0d1627ee5b9446ec9ce79e275c428d/third_party/WebKit/LayoutTests/TestExpectations
,
Oct 18
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by lukasza@chromium.org
, Oct 4Status: Started (was: Untriaged)