Migrate GpuChildThread to use ConnectionFilter instead of the ChildThread's InterfaceRegistry to exp

content/gpu/gpu_child_thread.cc

Index: content/gpu/gpu_child_thread.cc
diff --git a/content/gpu/gpu_child_thread.cc b/content/gpu/gpu_child_thread.cc
index f2ad57e72cf35fc1b8d1729bf3e3297ad92fae2f..faa4adb9eccbc7b09d0b5e8fd58446fe955c0e70 100644
--- a/content/gpu/gpu_child_thread.cc
+++ b/content/gpu/gpu_child_thread.cc
@@ -9,10 +9,12 @@
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
+#include "base/threading/thread_checker.h"
 #include "build/build_config.h"
 #include "content/child/child_process.h"
 #include "content/common/field_trial_recorder.mojom.h"
 #include "content/gpu/gpu_service_factory.h"
+#include "content/public/common/connection_filter.h"
 #include "content/public/common/content_client.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/service_manager_connection.h"
@@ -22,8 +24,8 @@
 #include "gpu/ipc/service/gpu_watchdog_thread.h"
 #include "ipc/ipc_sync_message_filter.h"
 #include "media/gpu/ipc/service/media_gpu_channel_manager.h"
+#include "services/service_manager/public/cpp/binder_registry.h"
 #include "services/service_manager/public/cpp/connector.h"
-#include "services/service_manager/public/cpp/interface_registry.h"
 #include "services/ui/gpu/interfaces/gpu_service.mojom.h"
 
 #if defined(USE_OZONE)
@@ -47,6 +49,7 @@ ChildThreadImpl::Options GetOptions() {
     builder.AddStartupFilter(message_filter);
 #endif
 
+  builder.AutoStartServiceManagerConnection(false);
   builder.ConnectToBrowser(true);
 
   return builder.Build();
@@ -54,6 +57,86 @@ ChildThreadImpl::Options GetOptions() {
 
 }  // namespace
 
+// This ConnectionFilter queues all incoming bind interface requests until
+// Release() is called.
+class GpuChildThread::QueueingConnectionFilter : public ConnectionFilter {
+ public:
+  QueueingConnectionFilter(
+      scoped_refptr<base::SequencedTaskRunner> io_task_runner,
+      std::unique_ptr<service_manager::BinderRegistry> registry)
+      : io_task_runner_(io_task_runner),
+        registry_(std::move(registry)),
+        weak_factory_(this) {
+    // This will be reattached by any of the IO thread functions on first call.
+    io_thread_checker_.DetachFromThread();
+  }
+  ~QueueingConnectionFilter() override {
+    DCHECK(io_thread_checker_.CalledOnValidThread());
+  }
+
+  void Release() {
+    base::AutoLock lock(lock_);
+    released_ = true;
+    io_task_runner_->PostTask(
+        FROM_HERE, base::Bind(&QueueingConnectionFilter::ReleaseOnIOThread,
+                              base::Unretained(this)));
+  }
+
+  base::WeakPtr<QueueingConnectionFilter> GetWeakPtr() {

[Ken Rockot(use gerrit already), 2017/04/20 15:30:22]

WeakPtr must be dereferenced and invalidated from the same thread - you're
expecting to use a WeakPtr on one thread while invalidating it on another.
Instead of doing that, how about binding a callback to Release on the IO thread
which uses a WeakPtr, and just having GpuChildThread hold and invoke the
callback.

Then you don't have to expose a public GetWeakPtr method, and the WeakPtr will
always be dereferenced only on the IO thread where it's also invalidated.

It's a little ugly:

base::Closure GetReleaseCallback() {
  return base::Bind(base::IgnoreResult(&base::TaskRunner::PostTask),
                    io_task_runner_, FROM_HERE,
                    base::Bind(&QueueingConnectionFilter::Release,
                               weak_factory_.GetWeakPtr()));
}

but I think safe.

+    return weak_factory_.GetWeakPtr();
+  }
+
+ private:
+  struct PendingRequest {
+    service_manager::Identity source_identity;
+    std::string interface_name;
+    mojo::ScopedMessagePipeHandle interface_pipe;
+  };
+
+  // ConnectionFilter:
+  void OnBindInterface(const service_manager::ServiceInfo& source_info,
+                       const std::string& interface_name,
+                       mojo::ScopedMessagePipeHandle* interface_pipe,
+                       service_manager::Connector* connector) override {
+    DCHECK(io_thread_checker_.CalledOnValidThread());
+    if (registry_->CanBindInterface(interface_name)) {
+      base::AutoLock lock(lock_);
+      if (released_) {
+        registry_->BindInterface(source_info.identity, interface_name,
+                                 std::move(*interface_pipe));
+      } else {
+        std::unique_ptr<PendingRequest> request =
+            base::MakeUnique<PendingRequest>();
+        request->source_identity = source_info.identity;
+        request->interface_name = interface_name;
+        request->interface_pipe = std::move(*interface_pipe);
+        pending_requests_.push_back(std::move(request));
+      }
+    }
+  }
+
+  void ReleaseOnIOThread() {
+    DCHECK(io_thread_checker_.CalledOnValidThread());
+    for (auto& request : pending_requests_) {
+      registry_->BindInterface(request->source_identity,
+                               request->interface_name,
+                               std::move(request->interface_pipe));
+    }
+  }
+
+  base::ThreadChecker io_thread_checker_;
+  scoped_refptr<base::SequencedTaskRunner> io_task_runner_;
+  // Guards |released_|.
+  base::Lock lock_;
+  bool released_ = false;
+  std::vector<std::unique_ptr<PendingRequest>> pending_requests_;
+  std::unique_ptr<service_manager::BinderRegistry> registry_;
+
+  base::WeakPtrFactory<QueueingConnectionFilter> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(QueueingConnectionFilter);
+};
+
 GpuChildThread::GpuChildThread(
     std::unique_ptr<gpu::GpuWatchdogThread> watchdog_thread,
     bool dead_on_arrival,
@@ -74,6 +157,7 @@ GpuChildThread::GpuChildThread(const InProcessChildThreadParams& params,
                                const gpu::GpuFeatureInfo& gpu_feature_info)
     : GpuChildThread(ChildThreadImpl::Options::Builder()
                          .InBrowserProcess(params)
+                         .AutoStartServiceManagerConnection(false)
                          .ConnectToBrowser(true)
                          .Build(),
                      nullptr /* watchdog_thread */,
@@ -119,15 +203,24 @@ void GpuChildThread::Init(const base::Time& process_start_time) {
     media::SetMediaDrmBridgeClient(
         GetContentClient()->GetMediaDrmBridgeClient());
 #endif
-  // We don't want to process any incoming interface requests until
-  // OnInitialize() is invoked.
-  GetInterfaceRegistry()->PauseBinding();
+  AssociatedInterfaceRegistry* associated_registry = &associated_interfaces_;
+  associated_registry->AddInterface(base::Bind(
+      &GpuChildThread::CreateGpuMainService, base::Unretained(this)));
 
+  auto registry = base::MakeUnique<service_manager::BinderRegistry>();
+  registry->AddInterface(base::Bind(&GpuChildThread::BindServiceFactoryRequest,
+                                    base::Unretained(this)),
+                         base::ThreadTaskRunnerHandle::Get());
   if (GetContentClient()->gpu())  // NULL in tests.
-    GetContentClient()->gpu()->Initialize(this);
-  AssociatedInterfaceRegistry* registry = &associated_interfaces_;
-  registry->AddInterface(base::Bind(
-      &GpuChildThread::CreateGpuMainService, base::Unretained(this)));
+    GetContentClient()->gpu()->Initialize(this, registry.get());
+
+  std::unique_ptr<QueueingConnectionFilter> filter =
+      base::MakeUnique<QueueingConnectionFilter>(GetIOTaskRunner(),
+                                                 std::move(registry));
+  queueing_filter_ = filter->GetWeakPtr();
+  GetServiceManagerConnection()->AddConnectionFilter(std::move(filter));
+
+  StartServiceManagerConnection();
 }
 
 void GpuChildThread::OnFieldTrialGroupFinalized(const std::string& trial_name,
@@ -196,16 +289,11 @@ void GpuChildThread::CreateGpuService(
   service_factory_.reset(new GpuServiceFactory(
       gpu_service_->media_gpu_channel_manager()->AsWeakPtr()));
 
-  GetInterfaceRegistry()->AddInterface(base::Bind(
-      &GpuChildThread::BindServiceFactoryRequest, base::Unretained(this)));
-
-  if (GetContentClient()->gpu()) {  // NULL in tests.
-    GetContentClient()->gpu()->ExposeInterfacesToBrowser(GetInterfaceRegistry(),
-                                                         gpu_preferences);
-    GetContentClient()->gpu()->ConsumeInterfacesFromBrowser(GetConnector());
-  }
+  if (GetContentClient()->gpu())  // NULL in tests.
+    GetContentClient()->gpu()->GpuServiceInitialized(gpu_preferences);
 
-  GetInterfaceRegistry()->ResumeBinding();
+  if (queueing_filter_)
+    queueing_filter_->Release();
 }
 
 void GpuChildThread::CreateFrameSinkManager(