Revert "Avoid unbounded queueing of IPC messages for crashed renderers."

This reverts commit b29fffc2.

Reason for revert: Caused https://crbug.com/861728

Original change's description:
> Avoid unbounded queueing of IPC messages for crashed renderers.
>
> If a renderer process crashes, then we retain the associated
> RenderProcessHostImpl object (to retain the old render_process_host_id
> that may have been stored elsewhere in the system).  Before this CL this
> had an undesired interaction with the ability of RenderProcessHostImpl
> to queue IPCs between the call to Init and the time when a renderer
> process is actually spawned (disclaimer - in practice the queueing has a
> slightly/undesirably longer timeline).
>
> Before this CL, RenderProcessHostImpl::ProcessDied(...) would call
> EnableSendQueue().  This meant that an unbounded number of IPC messages
> may be accumulated in
> ChannelAssociatedGroupController::outgoing_messages_ (until the crashed
> process is restarted).
>
> After this CL, RenderProcessHostImpl::ProcessDied(...) no longer calls
> EnableSendQueue().
> - Short-term: subsystems that require queueing of IPCs should
>   call RPH::EnableSendQueue() themselves (e.g. like RenderViewHostImpl's
>   constructor does).
> - Long-term: we should discard IPCs until RenderProcessHostImpl::Init
>   is called - queueing should only take place in the short time between
>   calling RenderProcessHostImpl::Init and the time the renderer process
>   is actually launched.
>
> There is a small risk that this CL will introduce crashes similar to the
> ones seen in https://crbug.com/658759.
>
> Bug: 813045
> Change-Id: I838bf7ea443633ba0f2314d2de5c83891ec9ac5f
> Reviewed-on: https://chromium-review.googlesource.com/1073621
> Reviewed-by: Nasko Oskov <nasko@chromium.org>
> Reviewed-by: Ken Rockot <rockot@chromium.org>
> Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#562491}

TBR=nasko@chromium.org,rockot@chromium.org,lukasza@chromium.org


Bug: 813045, 861728
Change-Id: I640ad5080d8b5475d3ab0350c45ff32a6d152667
Reviewed-on: https://chromium-review.googlesource.com/1151912
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578689}

content/browser/renderer_host/render_process_host_browsertest.cc

@@ -31,7 +31,6 @@
 #include "content/shell/browser/shell_browser_main_parts.h"
 #include "content/shell/browser/shell_content_browser_client.h"
 #include "content/test/test_content_browser_client.h"
-#include "ipc/ipc_mojo_bootstrap.h"
 #include "media/base/bind_to_current_loop.h"
 #include "media/base/media_switches.h"
 #include "media/base/test_data_util.h"
@@ -1087,62 +1086,5 @@ IN_PROC_BROWSER_TEST_F(RenderProcessHostTest, FastShutdownForStartingProcess) {
   process->Cleanup();
 }
 
-// Regression test for one part of https://crbug.com/813045.
-IN_PROC_BROWSER_TEST_F(RenderProcessHostTest,
-                       NoUnboundedQueueingOfIpcs_CrashedProcess) {
-  // Navigate to a random page (this guarantees that a renderer had to be
-  // spawned to render the page).
-  ASSERT_TRUE(embedded_test_server()->Start());
-  GURL main_url(embedded_test_server()->GetURL("/title1.html"));
-  EXPECT_TRUE(NavigateToURL(shell(), main_url));
-
-  // Kill the renderer process.
-  RenderProcessHost* process =
-      shell()->web_contents()->GetMainFrame()->GetProcess();
-  RenderProcessHostWatcher crash_observer(
-      process, RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);
-  EXPECT_TRUE(process->Shutdown(0));
-  crash_observer.Wait();
-
-  // Repeatedly send an IPC to the killed renderer process.
-  //
-  // This approximates a scenario where (over a long period of time) Chromium
-  // tries send an IPC to all renderers about environment changes (for example,
-  // see calls to RendererInterface::OnNetworkQualityChanged mentioned in
-  // https://crbug.com/813045#c17).
-  constexpr size_t kNumberOfTestIterations =
-      IPC::MojoBootstrap::kMaxOutgoingMessagesSizeForTesting + 2;
-  for (size_t i = 0; i < kNumberOfTestIterations; i++) {
-    // Verify that the process is still dead.
-    EXPECT_FALSE(process->IsInitializedAndNotDead());
-    EXPECT_FALSE(process->GetChannel());
-    EXPECT_FALSE(process->GetProcess().IsValid());
-    EXPECT_EQ(base::kNullProcessHandle, process->GetProcess().Handle());
-
-    // Attempt to send an IPC message to the dead process.
-    std::string user_agent = base::StringPrintf("to-be-discarded-%zu", i);
-    if (process->GetRendererInterface())
-      process->GetRendererInterface()->SetUserAgent(user_agent);
-  }
-
-  // The main test verification is that the loop above didn't hit the CHECK in
-  // ipc/ipc_mojo_bootstrap.cc:
-  //
-  //      CHECK_LE(outgoing_messages_.size(),
-  //               MojoBootstrap::kMaxOutgoingMessagesSizeForTesting);
-  //
-  // No messages should be accumulated/queued for a crashed renderer process.
-  // Instead, such messages should be discarded.
-  //
-  // If the messages weren't discarded, then the SetUserAgent should hit another
-  // DCHECK in RenderThreadImpl::SetUserAgent (DCHECK(user_agent_.IsNull()))
-  // after reloading the page.
-  ReloadBlockUntilNavigationsComplete(shell(), 1);
-  EXPECT_EQ(process, shell()->web_contents()->GetMainFrame()->GetProcess());
-  EXPECT_EQ(process->GetID(),
-            shell()->web_contents()->GetMainFrame()->GetProcess()->GetID());
-  EXPECT_TRUE(process->IsInitializedAndNotDead());
-}
-
 }  // namespace
 }  // namespace content

content/browser/renderer_host/render_process_host_impl.cc

@@ -3937,6 +3937,13 @@ void RenderProcessHostImpl::ProcessDied(
     iter.Advance();
   }
 
+  // Initialize a new ChannelProxy in case this host is re-used for a new
+  // process. This ensures that new messages can be sent on the host ASAP (even
+  // before Init()) and they'll eventually reach the new process.
+  //
+  // Note that this may have already been called by one of the above observers
+  EnableSendQueue();
+
   // It's possible that one of the calls out to the observers might have caused
   // this object to be no longer needed.
   if (delayed_cleanup_needed_)

ipc/ipc_mojo_bootstrap.h

@@ -56,8 +56,6 @@ class COMPONENT_EXPORT(IPC) MojoBootstrap {
   virtual void Flush() = 0;
 
   virtual mojo::AssociatedGroup* GetAssociatedGroup() = 0;
-
-  enum { kMaxOutgoingMessagesSizeForTesting = 100000u };
 };
 
 }  // namespace IPC