Refactor Blink's ServiceConnector and add ability to mock in layout tests

content/renderer/mojo/blink_connector_impl.cc

Index: content/renderer/mojo/blink_connector_impl.cc
diff --git a/content/renderer/mojo/blink_connector_impl.cc b/content/renderer/mojo/blink_connector_impl.cc
new file mode 100644
index 0000000000000000000000000000000000000000..6f75166a0b809c5a2385e78d166b58418b7f6364
--- /dev/null
+++ b/content/renderer/mojo/blink_connector_impl.cc
@@ -0,0 +1,90 @@
+// Copyright 2017 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.
+
+#include "content/renderer/mojo/blink_connector_impl.h"
+
+#include <utility>
+
+#include "base/bind.h"
+#include "base/single_thread_task_runner.h"
+#include "base/threading/thread_task_runner_handle.h"
+#include "services/service_manager/public/cpp/identity.h"
+#include "services/service_manager/public/cpp/interface_provider.h"
+
+namespace content {
+
+BlinkConnectorImpl::BlinkConnectorImpl(
+    std::unique_ptr<service_manager::Connector> connector)
+    : connector_(std::move(connector)),
+      main_thread_task_runner_(base::ThreadTaskRunnerHandle::Get()),
+      weak_ptr_factory_(this) {}
+
+BlinkConnectorImpl::~BlinkConnectorImpl() = default;
+
+void BlinkConnectorImpl::bindInterface(const char* service_name,
+                                       const char* interface_name,
+                                       mojo::ScopedMessagePipeHandle handle) {
+  // |connector_| is null in some testing contexts.
+  if (!connector_.get())

[dcheng, 2017/03/21 18:56:18]

Nit: no .get()

(Also, is this 'null in some testing contexts' something we'd be able to
eventually fix? It seems like we should also be trying to fix the tests so that
we don't have test-only branches)

[blundell, 2017/03/23 10:40:33]

Done.
Part of the larger RenderThread::current() issue, as you noted.

+    return;
+
+  if (!main_thread_task_runner_->BelongsToCurrentThread()) {
+    main_thread_task_runner_->PostTask(
+        FROM_HERE,
+        base::Bind(&BlinkConnectorImpl::bindInterface, GetWeakPtr(),
+                   service_name, interface_name, base::Passed(&handle)));
+    return;
+  }
+
+  // Tests might have overridden this interface with a local implementation.
+  InterfaceBinderMap* overrides_for_service =
+      GetOverridesForService(service_name);
+  if (overrides_for_service) {
+    auto it = overrides_for_service->find(std::string(interface_name));
+    if (it != overrides_for_service->end()) {
+      it->second.Run(std::move(handle));
+      return;
+    }
+  }
+
+  service_manager::Identity service_identity(
+      service_name, service_manager::mojom::kInheritUserID);
+  connector_->BindInterface(service_identity, interface_name,
+                            std::move(handle));
+}
+
+void BlinkConnectorImpl::AddOverrideForTesting(
+    const std::string& service_name,
+    const std::string& interface_name,
+    const base::Callback<void(mojo::ScopedMessagePipeHandle)>& binder) {
+  if (service_binders_.find(service_name) == service_binders_.end())
+    service_binders_[service_name] = base::MakeUnique<InterfaceBinderMap>();
+
+  (*(service_binders_[service_name].get()))[interface_name] = binder;

[dcheng, 2017/03/21 18:56:18]

I think this can be simplified slightly to:

(*(service_binders_[service_name]))[interface_name] = binder;

[blundell, 2017/03/23 10:40:33]

Done.

+}
+
+void BlinkConnectorImpl::ClearOverridesForTesting() {
+  service_binders_.clear();
+}
+
+base::WeakPtr<BlinkConnectorImpl> BlinkConnectorImpl::GetWeakPtr() {
+  return weak_ptr_factory_.GetWeakPtr();
+}
+
+BlinkConnectorImpl::InterfaceBinderMap*
+BlinkConnectorImpl::GetOverridesForService(const char* service_name) {
+  // Short-circuit out in the case where there are no overrides (always true in
+  // production).
+  if (service_binders_.empty())
+    return nullptr;
+
+  auto it = service_binders_.find(std::string(service_name));
+
+  if (it != service_binders_.end())
+    return it->second.get();
+
+  return nullptr;
+}
+
+}  // namespace content