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

Issue 847127 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 1
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Remove SiteInstanceTest.ProcessSharingByType

Project Member Reported by calamity@chromium.org, May 28 2018

Issue description

From https://chromium-review.googlesource.com/c/chromium/src/+/1025673

"In general, WebUIs should not be sharing processes, especially going forward. Given that Site Isolation is the default on trunk, this test does not add much meaningful value and I'd rather we not add it. Feel free to file a bug on me to remove the other test too : )."
 
Cc: nasko@chromium.org
Owner: lukasza@chromium.org
Status: Started (was: Assigned)
The old test also fails after making WebUI pages use origin locks (see https://crrev.com/c/1237392) so I'd also like to either remove the test.

Direct link to the CR comment quoted above: https://chromium-review.googlesource.com/c/chromium/src/+/1025673/23/content/browser/site_instance_impl_unittest.cc#690
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 28

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

commit 1f5ad409dbf5334523931df37598ea49e9849c87
Author: Lukasz Anforowicz <lukasza@chromium.org>
Date: Fri Sep 28 23:57:54 2018

Allow origin lock for WebUI pages.

Returning true for WebUI pages in DoesSiteRequireDedicatedProcess helps
to keep enforcing a SiteInstance swap during chrome://foo ->
chrome://bar navigation, even after relaxing
BrowsingInstance::GetSiteInstanceForURL to consider RPH::IsSuitableHost
(see https://crrev.com/c/783470 for that fixes process sharing in
isolated(b(c),d(c)) scenario).

I've manually tested this CL by visiting the following URLs:
- chrome://welcome/
- chrome://settings
- chrome://extensions
- chrome://history
- chrome://help and chrome://chrome (both redirect to chrome://settings/help)

Bug:  510588 ,  847127 
Change-Id: I55073bce00f32cb8bc5c1c91034438ff9a3f8971
Reviewed-on: https://chromium-review.googlesource.com/1237392
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595259}
[modify] https://crrev.com/1f5ad409dbf5334523931df37598ea49e9849c87/chrome/browser/renderer_host/render_process_host_chrome_browsertest.cc
[modify] https://crrev.com/1f5ad409dbf5334523931df37598ea49e9849c87/chrome/browser/resource_coordinator/tab_manager_browsertest.cc
[modify] https://crrev.com/1f5ad409dbf5334523931df37598ea49e9849c87/content/browser/frame_host/render_frame_host_manager_browsertest.cc
[modify] https://crrev.com/1f5ad409dbf5334523931df37598ea49e9849c87/content/browser/frame_host/webui_navigation_browsertest.cc
[modify] https://crrev.com/1f5ad409dbf5334523931df37598ea49e9849c87/content/browser/site_instance_impl.cc
[modify] https://crrev.com/1f5ad409dbf5334523931df37598ea49e9849c87/content/browser/site_instance_impl.h
[modify] https://crrev.com/1f5ad409dbf5334523931df37598ea49e9849c87/content/browser/site_instance_impl_unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment