Do not send IPCs to unconnected renderers.

This CL is desirable today, to avoid the memory leaks associated with
queueing of IPCs that are sent to RenderProcessHosts that are
constructed, but not yet initialized - such RenderProcessHosts
accumulate IPC messages and flush them only after renderer process is
actually launched at a later point / after a call to RPH::Init.
This aspect of the CL is very similar to r562797 which landed earlier.

This CL is desirable for the future - a tentative long-term plan for
https://crbug.com/813045 is to avoid creating an IPC channel before
RPH::Init is called.  This means that in the long-term
RPH::GetChildIdentity will crash (dereferencing a nullptr
|RenderProcessHostImpl::child_connection_|) if called on an
uninitialized RPH.  This CL ensures that this crash won't happen in
ClientSideDetectionService::SendModelToProcess and
SpellcheckService::OnCustomDictionaryChanged by ensuring that these
methods only work with already initialized RPHs.

The changes under //chrome/browser/spellchecker and
//chrome/browser/safe_browsing skip uninitialized RPHs for 2 specific
IPCs/subsystems.  The changes under //content/browser/renderer_host
prevent future IPCs/subsystems from using uninitialized RPHs, by
adding a DCHECK to RenderProcessHostImpl::GetChildIdentity.
The new DCHECK necessitates setting |is_initialized_ = true| much
earlier in RenderProcessHostImpl::Init (because
ChromeContentBrowserClient::RenderProcessWillLaunch needs to call
RPH::GetChildIdentity).

Bug: 813045
Change-Id: I4783de85cb7c199ab360f8361acd54c63287e1df
Reviewed-on: https://chromium-review.googlesource.com/1095433Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Jialiu Lin <jialiul@chromium.org>
Reviewed-by: Rachel Blum <groby@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567099}

chrome/browser/safe_browsing/client_side_detection_service.cc

@@ -231,6 +231,8 @@ void ClientSideDetectionService::Observe(
 
 void ClientSideDetectionService::SendModelToProcess(
     content::RenderProcessHost* process) {
+  DCHECK(process->IsInitializedAndNotDead());
+
   // The ClientSideDetectionService is enabled if _any_ active profile has
   // SafeBrowsing turned on.  Here we check the profile for each renderer
   // process and only send the model to those that have SafeBrowsing enabled,
@@ -268,7 +270,9 @@ void ClientSideDetectionService::SendModelToRenderers() {
   for (content::RenderProcessHost::iterator i(
            content::RenderProcessHost::AllHostsIterator());
        !i.IsAtEnd(); i.Advance()) {
-    SendModelToProcess(i.GetCurrentValue());
+    content::RenderProcessHost* process = i.GetCurrentValue();
+    if (process->IsInitializedAndNotDead())
+      SendModelToProcess(process);
   }
 }
 

chrome/browser/spellchecker/spellcheck_service.cc

@@ -4,7 +4,9 @@
 
 #include "chrome/browser/spellchecker/spellcheck_service.h"
 
+#include <algorithm>
 #include <set>
+#include <utility>
 
 #include "base/logging.h"
 #include "base/stl_util.h"
@@ -288,8 +290,11 @@ void SpellcheckService::OnCustomDictionaryChanged(
   const std::vector<std::string> deletions(change.to_remove().begin(),
                                            change.to_remove().end());
   while (!process_hosts.IsAtEnd()) {
-    service_manager::Identity renderer_identity =
-        process_hosts.GetCurrentValue()->GetChildIdentity();
+    content::RenderProcessHost* process = process_hosts.GetCurrentValue();
+    if (!process->IsInitializedAndNotDead())
+      continue;
+
+    service_manager::Identity renderer_identity = process->GetChildIdentity();
     spellcheck::mojom::SpellCheckerPtr spellchecker;
     ChromeService::GetInstance()->connector()->BindInterface(
         service_manager::Identity(chrome::mojom::kRendererServiceName,

chrome/browser/spellchecker/spellcheck_service_browsertest.cc

@@ -22,7 +22,6 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/spellchecker/spell_check_host_chrome_impl.h"
 #include "chrome/browser/spellchecker/spellcheck_factory.h"
-#include "chrome/browser/spellchecker/spellcheck_service.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/constants.mojom.h"
@@ -48,6 +47,7 @@ class SpellcheckServiceBrowserTest : public InProcessBrowserTest,
 
   void SetUpOnMainThread() override {
     renderer_.reset(new content::MockRenderProcessHost(GetContext()));
+    renderer_->Init();
     prefs_ = user_prefs::UserPrefs::Get(GetContext());
   }
 

content/browser/renderer_host/render_process_host_impl.cc

@@ -1531,8 +1531,6 @@ bool RenderProcessHostImpl::Init() {
   if (IsInitializedAndNotDead())
     return true;
 
-  is_dead_ = false;
-
   base::CommandLine::StringType renderer_prefix;
   // A command prefix is something prepended to the command line of the spawned
   // process.
@@ -1554,11 +1552,13 @@ bool RenderProcessHostImpl::Init() {
   if (renderer_path.empty())
     return false;
 
+  is_initialized_ = true;
+  is_dead_ = false;
+  sent_render_process_ready_ = false;
+
   if (gpu_client_)
     gpu_client_->PreEstablishGpuChannel();
 
-  sent_render_process_ready_ = false;
-
   // We may reach Init() during process death notification (e.g.
   // RenderProcessExited on some observer). In this case the Channel may be
   // null, so we re-initialize it here.
@@ -1665,7 +1665,6 @@ bool RenderProcessHostImpl::Init() {
     ui::GpuSwitchingManager::GetInstance()->AddObserver(this);
   }
 
-  is_initialized_ = true;
   init_time_ = base::TimeTicks::Now();
   return true;
 }
@@ -2132,6 +2131,11 @@ void RenderProcessHostImpl::BindInterface(
 
 const service_manager::Identity& RenderProcessHostImpl::GetChildIdentity()
     const {
+  // GetChildIdentity should only be called if the RPH is (or soon will be)
+  // backed by an actual renderer process.  This helps prevent leaks similar to
+  // the ones raised in https://crbug.com/813045.
+  DCHECK(IsInitializedAndNotDead());
+
   return child_connection_->child_identity();
 }