Shutdown renderer main message loop before blink::shutdown()

content/renderer/render_thread_impl.cc

Index: content/renderer/render_thread_impl.cc
diff --git a/content/renderer/render_thread_impl.cc b/content/renderer/render_thread_impl.cc
index b6fb747b15cb57b1a854ad4a498a0753b8a5f50d..a4216e3eda2d06a7bd06aae53d62897bbc3f56e0 100644
--- a/content/renderer/render_thread_impl.cc
+++ b/content/renderer/render_thread_impl.cc
@@ -697,7 +697,7 @@ void RenderThreadImpl::Init(
       new PeerConnectionDependencyFactory(p2p_socket_dispatcher_.get()));
 
   aec_dump_message_filter_ = new AecDumpMessageFilter(
-      GetIOTaskRunner(), message_loop()->task_runner());
+      GetIOTaskRunner(), main_task_runner());
 
   AddFilter(aec_dump_message_filter_.get());
 
@@ -987,6 +987,14 @@ void RenderThreadImpl::Shutdown() {
     // blink::shutdown() must be called after all strong references from
     // Chromium to Blink are cleared.
     blink::shutdown();
+
+    // Clear the message loop here because pending tasks can hold Persintent
+    // handle to GCed object, which can not outlive Oilpan heap.
+    if (main_message_loop_)
+      base::RunLoop().RunUntilIdle();

[kinuko, 2016/08/09 14:12:18]

Hmm, I feel uneasy calling this in production code. Do we really have a
guarantee that this finishes? In browser process we prefer just dropping tasks
at least after processing limited # of tasks.

How frequently does the crash happen?

[haraken, 2016/08/09 15:46:58]

Note that we're already calling RunUntilIdle at line 982. So this RunUntileIdle
won't make things that worse.

On the other hand, I totally agree with your point and that's why I want to
entirely remove the graceful shutdown sequence.

[kinuko, 2016/08/11 06:10:55]

I see that we're already calling it... thanks.  Another question is if we are
sure that running message loop until idle runs tasks that need to run (e.g. no
delayed tasks would hold persistent handles)-- if not making this change as a
stop-gap doesn't feel very convincing to me (depending on the severity of the
original crash).

+    main_message_loop_ = nullptr;
+
+    blink::Platform::shutdown();
   }
 
   // Delay shutting down DiscardableSharedMemoryManager until blink::shutdown
@@ -994,11 +1002,6 @@ void RenderThreadImpl::Shutdown() {
   // may try to unlock their underlying discardable memory.
   ChildThreadImpl::ShutdownDiscardableSharedMemoryManager();
 
-  // The message loop must be cleared after shutting down
-  // the DiscardableSharedMemoryManager, which needs to send messages
-  // to the browser process.
-  main_message_loop_.reset();
-
   lazy_tls.Pointer()->Set(nullptr);
 }
 
@@ -1185,7 +1188,8 @@ void RenderThreadImpl::InitializeWebKit(
   blink_platform_impl_.reset(new RendererBlinkPlatformImpl(
       renderer_scheduler_.get(),
       GetRemoteInterfaces()->GetWeakPtr()));
-  blink::initialize(blink_platform_impl_.get());
+  blink::Platform::initialize(blink_platform_impl_.get());
+  blink::initialize();
 
   v8::Isolate* isolate = blink::mainThreadIsolate();
   isolate->SetCreateHistogramFunction(CreateHistogram);
@@ -2069,7 +2073,7 @@ void RenderThreadImpl::OnMemoryPressure(
 
 scoped_refptr<base::SingleThreadTaskRunner>
 RenderThreadImpl::GetFileThreadTaskRunner() {
-  DCHECK(message_loop()->task_runner()->BelongsToCurrentThread());
+  DCHECK(main_task_runner()->BelongsToCurrentThread());
   if (!file_thread_) {
     file_thread_.reset(new base::Thread("Renderer::FILE"));
     file_thread_->Start();
@@ -2079,7 +2083,7 @@ RenderThreadImpl::GetFileThreadTaskRunner() {
 
 scoped_refptr<base::SingleThreadTaskRunner>
 RenderThreadImpl::GetMediaThreadTaskRunner() {
-  DCHECK(message_loop()->task_runner()->BelongsToCurrentThread());
+  DCHECK(main_task_runner()->BelongsToCurrentThread());
   if (!media_thread_) {
     media_thread_.reset(new base::Thread("Media"));
     media_thread_->Start();