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

Issue 602818 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
not working at Google anymore
Closed: May 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 477150



Sign in to add a comment

blob URLs end up in the wrong SiteInstance

Project Member Reported by creis@chromium.org, Apr 12 2016

Issue description

Version: 52.0.2706.0 or earlier
OS: All

What steps will reproduce the problem?
(1) Visit http://csreis.github.io/tests/blob.html
(2) Click "Open blob popup with null opener" button.

What is the expected output?

The two windows are same origin, so they should be in the same process.  Instead, they end up in different processes.

This happens because the SiteInstance's Site URL ends up as "blob:" instead of the inner origin, which is http://csreis.github.io.

Nick suspects this will affect filesystem URLs as well.
 

Comment 1 by creis@chromium.org, Apr 12 2016

Note: I would have expected these to be another set of repro steps:
1) Start Chrome with --site-per-process.
2) Visit http://csreis.github.io/tests/blob.html
3) Click "Add iframe with blob URL" button.

The iframe should be in process, but I would expect it to become an out-of-process iframe.  It's unexpectedly staying in process...?

Comment 2 by creis@chromium.org, Apr 12 2016

Sounds like Nick's https://codereview.chromium.org/1878273002/ will be one part of the fix.

Comment 3 by nick@chromium.org, Apr 18 2016

Status: Started (was: Assigned)
With the fix mentioned in #2 now landed, we are now seeing this bug in a layouttest: http/tests/fileapi/blob-url-in-subframe.html

I'll fix it.
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 18 2016

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

commit 31d0ea2933c9d8fbeb1c30ff0c34473c7e4784db
Author: nick <nick@chromium.org>
Date: Mon Apr 18 22:24:31 2016

Update test expectations for blob and bindings check bugs.

BUG= 602818 , 584984 

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

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

[modify] https://crrev.com/31d0ea2933c9d8fbeb1c30ff0c34473c7e4784db/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process

Comment 5 by nick@chromium.org, Apr 20 2016

The remaining part of this fix turns out to be easy: we just need to teach SiteInstance::GetSiteForURL about blobs. We'll want to do the same for filesystem URLs, and we'll get it for free if we use url::Origin.
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 29 2016

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

commit 1dd47922979b128ce7ee8debf138b83c65661024
Author: nick <nick@chromium.org>
Date: Fri Apr 29 16:44:48 2016

Teach SiteInstance::GetSiteForURL() about blob and filesystem URLs.

Use url::Origin to do the heavy lifting.

This fixes FrameTreeBrowsertest.NavigateGrandchildToBlob under --site-per-process, and fixes process transfers for blob URLs in general.

url::Origin extraction exposed a bug where tests encountered chrome-search:// URLs with ports. This is fixed by clearing the port.

BUG= 602818 , 564316 ,490074, 605720 

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

[modify] https://crrev.com/1dd47922979b128ce7ee8debf138b83c65661024/chrome/browser/search/search.cc
[modify] https://crrev.com/1dd47922979b128ce7ee8debf138b83c65661024/chrome/browser/search/search_unittest.cc
[modify] https://crrev.com/1dd47922979b128ce7ee8debf138b83c65661024/content/browser/site_instance_impl.cc
[modify] https://crrev.com/1dd47922979b128ce7ee8debf138b83c65661024/content/browser/site_instance_impl_unittest.cc
[modify] https://crrev.com/1dd47922979b128ce7ee8debf138b83c65661024/testing/buildbot/filters/site-per-process.content_browsertests.filter

Blocking: 477150
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 29 2016

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

commit c95b1fe8a47162538c389ec2361332473437beda
Author: lukasza <lukasza@chromium.org>
Date: Fri Apr 29 20:14:08 2016

Remove exceptions for tests fixed by teaching GetSiteForURL() about blob URLs.

These tests got recently fixed by https://crrev.com/1911573002

BUG= 602818 

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

[modify] https://crrev.com/c95b1fe8a47162538c389ec2361332473437beda/third_party/WebKit/LayoutTests/FlagExpectations/site-per-process

Comment 9 by nick@chromium.org, May 2 2016

Status: Fixed (was: Started)

Sign in to add a comment