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

Issue 726370 link

Starred by 1 user
Status: Fixed
Owner:
alex...@chromium.org
Closed: Jun 2017
Cc:
█ nick@chromium.org
creis@chromium.org
wfh@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Scheme check in SiteInstance::IsSameWebSite may not be correct for blob/filesystem URLs

Project Member Reported by alex...@chromium.org, May 25 2017 
While working on https://codereview.chromium.org/2831683002 we noticed that SiteInstance::IsSameWebSite has a scheme check when comparing sites:

   if (src_url.scheme() != dest_url.scheme())
     return false;

This won't work for blob or filesystem URLs.  For example, if src_url is http://foo.com and dest_url is blob:http://foo.com/{id}, I think the two URLs should be considered to have the same site.

We need to investigate exactly what might be broken because of this and fix it.  E.g., this might be causing unnecessary process transfers.

Comment 1 by alex...@chromium.org, May 26 2017

Status: Started (was: Available)
So I think this is relatively benign and doesn't actually cause unnecessary transfers in cases like navigating from http://foo.com to blob:http://foo.com/{id}.  This is because even though IsSameWebSite returns false, causing IsRendererTransferNeededForNavigation() to return true and to proceed down the transfer path, later on as part of GetSiteInstanceForNavigation, GetSiteForURL(blob_url) returns the right thing (http://foo.com), which causes BrowsingInstance::GetSiteInstanceForURL to find and return the existing (current) SiteInstance.  Next, UpdateStateForNavigate realizes that the current and destination SiteInstances are the same, and leaves the navigation in the current RFH and doesn't call Transfer().

We should still fix this though, as there's no need to even start going down the transfer path for a case like this.   IsRendererTransferNeededForNavigation() should be returning false.

Comment 2 by nick@chromium.org, May 26 2017 

I think the behavior alex describes would pair nicely with a "block local-scheme URL navigations in NavigationThrottle if different-origin from the last committed origin of the initiating RenderFrameHost". Currently we can't implement that, because of the lack of initiator info -- but I'm told it's coming soon.
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 1 2017 

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

commit cbf99578af94e8d3e95ac8da4ee10d0689e20993
Author: alexmos <alexmos@chromium.org>
Date: Thu Jun 01 03:13:13 2017

Fix scheme check in SiteInstance::IsSameWebSite for nested URLs.

BUG= 726370 

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

[modify] https://crrev.com/cbf99578af94e8d3e95ac8da4ee10d0689e20993/content/browser/site_instance_impl.cc
[modify] https://crrev.com/cbf99578af94e8d3e95ac8da4ee10d0689e20993/content/browser/site_instance_impl_unittest.cc

Comment 4 by alex...@chromium.org, Jun 1 2017

Status: Fixed (was: Started)
►

Sign in to add a comment