GitLab

CL adapted from clamy@'s https://crrev.com/c/1565876.
This is a partial revert of r638122 and r642275, plus a new test.

This CL fixes a process leak that happens in navigations where the
current renderer process is not live (crashed process or NTP). The
mechanism is the following:

1) We want to navigate to a page in a FrameTreeNode where the current
RFH is not live. This RFH has a SiteInstance with id 1, and a process
with id 1.
2) We create a NavigationRequest, which takes as its starting
SiteInstance the SiteInstance with id 1.
3) We call into RenderFrameHostManager::GetFrameHostForNavigation. It
creates a speculative RFH, with a new SiteInstance with id 2, and a
process with id 2. It commits this RFH immediately, since the current
one is not live. This has the side effect of deleting process 1, however
it does not delete SiteInstance 1 since it is referenced by the
NavigationRequest. So SiteInstance 1 ends up associated with no RFH, and
has no process either.
4) We create the NavigationHandle and call DidStartNavigation. One of
the WebContentsObservers accesses NavigationHandle::GetStartingSiteInstance
(which is SiteInstance 1), and asks it for its process
(https://cs.chromium.org/chromium/src/chrome/browser/safe_browsing/safe_browsing_navigation_observer.cc?type=cs&q=safe_browsing::SafeBrowsingNavigationObserver::DidStartNavigation&g=0&l=185).
Because the SiteInstance no longer has a process, we re-create a process
with id 3 due to this call.
5) We finish the navigation and delete the last reference to
SiteInstance 1. However, this does not delete process 3, which is not
referenced anymore, and we leak the process.

To fix the issue, this CL makes sure that the starting SiteInstance is
captured just before creating the NavigationHandle, which is after a new
RFH has been committed to replace the non-live current RFH. This way,
the SiteInstance is properly linked to a RFH and it has a
RenderProcessHost. Separately, we can ensure that unused processes will
be cleaned up if GetProcess() is called unexpectedly.

TBR=clamy@chromium.org
TBR=jbudorick@chromium.org

Bug: 949977
Change-Id: Ia84e238af0036822dd8c4cd47be5b497d0e7be3d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1566643
Commit-Queue: Charlie Reis <creis@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#650619}