Remove eager SetSite on the current SiteInstance in session restore c…

…ases

This code does not appear to be doing anything useful at this
point.  This is because in the session restore flow, we already
create the restored SiteInstance via CreateForURL from:

  tab_util::GetSiteInstanceForNewTab()
  chrome::(anonymous namespace)::CreateRestoredTab()
  chrome::AddRestoredTab()

It appears that the behavior of
tab_util::GetSiteInstanceForNewTab() might've evolved, but now it
calls CreateForURL() for all URLs except for those where
ShouldAssignSiteForURL(url) is false (but this case is also
skipped by the logic being removed here).

The implementation of CreateForURL() already calls SetSite(),
which means that when we get to DetermineSiteInstanceForURL(),
`current_instance`'s HasSite() will be true and we will never hit
the dest_is_restore logic.

Other considerations for removing this logic include:

- DetermineSiteInstanceForURL() shouldn't really have side
  effects.

- session restore no longer loads all the restored tabs right
  away, deferring most of them until the user focuses them.

- the process-per-site behavior of restored tabs should also be
  ensured outside of this function.  In particular, GetProcess()
  is already called before getting to DetermineSiteInstanceForURL
  on restored SiteInstances, via the following stack:

  content::RenderProcessHostImpl::GetProcessHostForSiteInstance()
  content::SiteInstanceImpl::GetProcess()
  content::RenderFrameHostManager::CreateRenderFrameHost()
  content::RenderFrameHostManager::InitRoot()
  content::FrameTree::Init()
  content::WebContentsImpl::Init()
  content::WebContents::CreateWithSessionStorage()
  chrome::(anonymous namespace)::CreateRestoredTab()
  chrome::AddRestoredTab()

  Thus, GetProcessHostForSiteInstance() should be able to ensure that
  process-per-site cases behave correctly.  Unfortunately, there's
  currently an independent bug with that for NTP in crbug.com/1399874,
  but it should be fixed in a different way; the logic being removed
  here won't help it due to the reasons explained above.

Change-Id: Ia0695a861b01b8c2167e5a32fa10ee92b5675b42
Bug: 1394513, 1399874
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4089811
Reviewed-by: Charlie Reis <creis@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1084596}

content/browser/renderer_host/render_frame_host_manager.cc

@@ -2429,8 +2429,7 @@ RenderFrameHostManager::DetermineSiteInstanceForURL(
 
   // If we haven't used our SiteInstance yet, then we can use it for this
   // navigation.  We won't commit the SiteInstance to this site until the
-  // response is received (in OnResponseStarted), unless the navigation entry
-  // was restored or it's a Web UI as described below.
+  // response is received (in OnResponseStarted).
   // TODO(ahemery): In theory we should be able to go for an unused
   // SiteInstance with the same web exposed isolation status.
   if (!current_instance->HasSite() && !dest_url_info.IsIsolated() &&
@@ -2480,25 +2479,6 @@ RenderFrameHostManager::DetermineSiteInstanceForURL(
                                     SiteInstanceRelation::RELATED);
     }
 
-    // Normally the "site" on the SiteInstance is set lazily when the response
-    // is received and SiteInstance selection is finalized. This is to
-    // support better process sharing in case the site redirects to some other
-    // site: we want to use the destination site in the site instance.
-    //
-    // In the case of session restore, as it loads all the pages immediately
-    // we need to set the site first, otherwise after a restore none of the
-    // pages would share renderers in process-per-site.
-    //
-    // The embedder can request some urls never to be assigned to SiteInstance
-    // through the ShouldAssignSiteForURL() content client method, so that
-    // renderers created for particular chrome urls (e.g. the chrome-native://
-    // scheme) can be reused for subsequent navigations in the same WebContents.
-    // See http://crbug.com/386542.
-    if (dest_is_restore &&
-        SiteInstanceImpl::ShouldAssignSiteForURL(dest_url_info.url)) {
-      current_instance->ConvertToDefaultOrSetSite(dest_url_info);
-    }
-
     AppendReason(reason, "DetermineSiteInstanceForURL => current_instance");
     return SiteInstanceDescriptor(current_instance);
   }

content/browser/site_instance_impl.h

@@ -316,6 +316,12 @@ class CONTENT_EXPORT SiteInstanceImpl final : public SiteInstance {
   void ConvertToDefaultOrSetSite(const UrlInfo& url_info);
 
   // Returns whether SetSite() has been called.
+  //
+  // In some cases, the "site" is not set at SiteInstance creation time, and
+  // instead it's set lazily when a navigation response is received and
+  // SiteInstance selection is finalized. This is to support better process
+  // sharing in case the site redirects to some other site: we want to use the
+  // destination site in the SiteInstance.
   bool HasSite() const;
 
   // Returns whether there is currently a related SiteInstance (registered with