Remove SiteInstance::GetSiteForURL().

Removing SiteInstance::GetSiteForURL() and migrate existing usage to
alternatives like GetSiteURL() on the SiteInstance or SiteInfo based
checks. This method was only used for testing and no production code
paths are affected.

Bug: 1085275
Change-Id: I9be7cb73508de32851b981316c63e10c3cdf0b16
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2680274
Reviewed-by: Giovanni Ortuño Urquidi <[email protected]>
Reviewed-by: Nasko Oskov <[email protected]>
Commit-Queue: Aaron Colwell <[email protected]>
Cr-Commit-Position: refs/heads/master@{#854574}

chrome/browser/chrome_content_browser_client_browsertest.cc

@@ -149,11 +149,16 @@ IN_PROC_BROWSER_TEST_F(IsolatedOriginNTPBrowserTest,
   scoped_refptr<content::SiteInstance> site_instance =
       content::SiteInstance::CreateForURL(context, isolated_url);
   EXPECT_TRUE(site_instance->RequiresDedicatedProcess());
+  // Verify the isolated origin does not receive an NTP site URL scheme.
+  EXPECT_FALSE(
+      site_instance->GetSiteURL().SchemeIs(chrome::kChromeSearchScheme));
 
   // The site URL for the NTP URL should resolve to a chrome-search:// URL via
   // GetEffectiveURL(), even if the NTP URL matches an isolated origin.
-  GURL site_url(content::SiteInstance::GetSiteForURL(context, ntp_url));
-  EXPECT_TRUE(site_url.SchemeIs(chrome::kChromeSearchScheme));
+  scoped_refptr<content::SiteInstance> ntp_site_instance =
+      content::SiteInstance::CreateForURL(context, ntp_url);
+  EXPECT_TRUE(
+      ntp_site_instance->GetSiteURL().SchemeIs(chrome::kChromeSearchScheme));
 
   // Navigate to the NTP URL and verify that the resulting process is marked as
   // an Instant process.
@@ -164,12 +169,16 @@ IN_PROC_BROWSER_TEST_F(IsolatedOriginNTPBrowserTest,
       InstantServiceFactory::GetForProfile(browser()->profile());
   EXPECT_TRUE(instant_service->IsInstantProcess(
       contents->GetMainFrame()->GetProcess()->GetID()));
+  EXPECT_EQ(contents->GetMainFrame()->GetSiteInstance()->GetSiteURL(),
+            ntp_site_instance->GetSiteURL());
 
   // Navigating to a non-NTP URL on ntp.com should not result in an Instant
   // process.
   ui_test_utils::NavigateToURL(browser(), isolated_url);
   EXPECT_FALSE(instant_service->IsInstantProcess(
       contents->GetMainFrame()->GetProcess()->GetID()));
+  EXPECT_EQ(contents->GetMainFrame()->GetSiteInstance()->GetSiteURL(),
+            site_instance->GetSiteURL());
 }
 
 // Helper class to test window creation from NTP.

chrome/browser/ui/extensions/hosted_app_browsertest.cc

@@ -850,6 +850,12 @@ class HostedAppProcessModelTest : public HostedOrWebAppTest {
     return subframe;
   }
 
+  GURL GetSiteForURL(content::BrowserContext* browser_context,
+                     const GURL& url) {
+    return content::SiteInstance::CreateForURL(browser_context, url)
+        ->GetSiteURL();
+  }
+
  protected:
   bool should_swap_for_cross_site_;
 
@@ -897,22 +903,22 @@ IN_PROC_BROWSER_TEST_P(HostedAppProcessModelTest, IframesInsideHostedApp) {
   RenderFrameHost* cross_site = find_frame("CrossSite");
 
   // Sanity-check sites of all relevant frames to verify test setup.
-  GURL app_site = content::SiteInstance::GetSiteForURL(
-      app_browser_->profile(), app->GetLastCommittedURL());
+  GURL app_site =
+      GetSiteForURL(app_browser_->profile(), app->GetLastCommittedURL());
   EXPECT_EQ(extensions::kExtensionScheme, app_site.scheme());
 
-  GURL same_dir_site = content::SiteInstance::GetSiteForURL(
-      app_browser_->profile(), same_dir->GetLastCommittedURL());
+  GURL same_dir_site =
+      GetSiteForURL(app_browser_->profile(), same_dir->GetLastCommittedURL());
   EXPECT_EQ(extensions::kExtensionScheme, same_dir_site.scheme());
   EXPECT_EQ(same_dir_site, app_site);
 
-  GURL diff_dir_site = content::SiteInstance::GetSiteForURL(
-      app_browser_->profile(), diff_dir->GetLastCommittedURL());
+  GURL diff_dir_site =
+      GetSiteForURL(app_browser_->profile(), diff_dir->GetLastCommittedURL());
   EXPECT_NE(extensions::kExtensionScheme, diff_dir_site.scheme());
   EXPECT_NE(diff_dir_site, app_site);
 
-  GURL same_site_site = content::SiteInstance::GetSiteForURL(
-      app_browser_->profile(), same_site->GetLastCommittedURL());
+  GURL same_site_site =
+      GetSiteForURL(app_browser_->profile(), same_site->GetLastCommittedURL());
   EXPECT_NE(extensions::kExtensionScheme, same_site_site.scheme());
   EXPECT_NE(same_site_site, app_site);
   EXPECT_EQ(same_site_site, diff_dir_site);
@@ -928,15 +934,15 @@ IN_PROC_BROWSER_TEST_P(HostedAppProcessModelTest, IframesInsideHostedApp) {
   // content/public via SiteInfo.  For now, this verification will be done
   // implicitly by comparing SiteInstances and then actual processes further
   // below.
-  GURL isolated_site = content::SiteInstance::GetSiteForURL(
-      app_browser_->profile(), isolated->GetLastCommittedURL());
+  GURL isolated_site =
+      GetSiteForURL(app_browser_->profile(), isolated->GetLastCommittedURL());
   EXPECT_EQ(extensions::kExtensionScheme, isolated_site.scheme());
   EXPECT_EQ(isolated_site, app_site);
   EXPECT_NE(isolated->GetSiteInstance(), app->GetSiteInstance());
   EXPECT_NE(isolated_site, diff_dir_site);
 
-  GURL cross_site_site = content::SiteInstance::GetSiteForURL(
-      app_browser_->profile(), cross_site->GetLastCommittedURL());
+  GURL cross_site_site =
+      GetSiteForURL(app_browser_->profile(), cross_site->GetLastCommittedURL());
   EXPECT_NE(cross_site_site, app_site);
   EXPECT_NE(cross_site_site, same_site_site);
 
@@ -1341,8 +1347,8 @@ IN_PROC_BROWSER_TEST_P(HostedAppIsolatedOriginTest,
   RenderFrameHost* app = web_contents->GetMainFrame();
   EXPECT_EQ(extensions::kExtensionScheme,
             app->GetSiteInstance()->GetSiteURL().scheme());
-  GURL app_site = content::SiteInstance::GetSiteForURL(
-      app_browser_->profile(), app->GetLastCommittedURL());
+  GURL app_site =
+      GetSiteForURL(app_browser_->profile(), app->GetLastCommittedURL());
   EXPECT_EQ(extensions::kExtensionScheme, app_site.scheme());
   EXPECT_TRUE(process_map_->Contains(app->GetProcess()->GetID()));
 

content/browser/isolated_origin_browsertest.cc

@@ -3122,9 +3122,10 @@ IN_PROC_BROWSER_TEST_F(IsolatedOriginTest, SubframeErrorPages) {
       if (!SiteIsolationPolicy::IsErrorPageIsolationEnabled(
               /*in_main_frame=*/false)) {
         EXPECT_EQ(
-            SiteInstance::GetSiteForURL(web_contents()->GetBrowserContext(),
-                                        regular_url),
-            child2->current_frame_host()->GetSiteInstance()->GetSiteURL());
+            SiteInfo::CreateForTesting(
+                IsolationContext(web_contents()->GetBrowserContext()),
+                regular_url),
+            child2->current_frame_host()->GetSiteInstance()->GetSiteInfo());
       }
     } else {
       EXPECT_EQ(root->current_frame_host()->GetSiteInstance(),

content/browser/renderer_host/navigation_controller_impl_browsertest.cc

@@ -11531,9 +11531,11 @@ IN_PROC_BROWSER_TEST_P(NavigationControllerBrowserTest,
   if (AreAllSitesIsolatedForTesting()) {
     EXPECT_NE(initial_site_instance,
               root->current_frame_host()->GetSiteInstance());
-    EXPECT_EQ(SiteInstance::GetSiteForURL(
-                  shell()->web_contents()->GetBrowserContext(), url3),
-              root->current_frame_host()->GetSiteInstance()->GetSiteURL());
+    EXPECT_EQ(
+        SiteInfo::CreateForTesting(
+            IsolationContext(shell()->web_contents()->GetBrowserContext()),
+            url3),
+        root->current_frame_host()->GetSiteInstance()->GetSiteInfo());
     EXPECT_EQ(NAVIGATION_TYPE_NEW_ENTRY, capturer.navigation_type());
   } else {
     EXPECT_EQ(initial_site_instance,

content/browser/renderer_host/navigator_unittest.cc

@@ -75,13 +75,19 @@ class NavigatorTest : public RenderViewHostImplTestHarness {
         node->render_manager()->speculative_render_frame_host_.get());
   }
 
-  scoped_refptr<SiteInstance> ConvertToSiteInstance(
+  scoped_refptr<SiteInstanceImpl> ConvertToSiteInstance(
       RenderFrameHostManager* rfhm,
       const SiteInstanceDescriptor& descriptor,
       SiteInstance* candidate_instance) {
-    return rfhm->ConvertToSiteInstance(
-        descriptor, static_cast<SiteInstanceImpl*>(candidate_instance),
-        false /* is_speculative */);
+    return static_cast<SiteInstanceImpl*>(
+        rfhm->ConvertToSiteInstance(
+                descriptor, static_cast<SiteInstanceImpl*>(candidate_instance),
+                false /* is_speculative */)
+            .get());
+  }
+
+  SiteInfo CreateExpectedSiteInfo(const GURL& url) {
+    return SiteInfo::CreateForTesting(IsolationContext(browser_context()), url);
   }
 };
 
@@ -122,8 +128,8 @@ TEST_F(NavigatorTest, SimpleBrowserInitiatedNavigationFromNonLiveRenderer) {
   if (AreDefaultSiteInstancesEnabled()) {
     EXPECT_TRUE(main_test_rfh()->GetSiteInstance()->IsDefaultSiteInstance());
   } else {
-    EXPECT_EQ(SiteInstance::GetSiteForURL(browser_context(), kUrl),
-              main_test_rfh()->GetSiteInstance()->GetSiteURL());
+    EXPECT_EQ(CreateExpectedSiteInfo(kUrl),
+              main_test_rfh()->GetSiteInstance()->GetSiteInfo());
   }
   EXPECT_EQ(kUrl, contents()->GetLastCommittedURL());
 
@@ -190,8 +196,8 @@ TEST_F(NavigatorTest, SimpleRendererInitiatedSameSiteNavigation) {
   if (AreDefaultSiteInstancesEnabled()) {
     EXPECT_TRUE(main_test_rfh()->GetSiteInstance()->IsDefaultSiteInstance());
   } else {
-    EXPECT_EQ(SiteInstance::GetSiteForURL(browser_context(), kUrl2),
-              main_test_rfh()->GetSiteInstance()->GetSiteURL());
+    EXPECT_EQ(CreateExpectedSiteInfo(kUrl2),
+              main_test_rfh()->GetSiteInstance()->GetSiteInfo());
   }
   EXPECT_EQ(kUrl2, contents()->GetLastCommittedURL());
   EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
@@ -658,9 +664,9 @@ TEST_F(NavigatorTest, RedirectCrossSite) {
 TEST_F(NavigatorTest, BrowserInitiatedNavigationCancel) {
   const GURL kUrl0("http://www.wikipedia.org/");
   const GURL kUrl1("http://www.chromium.org/");
-  const GURL kUrl1_site = SiteInstance::GetSiteForURL(browser_context(), kUrl1);
+  const auto kUrl1SiteInfo = CreateExpectedSiteInfo(kUrl1);
   const GURL kUrl2("http://www.google.com/");
-  const GURL kUrl2_site = SiteInstance::GetSiteForURL(browser_context(), kUrl2);
+  const auto kUrl2SiteInfo = CreateExpectedSiteInfo(kUrl2);
 
   // Initialization.
   contents()->NavigateAndCommit(kUrl0);
@@ -686,7 +692,7 @@ TEST_F(NavigatorTest, BrowserInitiatedNavigationCancel) {
   if (AreDefaultSiteInstancesEnabled()) {
     EXPECT_TRUE(speculative_rfh->GetSiteInstance()->IsDefaultSiteInstance());
   } else {
-    EXPECT_EQ(kUrl1_site, speculative_rfh->GetSiteInstance()->GetSiteURL());
+    EXPECT_EQ(kUrl1SiteInfo, speculative_rfh->GetSiteInstance()->GetSiteInfo());
   }
 
   // Request navigation to the 2nd URL; the NavigationRequest must have been
@@ -726,7 +732,7 @@ TEST_F(NavigatorTest, BrowserInitiatedNavigationCancel) {
   if (AreDefaultSiteInstancesEnabled()) {
     EXPECT_TRUE(main_test_rfh()->GetSiteInstance()->IsDefaultSiteInstance());
   } else {
-    EXPECT_EQ(kUrl2_site, main_test_rfh()->GetSiteInstance()->GetSiteURL());
+    EXPECT_EQ(kUrl2SiteInfo, main_test_rfh()->GetSiteInstance()->GetSiteInfo());
   }
   EXPECT_EQ(kUrl2, contents()->GetLastCommittedURL());
 
@@ -1019,8 +1025,8 @@ TEST_F(NavigatorTest, SpeculativeRendererWorksBaseCase) {
   if (AreDefaultSiteInstancesEnabled()) {
     EXPECT_TRUE(speculative_rfh->GetSiteInstance()->IsDefaultSiteInstance());
   } else {
-    EXPECT_EQ(SiteInstance::GetSiteForURL(browser_context(), kUrl),
-              speculative_rfh->GetSiteInstance()->GetSiteURL());
+    EXPECT_EQ(CreateExpectedSiteInfo(kUrl),
+              speculative_rfh->GetSiteInstance()->GetSiteInfo());
   }
 
   navigation->ReadyToCommit();
@@ -1059,8 +1065,8 @@ TEST_F(NavigatorTest, SpeculativeRendererDiscardedAfterRedirectToAnotherSite) {
   if (AreDefaultSiteInstancesEnabled()) {
     EXPECT_TRUE(speculative_rfh->GetSiteInstance()->IsDefaultSiteInstance());
   } else {
-    EXPECT_EQ(SiteInstance::GetSiteForURL(browser_context(), kUrl),
-              speculative_rfh->GetSiteInstance()->GetSiteURL());
+    EXPECT_EQ(CreateExpectedSiteInfo(kUrl),
+              speculative_rfh->GetSiteInstance()->GetSiteInfo());
   }
 
   // It then redirects to yet another site.
@@ -1103,8 +1109,8 @@ TEST_F(NavigatorTest, SpeculativeRendererDiscardedAfterRedirectToAnotherSite) {
     // the SiteInstance stayed the same.
     EXPECT_FALSE(rfh_deleted_observer.deleted());
   } else {
-    EXPECT_EQ(SiteInstance::GetSiteForURL(browser_context(), kUrlRedirect),
-              speculative_rfh->GetSiteInstance()->GetSiteURL());
+    EXPECT_EQ(CreateExpectedSiteInfo(kUrlRedirect),
+              speculative_rfh->GetSiteInstance()->GetSiteInfo());
     EXPECT_NE(site_instance_id, redirect_site_instance_id);
 
     // Verify the old speculative RenderFrameHost was deleted because
@@ -1230,7 +1236,7 @@ TEST_F(NavigatorTest, SiteInstanceDescriptionConversion) {
   // 4) Convert a descriptor of a related instance with a site different from
   // the current one.
   GURL kUrlSameSiteAs2("http://www.b.com/foo");
-  scoped_refptr<SiteInstance> related_instance;
+  scoped_refptr<SiteInstanceImpl> related_instance;
   {
     SiteInstanceDescriptor descriptor(
         UrlInfo::CreateForTesting(kUrlSameSiteAs2),
@@ -1246,15 +1252,12 @@ TEST_F(NavigatorTest, SiteInstanceDescriptionConversion) {
     EXPECT_NE(current_instance, related_instance.get());
     EXPECT_NE(unrelated_instance.get(), related_instance.get());
 
-    auto* related_instance_impl =
-        static_cast<SiteInstanceImpl*>(related_instance.get());
-
     if (AreDefaultSiteInstancesEnabled()) {
-      ASSERT_TRUE(related_instance_impl->IsDefaultSiteInstance());
+      ASSERT_TRUE(related_instance->IsDefaultSiteInstance());
     } else {
       EXPECT_EQ(SiteInfo::CreateForTesting(
                     current_instance->GetIsolationContext(), kUrlSameSiteAs2),
-                related_instance_impl->GetSiteInfo());
+                related_instance->GetSiteInfo());
     }
   }
 
@@ -1265,20 +1268,20 @@ TEST_F(NavigatorTest, SiteInstanceDescriptionConversion) {
         UrlInfo::CreateForTesting(kUrlSameSiteAs1),
         SiteInstanceRelation::UNRELATED,
         CoopCoepCrossOriginIsolatedInfo::CreateNonIsolated());
-    scoped_refptr<SiteInstance> converted_instance_1 =
+    scoped_refptr<SiteInstanceImpl> converted_instance_1 =
         ConvertToSiteInstance(rfhm, descriptor, nullptr);
     // Should return a new instance, unrelated to the current one, set to the
     // provided site URL.
     EXPECT_FALSE(
         current_instance->IsRelatedSiteInstance(converted_instance_1.get()));
     EXPECT_NE(current_instance, converted_instance_1.get());
     EXPECT_NE(unrelated_instance.get(), converted_instance_1.get());
-    EXPECT_EQ(SiteInstance::GetSiteForURL(browser_context(), kUrlSameSiteAs1),
-              converted_instance_1->GetSiteURL());
+    EXPECT_EQ(CreateExpectedSiteInfo(kUrlSameSiteAs1),
+              converted_instance_1->GetSiteInfo());
 
     // Does the same but this time using unrelated_instance as a candidate,
     // which has a different site.
-    scoped_refptr<SiteInstance> converted_instance_2 =
+    scoped_refptr<SiteInstanceImpl> converted_instance_2 =
         ConvertToSiteInstance(rfhm, descriptor, unrelated_instance.get());
     // Should return yet another new instance, unrelated to the current one, set
     // to the same site URL.
@@ -1287,8 +1290,8 @@ TEST_F(NavigatorTest, SiteInstanceDescriptionConversion) {
     EXPECT_NE(current_instance, converted_instance_2.get());
     EXPECT_NE(unrelated_instance.get(), converted_instance_2.get());
     EXPECT_NE(converted_instance_1.get(), converted_instance_2.get());
-    EXPECT_EQ(SiteInstance::GetSiteForURL(browser_context(), kUrlSameSiteAs1),
-              converted_instance_2->GetSiteURL());
+    EXPECT_EQ(CreateExpectedSiteInfo(kUrlSameSiteAs1),
+              converted_instance_1->GetSiteInfo());
 
     // Converts once more but with |converted_instance_1| as a candidate.
     scoped_refptr<SiteInstance> converted_instance_3 =
@@ -1305,7 +1308,7 @@ TEST_F(NavigatorTest, SiteInstanceDescriptionConversion) {
         UrlInfo::CreateForTesting(kUrlSameSiteAs2),
         SiteInstanceRelation::UNRELATED,
         CoopCoepCrossOriginIsolatedInfo::CreateNonIsolated());
-    scoped_refptr<SiteInstance> converted_instance_1 =
+    scoped_refptr<SiteInstanceImpl> converted_instance_1 =
         ConvertToSiteInstance(rfhm, descriptor, related_instance.get());
     // Should return a new instance, unrelated to the current, set to the
     // provided site URL.
@@ -1315,11 +1318,10 @@ TEST_F(NavigatorTest, SiteInstanceDescriptionConversion) {
     EXPECT_NE(unrelated_instance.get(), converted_instance_1.get());
 
     if (AreDefaultSiteInstancesEnabled()) {
-      EXPECT_TRUE(static_cast<SiteInstanceImpl*>(converted_instance_1.get())
-                      ->IsDefaultSiteInstance());
+      EXPECT_TRUE(converted_instance_1->IsDefaultSiteInstance());
     } else {
-      EXPECT_EQ(SiteInstance::GetSiteForURL(browser_context(), kUrlSameSiteAs2),
-                converted_instance_1->GetSiteURL());
+      EXPECT_EQ(CreateExpectedSiteInfo(kUrlSameSiteAs2),
+                converted_instance_1->GetSiteInfo());
     }
 
     scoped_refptr<SiteInstance> converted_instance_2 =

content/browser/renderer_host/render_frame_host_manager_browsertest.cc

@@ -2014,10 +2014,12 @@ IN_PROC_BROWSER_TEST_P(
   const GURL kCrossSiteURL =
       embedded_test_server()->GetURL("c.com", "/title1.html");
 
-  const GURL kOriginalSiteURL = SiteInstance::GetSiteForURL(
-      shell()->web_contents()->GetBrowserContext(), kOriginalURL);
-  const GURL kRedirectSiteURL = SiteInstance::GetSiteForURL(
-      shell()->web_contents()->GetBrowserContext(), kRedirectURL);
+  IsolationContext isolation_context(
+      shell()->web_contents()->GetBrowserContext());
+  const auto kOriginalSiteInfo =
+      SiteInfo::CreateForTesting(isolation_context, kOriginalURL);
+  const auto kRedirectSiteInfo =
+      SiteInfo::CreateForTesting(isolation_context, kRedirectURL);
 
   // First navigate to the initial URL.
   shell()->LoadURL(kOriginalURL);
@@ -2072,13 +2074,13 @@ IN_PROC_BROWSER_TEST_P(
   CHECK(speculative_rfh);
   EXPECT_TRUE(speculative_rfh->is_loading());
   if (AreAllSitesIsolatedForTesting()) {
-    EXPECT_EQ(kRedirectSiteURL,
-              speculative_rfh->GetSiteInstance()->GetSiteURL());
+    EXPECT_EQ(kRedirectSiteInfo,
+              speculative_rfh->GetSiteInstance()->GetSiteInfo());
   } else if (AreDefaultSiteInstancesEnabled()) {
     EXPECT_TRUE(speculative_rfh->GetSiteInstance()->IsDefaultSiteInstance());
   } else {
-    EXPECT_EQ(kOriginalSiteURL,
-              speculative_rfh->GetSiteInstance()->GetSiteURL());
+    EXPECT_EQ(kOriginalSiteInfo,
+              speculative_rfh->GetSiteInstance()->GetSiteInfo());
   }
   int site_instance_id = speculative_rfh->GetSiteInstance()->GetId();
 
@@ -2098,8 +2100,8 @@ IN_PROC_BROWSER_TEST_P(
   if (AreDefaultSiteInstancesEnabled()) {
     EXPECT_TRUE(speculative_rfh->GetSiteInstance()->IsDefaultSiteInstance());
   } else {
-    EXPECT_EQ(kRedirectSiteURL,
-              speculative_rfh->GetSiteInstance()->GetSiteURL());
+    EXPECT_EQ(kRedirectSiteInfo,
+              speculative_rfh->GetSiteInstance()->GetSiteInfo());
     if (AreAllSitesIsolatedForTesting())
       EXPECT_EQ(site_instance_id, speculative_rfh->GetSiteInstance()->GetId());
   }
@@ -2120,8 +2122,8 @@ IN_PROC_BROWSER_TEST_P(
   if (AreDefaultSiteInstancesEnabled()) {
     EXPECT_TRUE(speculative_rfh->GetSiteInstance()->IsDefaultSiteInstance());
   } else {
-    EXPECT_EQ(kOriginalSiteURL,
-              speculative_rfh->GetSiteInstance()->GetSiteURL());
+    EXPECT_EQ(kOriginalSiteInfo,
+              speculative_rfh->GetSiteInstance()->GetSiteInfo());
   }
   if (AreAllSitesIsolatedForTesting())
     EXPECT_NE(site_instance_id, speculative_rfh->GetSiteInstance()->GetId());