From f1f5e7417e9b4f39d7f007b4d98d8551efd23a8a Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Thu, 6 Sep 2018 13:10:10 +0200 Subject: [Backport] Avoid unneeded call to Origin::GetURL from SiteInstance::GetSiteForURL. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: 820070 Change-Id: I3fd1cd2fb5c53568a53a59046b8180a01d8b8877 Reviewed-on: https://chromium-review.googlesource.com/956308 Reviewed-by: Michael BrĂ¼ning --- chromium/content/browser/site_instance_impl.cc | 8 ++-- .../content/browser/site_instance_impl_unittest.cc | 11 +++++ .../browser/site_per_process_browsertest.cc | 50 ++++++++++++++++++++++ 3 files changed, 66 insertions(+), 3 deletions(-) diff --git a/chromium/content/browser/site_instance_impl.cc b/chromium/content/browser/site_instance_impl.cc index 357894a1d48..d5f3fcc1424 100644 --- a/chromium/content/browser/site_instance_impl.cc +++ b/chromium/content/browser/site_instance_impl.cc @@ -428,10 +428,12 @@ GURL SiteInstance::GetSiteForURL(BrowserContext* browser_context, if (!origin.unique()) { // Prefer to use the scheme of |origin| rather than |url|, to correctly // cover blob: and filesystem: URIs (see also https://crbug.com/697111). - DCHECK(origin.GetURL().has_scheme()) << origin; - return GURL(origin.GetURL().scheme() + ":"); - } else if (url.has_scheme()) + DCHECK(!origin.scheme().empty()); + return GURL(origin.scheme() + ":"); + } else if (url.has_scheme()) { + DCHECK(!url.scheme().empty()); return GURL(url.scheme() + ":"); + } // Otherwise the URL should be invalid; return an empty site. DCHECK(!url.is_valid()) << url; diff --git a/chromium/content/browser/site_instance_impl_unittest.cc b/chromium/content/browser/site_instance_impl_unittest.cc index 4297915e09f..a31de99b666 100644 --- a/chromium/content/browser/site_instance_impl_unittest.cc +++ b/chromium/content/browser/site_instance_impl_unittest.cc @@ -368,6 +368,17 @@ TEST_F(SiteInstanceTest, GetSiteForURL) { EXPECT_EQ("file", site_url.scheme()); EXPECT_FALSE(site_url.has_host()); + // Blob URLs created from a unique origin use the full URL as the site URL, + // except for the hash. + test_url = GURL("blob:null/1029e5a4-2983-4b90-a585-ed217563acfeb"); + site_url = SiteInstanceImpl::GetSiteForURL(nullptr, test_url); + EXPECT_EQ(site_url, test_url); + test_url = GURL("blob:null/1029e5a4-2983-4b90-a585-ed217563acfeb#foo"); + site_url = SiteInstanceImpl::GetSiteForURL(nullptr, test_url); + EXPECT_NE(site_url, test_url); + EXPECT_FALSE(site_url.has_ref()); + EXPECT_TRUE(site_url.EqualsIgnoringRef(test_url)); + // Private domains are preserved, appspot being such a site. test_url = GURL( "blob:http://www.example.appspot.com:44/" diff --git a/chromium/content/browser/site_per_process_browsertest.cc b/chromium/content/browser/site_per_process_browsertest.cc index 645960a3262..92bff576b82 100644 --- a/chromium/content/browser/site_per_process_browsertest.cc +++ b/chromium/content/browser/site_per_process_browsertest.cc @@ -13750,4 +13750,54 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, } } +// Ensure that when two cross-site frames have subframes with unique origins, +// and those subframes create blob URLs and navigate to them, the blob URLs end +// up in different processes. See https://crbug.com/863623. +IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, + TwoBlobURLsWithNullOriginDontShareProcess) { + GURL main_url(embedded_test_server()->GetURL( + "a.com", "/navigation_controller/page_with_data_iframe.html")); + EXPECT_TRUE(NavigateToURL(shell(), main_url)); + FrameTreeNode* root = web_contents()->GetFrameTree()->root(); + FrameTreeNode* subframe = root->child_at(0); + + // Create a blob URL in the subframe, and navigate to it. + TestNavigationObserver observer(shell()->web_contents()); + std::string blob_script = + "var blob = new Blob(['foo'], {type : 'text/html'});" + "var url = URL.createObjectURL(blob);" + "location = url;"; + EXPECT_TRUE(ExecuteScript(subframe, blob_script)); + observer.Wait(); + RenderFrameHostImpl* subframe_rfh = subframe->current_frame_host(); + EXPECT_TRUE(subframe_rfh->GetLastCommittedURL().SchemeIsBlob()); + + // Open a cross-site popup and repeat these steps. + GURL popup_url(embedded_test_server()->GetURL( + "b.com", "/navigation_controller/page_with_data_iframe.html")); + Shell* new_shell = OpenPopup(root, popup_url, ""); + FrameTreeNode* popup_root = + static_cast(new_shell->web_contents()) + ->GetFrameTree() + ->root(); + FrameTreeNode* popup_subframe = popup_root->child_at(0); + + TestNavigationObserver popup_observer(new_shell->web_contents()); + EXPECT_TRUE(ExecuteScript(popup_subframe, blob_script)); + popup_observer.Wait(); + RenderFrameHostImpl* popup_subframe_rfh = + popup_subframe->current_frame_host(); + EXPECT_TRUE(popup_subframe_rfh->GetLastCommittedURL().SchemeIsBlob()); + + // Ensure that the two blob subframes don't share a process or SiteInstance. + EXPECT_NE(subframe->current_frame_host()->GetSiteInstance(), + popup_subframe->current_frame_host()->GetSiteInstance()); + EXPECT_NE( + subframe->current_frame_host()->GetSiteInstance()->GetProcess(), + popup_subframe->current_frame_host()->GetSiteInstance()->GetProcess()); + EXPECT_NE( + subframe->current_frame_host()->GetSiteInstance()->GetSiteURL(), + popup_subframe->current_frame_host()->GetSiteInstance()->GetSiteURL()); +} + } // namespace content -- cgit v1.2.3