Fix data races in MojoShellConnectionImpl

content/browser/renderer_host/render_process_host_impl.cc

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Represents the browser side of the browser <--> renderer communication
 // channel. There will be one RenderProcessHost per renderer process.
 
 #include "content/browser/renderer_host/render_process_host_impl.h"
 
 #include <algorithm>
@@ skipping 454 matching lines @@
 
 // Held by the RPH's BrowserContext's MojoShellConnection, ownership transferred
 // back to RPH upon RPH destruction.
 class RenderProcessHostImpl::ConnectionFilterImpl : public ConnectionFilter {
  public:
   ConnectionFilterImpl(
       const shell::Identity& child_identity,
       std::unique_ptr<shell::InterfaceRegistry> registry)
       : child_identity_(child_identity),
         registry_(std::move(registry)),
-        weak_factory_(this) {}
+        weak_factory_(this) {
+    // Registration of this filter may race with browser shutdown, in which case
+    // it's possible for this filter to be destroyed on the main thread. This
+    // is fine as long as the filter hasn't been used on the IO thread yet. We
+    // detach the ThreadChecker initially and the first use of the filter will
+    // bind it.
+    thread_checker_.DetachFromThread();
+  }
+
   ~ConnectionFilterImpl() override {
-    DCHECK_CURRENTLY_ON(BrowserThread::IO);
+    DCHECK(thread_checker_.CalledOnValidThread());
   }
 
  private:
   // ConnectionFilter:
   bool OnConnect(const shell::Identity& remote_identity,
                  shell::InterfaceRegistry* registry,
                  shell::Connector* connector) override {
+    DCHECK(thread_checker_.CalledOnValidThread());
+    DCHECK_CURRENTLY_ON(BrowserThread::IO);
     // We only fulfill connections from the renderer we host.
     if (child_identity_.name() != remote_identity.name() ||
         child_identity_.instance() != remote_identity.instance()) {
       return false;
     }
 
     std::set<std::string> interface_names;
     registry_->GetInterfaceNames(&interface_names);
     for (auto& interface_name : interface_names) {
       // Note that the added callbacks may outlive this object, which is
       // destroyed in RPH::Cleanup().
       registry->AddInterface(interface_name,
                              base::Bind(&ConnectionFilterImpl::GetInterface,
                                         weak_factory_.GetWeakPtr(),
                                         interface_name));
     }
     return true;
   }
 
   void GetInterface(const std::string& interface_name,
                     mojo::ScopedMessagePipeHandle handle) {
+    DCHECK(thread_checker_.CalledOnValidThread());
+    DCHECK_CURRENTLY_ON(BrowserThread::IO);
     shell::mojom::InterfaceProvider* provider = registry_.get();
     provider->GetInterface(interface_name, std::move(handle));
   }
 
+  base::ThreadChecker thread_checker_;
   shell::Identity child_identity_;
   std::unique_ptr<shell::InterfaceRegistry> registry_;
   base::WeakPtrFactory<ConnectionFilterImpl> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ConnectionFilterImpl);
 };
 
 base::MessageLoop*
 RenderProcessHostImpl::GetInProcessRendererThreadForTesting() {
   return g_in_process_thread;
@@ skipping 2318 matching lines @@
   LOG(ERROR) << "Terminating render process for bad Mojo message: " << error;
 
   // The ReceivedBadMessage call below will trigger a DumpWithoutCrashing. Alias
   // enough information here so that we can determine what the bad message was.
   base::debug::Alias(&error);
   bad_message::ReceivedBadMessage(process.get(),
                                   bad_message::RPH_MOJO_PROCESS_ERROR);
 }
 
 }  // namespace content