[Extensions] Make ProcessManager::GetSiteInstanceForUrl return ref ptr

ProcessManager::GetSiteInstanceForUrl calls through into
SiteInstance::GetRelatedSiteInstance(), which has the appropriate comment
"Callers should ensure that this SiteInstance becomes ref counted, by storing
it in a scoped_refptr." This implies that GetSiteInstanceForUrl should return
a refptr (as, probably, should GetRelatedSiteInstance, but that's a bigger
change).

(Discovered by a test failing on asan for leaking a site instance.)

BUG=456858
TBR=mtomasz@chromium.org (micro file manager change)

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

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

7 files changed, 20 insertions, 19 deletions

diff --git a/chrome/browser/chromeos/file_manager/file_browser_handlers.cc b/chrome/browser/chromeos/file_manager/file_browser_handlers.cc
index 0b52694..da0aebd 100644
--- a/chrome/browser/chromeos/file_manager/file_browser_handlers.cc
+++ b/chrome/browser/chromeos/file_manager/file_browser_handlers.cc
@@ -67,7 +67,8 @@ int ExtractProcessFromExtensionId(Profile* profile,
   extensions::ProcessManager* manager =
       extensions::ProcessManager::Get(profile);
 
-  SiteInstance* site_instance = manager->GetSiteInstanceForURL(extension_url);
+  scoped_refptr<SiteInstance> site_instance =
+      manager->GetSiteInstanceForURL(extension_url);
   if (!site_instance || !site_instance->HasProcess())
     return -1;
   content::RenderProcessHost* process = site_instance->GetProcess();
diff --git a/chrome/browser/extensions/api/messaging/message_service.cc b/chrome/browser/extensions/api/messaging/message_service.cc
index 15eed45..a300b98b 100644
--- a/chrome/browser/extensions/api/messaging/message_service.cc
+++ b/chrome/browser/extensions/api/messaging/message_service.cc
@@ -176,7 +176,7 @@ static base::StaticAtomicSequenceNumber g_channel_id_overflow_count;
 static content::RenderProcessHost* GetExtensionProcess(
     BrowserContext* context,
     const std::string& extension_id) {
-  SiteInstance* site_instance =
+  scoped_refptr<SiteInstance> site_instance =
       ProcessManager::Get(context)->GetSiteInstanceForURL(
           Extension::GetBaseURLFromExtensionId(extension_id));
   return site_instance->HasProcess() ? site_instance->GetProcess() : NULL;
diff --git a/chrome/browser/extensions/extension_util.cc b/chrome/browser/extensions/extension_util.cc
index c4d4ce5..8cf30d4 100644
--- a/chrome/browser/extensions/extension_util.cc
+++ b/chrome/browser/extensions/extension_util.cc
@@ -318,7 +318,7 @@ bool IsExtensionIdle(const std::string& extension_id,
     if (host)
       return false;
 
-    content::SiteInstance* site_instance =
+    scoped_refptr<content::SiteInstance> site_instance =
         process_manager->GetSiteInstanceForURL(
             Extension::GetBaseURLFromExtensionId(id));
     if (site_instance && site_instance->HasProcess())
diff --git a/chrome/browser/extensions/extension_view_host_factory.cc b/chrome/browser/extensions/extension_view_host_factory.cc
index b8565c2..6a14fdc 100644
--- a/chrome/browser/extensions/extension_view_host_factory.cc
+++ b/chrome/browser/extensions/extension_view_host_factory.cc
@@ -33,13 +33,13 @@ ExtensionViewHost* CreateViewHostForExtension(const Extension* extension,
   DCHECK(profile);
   // A NULL browser may only be given for dialogs.
   DCHECK(browser || view_type == VIEW_TYPE_EXTENSION_DIALOG);
-  content::SiteInstance* site_instance =
+  scoped_refptr<content::SiteInstance> site_instance =
       ProcessManager::Get(profile)->GetSiteInstanceForURL(url);
   ExtensionViewHost* host =
 #if defined(OS_MACOSX)
-      new ExtensionViewHostMac(extension, site_instance, url, view_type);
+      new ExtensionViewHostMac(extension, site_instance.get(), url, view_type);
 #else
-      new ExtensionViewHost(extension, site_instance, url, view_type);
+      new ExtensionViewHost(extension, site_instance.get(), url, view_type);
 #endif
   host->CreateView(browser);
   return host;
diff --git a/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.cc b/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.cc
index 927c00f..85a30cd 100644
--- a/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.cc
+++ b/extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.cc
@@ -127,18 +127,19 @@ void MimeHandlerViewGuest::CreateWebContents(
   // Use the mime handler extension's SiteInstance to create the guest so it
   // goes under the same process as the extension.
   ProcessManager* process_manager = ProcessManager::Get(browser_context());
-  content::SiteInstance* guest_site_instance =
+  scoped_refptr<content::SiteInstance> guest_site_instance =
       process_manager->GetSiteInstanceForURL(stream_->handler_url());
 
   // Clear the zoom level for the mime handler extension. The extension is
   // responsible for managing its own zoom. This is necessary for OOP PDF, as
   // otherwise the UI is zoomed and the calculations to determine the PDF size
   // mix zoomed and unzoomed units.
-  content::HostZoomMap::Get(guest_site_instance)
+  content::HostZoomMap::Get(guest_site_instance.get())
       ->SetZoomLevelForHostAndScheme(kExtensionScheme, stream_->extension_id(),
                                      0);
 
-  WebContents::CreateParams params(browser_context(), guest_site_instance);
+  WebContents::CreateParams params(browser_context(),
+                                   guest_site_instance.get());
   params.guest_delegate = this;
   callback.Run(WebContents::Create(params));
 }
diff --git a/extensions/browser/process_manager.cc b/extensions/browser/process_manager.cc
index 449e33c..894c8f9 100644
--- a/extensions/browser/process_manager.cc
+++ b/extensions/browser/process_manager.cc
@@ -119,7 +119,7 @@ class IncognitoProcessManager : public ProcessManager {
   ~IncognitoProcessManager() override {}
   bool CreateBackgroundHost(const Extension* extension,
                             const GURL& url) override;
-  SiteInstance* GetSiteInstanceForURL(const GURL& url) override;
+  scoped_refptr<SiteInstance> GetSiteInstanceForURL(const GURL& url) override;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(IncognitoProcessManager);
@@ -356,7 +356,7 @@ bool ProcessManager::CreateBackgroundHost(const Extension* extension,
     return true;  // TODO(kalman): return false here? It might break things...
 
   ExtensionHost* host =
-      new ExtensionHost(extension, GetSiteInstanceForURL(url), url,
+      new ExtensionHost(extension, GetSiteInstanceForURL(url).get(), url,
                         VIEW_TYPE_EXTENSION_BACKGROUND_PAGE);
   host->CreateRenderViewSoon();
   OnBackgroundHostCreated(host);
@@ -467,8 +467,9 @@ bool ProcessManager::RegisterRenderViewHost(RenderViewHost* render_view_host) {
   return true;
 }
 
-SiteInstance* ProcessManager::GetSiteInstanceForURL(const GURL& url) {
-  return site_instance_->GetRelatedSiteInstance(url);
+scoped_refptr<SiteInstance> ProcessManager::GetSiteInstanceForURL(
+    const GURL& url) {
+  return make_scoped_refptr(site_instance_->GetRelatedSiteInstance(url));
 }
 
 bool ProcessManager::IsBackgroundHostClosing(const std::string& extension_id) {
@@ -1030,7 +1031,8 @@ bool IncognitoProcessManager::CreateBackgroundHost(const Extension* extension,
   return false;
 }
 
-SiteInstance* IncognitoProcessManager::GetSiteInstanceForURL(const GURL& url) {
+scoped_refptr<SiteInstance> IncognitoProcessManager::GetSiteInstanceForURL(
+    const GURL& url) {
   const Extension* extension =
       extension_registry_->enabled_extensions().GetExtensionOrAppByURL(url);
   if (extension && !IncognitoInfo::IsSplitMode(extension)) {
diff --git a/extensions/browser/process_manager.h b/extensions/browser/process_manager.h
index 2bfcb60e4..60056a1 100644
--- a/extensions/browser/process_manager.h
+++ b/extensions/browser/process_manager.h
@@ -73,13 +73,10 @@ class ProcessManager : public KeyedService,
   ExtensionHost* GetBackgroundHostForExtension(const std::string& extension_id);
 
   // Returns the SiteInstance that the given URL belongs to.
-  // Callers should wrap the result in a scoped_refptr to ensure the
-  // SiteInstance becomes refcounted.
-  // TODO(devlin): The above comment clearly indicates that this should just
-  // return a refptr. Update callers.
   // TODO(aa): This only returns correct results for extensions and packaged
   // apps, not hosted apps.
-  virtual content::SiteInstance* GetSiteInstanceForURL(const GURL& url);
+  virtual scoped_refptr<content::SiteInstance> GetSiteInstanceForURL(
+      const GURL& url);
 
   // If the view isn't keeping the lazy background page alive, increments the
   // keepalive count to do so.