[aw] Fix a race associated with terminating a WebView renderer.

Race condition is caused by RenderProcessHost::Shutdown recreating the
sync IPC channel and asynchronously causing SyncContext::OnChannelOpened
to be called on the IO thread.

In the interim the RPH has been deleted, causing the WaitableEvent that
was passed to the SyncContext to be destroyed before it is used.

Because we never intend to use the WaitableEvent to signal shutdown in
WebView, it's simplest to just pass null for the shutdown_event,
indicating that the caller never intends to signal shutdown.

Bug: 904043,865062
Change-Id: I69a51adc78a0f1c0d6f84fbb7206fc9d58e69ed2
Reviewed-on: https://chromium-review.googlesource.com/c/1329670
Commit-Queue: Tobias Sargeant <tobiasjs@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607343}

content/browser/renderer_host/render_process_host_impl.cc

@@ -1587,10 +1587,6 @@ RenderProcessHostImpl::RenderProcessHostImpl(
           id_)),
       channel_connected_(false),
       sent_render_process_ready_(false),
-#if defined(OS_ANDROID)
-      never_signaled_(base::WaitableEvent::ResetPolicy::MANUAL,
-                      base::WaitableEvent::InitialState::NOT_SIGNALED),
-#endif
       renderer_host_binding_(this),
       instance_weak_factory_(
           new base::WeakPtrFactory<RenderProcessHostImpl>(this)),
@@ -1902,9 +1898,11 @@ void RenderProcessHostImpl::InitializeChannelProxy() {
   // See crbug.com/526842 for details.
 #if defined(OS_ANDROID)
   if (GetContentClient()->UsingSynchronousCompositing()) {
-    channel_ = IPC::SyncChannel::Create(this, io_task_runner.get(),
-                                        base::ThreadTaskRunnerHandle::Get(),
-                                        &never_signaled_);
+    // Android never performs a clean shutdown, so we pass nullptr for
+    // shudown_event to indicate that we never intend to signal a shutdown.
+    channel_ =
+        IPC::SyncChannel::Create(this, io_task_runner.get(),
+                                 base::ThreadTaskRunnerHandle::Get(), nullptr);
   }
 #endif  // OS_ANDROID
   if (!channel_) {

content/browser/renderer_host/render_process_host_impl.h

@@ -834,15 +834,6 @@ class CONTENT_EXPORT RenderProcessHostImpl
   bool channel_connected_;
   bool sent_render_process_ready_;
 
-#if defined(OS_ANDROID)
-  // UI thread is the source of sync IPCs and all shutdown signals.
-  // Therefore a proper shutdown event to unblock the UI thread is not
-  // possible without massive refactoring shutdown code.
-  // Luckily Android never performs a clean shutdown. So explicitly
-  // ignore this problem.
-  base::WaitableEvent never_signaled_;
-#endif
-
   scoped_refptr<ResourceMessageFilter> resource_message_filter_;
   std::unique_ptr<FileSystemManagerImpl, BrowserThread::DeleteOnIOThread>
       file_system_manager_impl_;

ipc/ipc_sync_channel.cc

@@ -495,11 +495,13 @@ void SyncChannel::SyncContext::OnChannelError() {
 }
 
 void SyncChannel::SyncContext::OnChannelOpened() {
-  shutdown_watcher_.StartWatching(
-      shutdown_event_,
-      base::Bind(&SyncChannel::SyncContext::OnShutdownEventSignaled,
-                 base::Unretained(this)),
-      base::SequencedTaskRunnerHandle::Get());
+  if (shutdown_event_) {
+    shutdown_watcher_.StartWatching(
+        shutdown_event_,
+        base::BindOnce(&SyncChannel::SyncContext::OnShutdownEventSignaled,
+                       base::Unretained(this)),
+        base::SequencedTaskRunnerHandle::Get());
+  }
   Context::OnChannelOpened();
 }
 
@@ -539,6 +541,11 @@ std::unique_ptr<SyncChannel> SyncChannel::Create(
     const scoped_refptr<base::SingleThreadTaskRunner>& listener_task_runner,
     bool create_pipe_now,
     base::WaitableEvent* shutdown_event) {
+  // TODO(tobiasjs): The shutdown_event object is passed to a refcounted
+  // Context object, and as a result it is not easy to ensure that it
+  // outlives the Context.  There should be some way to either reset
+  // the shutdown_event when it is destroyed, or allow the Context to
+  // control the lifetime of shutdown_event.
   std::unique_ptr<SyncChannel> channel =
       Create(listener, ipc_task_runner, listener_task_runner, shutdown_event);
   channel->Init(channel_handle, mode, create_pipe_now);