Revert of [mojo] Enable ServiceRegistryImpl to override remote services...

Revert of [mojo] Enable ServiceRegistryImpl to override remote services (patchset #12 id:220001 of https://codereview.chromium.org/1470153002/ )

Reason for revert:
Causing Oilpan leak bot failure

https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Leak/builds/17113

Original issue's description:
> [mojo] Enable ServiceRegistryImpl to override remote services
>
> This adds a new AddServiceOverride method to ServiceRegistryImpl.
> The purpose of this is to allow testing code to intercept remote
> service connections and opaquely provide a mock implementation of
> any service.
>
> The new interface is also added to the ServiceRegistry's JS
> bindings.
>
> Committed: https://crrev.com/9b0d7173af550086d5688137752a6f70109e9c98
> Cr-Commit-Position: refs/heads/master@{#371736}

TBR=jam@chromium.org,reillyg@chromium.org,rockot@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.

Review URL: https://codereview.chromium.org/1639233004

Cr-Commit-Position: refs/heads/master@{#372061}

content/common/mojo/service_registry_impl.cc

@@ -39,14 +39,9 @@ void ServiceRegistryImpl::BindRemoteServiceProvider(
   }
 }
 
-void ServiceRegistryImpl::AddServiceOverrideForTesting(
+void ServiceRegistryImpl::AddService(
     const std::string& service_name,
-    const ServiceFactory& factory) {
-  service_overrides_[service_name] = factory;
-}
-
-void ServiceRegistryImpl::AddService(const std::string& service_name,
-                                     const ServiceFactory service_factory) {
+    const base::Callback<void(mojo::ScopedMessagePipeHandle)> service_factory) {
   service_factories_[service_name] = service_factory;
 }
 
@@ -57,12 +52,6 @@ void ServiceRegistryImpl::RemoveService(const std::string& service_name) {
 void ServiceRegistryImpl::ConnectToRemoteService(
     const base::StringPiece& service_name,
     mojo::ScopedMessagePipeHandle handle) {
-  auto override_it = service_overrides_.find(service_name.as_string());
-  if (override_it != service_overrides_.end()) {
-    override_it->second.Run(std::move(handle));
-    return;
-  }
-
   if (!remote_provider_) {
     pending_connects_.push(
         std::make_pair(service_name.as_string(), handle.release()));
@@ -83,7 +72,9 @@ base::WeakPtr<ServiceRegistry> ServiceRegistryImpl::GetWeakPtr() {
 void ServiceRegistryImpl::ConnectToService(
     const mojo::String& name,
     mojo::ScopedMessagePipeHandle client_handle) {
-  auto it = service_factories_.find(name);
+  std::map<std::string,
+           base::Callback<void(mojo::ScopedMessagePipeHandle)> >::iterator it =
+      service_factories_.find(name);
   if (it == service_factories_.end())
     return;
 

content/common/mojo/service_registry_impl.h

@@ -24,8 +24,6 @@ class CONTENT_EXPORT ServiceRegistryImpl
     : public ServiceRegistry,
       public NON_EXPORTED_BASE(mojo::ServiceProvider) {
  public:
-  using ServiceFactory = base::Callback<void(mojo::ScopedMessagePipeHandle)>;
-
   ServiceRegistryImpl();
   ~ServiceRegistryImpl() override;
 
@@ -38,15 +36,10 @@ class CONTENT_EXPORT ServiceRegistryImpl
   // ServiceProvider.
   void BindRemoteServiceProvider(mojo::ServiceProviderPtr service_provider);
 
-  // Registers a local service factory to intercept ConnectToRemoteService
-  // requests instead of actually connecting to the remote registry. Used only
-  // for testing.
-  void AddServiceOverrideForTesting(const std::string& service_name,
-                                    const ServiceFactory& service_factory);
-
   // ServiceRegistry overrides.
   void AddService(const std::string& service_name,
-                  const ServiceFactory service_factory) override;
+                  const base::Callback<void(mojo::ScopedMessagePipeHandle)>
+                      service_factory) override;
   void RemoveService(const std::string& service_name) override;
   void ConnectToRemoteService(const base::StringPiece& service_name,
                               mojo::ScopedMessagePipeHandle handle) override;
@@ -65,12 +58,11 @@ class CONTENT_EXPORT ServiceRegistryImpl
   mojo::Binding<mojo::ServiceProvider> binding_;
   mojo::ServiceProviderPtr remote_provider_;
 
-  std::map<std::string, ServiceFactory> service_factories_;
+  std::map<std::string, base::Callback<void(mojo::ScopedMessagePipeHandle)> >
+      service_factories_;
   std::queue<std::pair<std::string, mojo::MessagePipeHandle> >
       pending_connects_;
 
-  std::map<std::string, ServiceFactory> service_overrides_;
-
   base::WeakPtrFactory<ServiceRegistry> weak_factory_;
 };
 

content/renderer/mojo/service_registry_js_wrapper.cc

@@ -6,27 +6,12 @@
 
 #include <utility>
 
-#include "base/memory/scoped_ptr.h"
 #include "content/common/mojo/service_registry_impl.h"
 #include "content/public/common/service_registry.h"
 #include "third_party/mojo/src/mojo/edk/js/handle.h"
-#include "v8/include/v8.h"
 
 namespace content {
 
-namespace {
-
-void CallJsFactory(scoped_ptr<v8::Persistent<v8::Function>> factory,
-                   mojo::ScopedMessagePipeHandle pipe) {
-  v8::Isolate* isolate = v8::Isolate::GetCurrent();
-  v8::Local<v8::Value> argv[] = {
-    gin::ConvertToV8(isolate, mojo::Handle(pipe.release().value()))
-  };
-  factory->Get(isolate)->Call(v8::Undefined(isolate), 1, argv);
-}
-
-}  // namespace
-
 gin::WrapperInfo ServiceRegistryJsWrapper::kWrapperInfo = {
     gin::kEmbedderNativeGin};
 const char ServiceRegistryJsWrapper::kModuleName[] =
@@ -49,9 +34,7 @@ gin::ObjectTemplateBuilder ServiceRegistryJsWrapper::GetObjectTemplateBuilder(
     v8::Isolate* isolate) {
   return Wrappable<ServiceRegistryJsWrapper>::GetObjectTemplateBuilder(isolate).
       SetMethod("connectToService",
-                &ServiceRegistryJsWrapper::ConnectToService).
-      SetMethod("addServiceOverrideForTesting",
-                &ServiceRegistryJsWrapper::AddServiceOverrideForTesting);
+                &ServiceRegistryJsWrapper::ConnectToService);
 }
 
 mojo::Handle ServiceRegistryJsWrapper::ConnectToService(
@@ -63,20 +46,6 @@ mojo::Handle ServiceRegistryJsWrapper::ConnectToService(
   return pipe.handle1.release();
 }
 
-void ServiceRegistryJsWrapper::AddServiceOverrideForTesting(
-    const std::string& service_name,
-    v8::Local<v8::Function> service_factory) {
-  ServiceRegistryImpl* registry =
-      static_cast<ServiceRegistryImpl*>(service_registry_.get());
-  if (!registry)
-    return;
-  scoped_ptr<v8::Persistent<v8::Function>> factory(
-      new v8::Persistent<v8::Function>(v8::Isolate::GetCurrent(),
-                                       service_factory));
-  registry->AddServiceOverrideForTesting(
-      service_name, base::Bind(&CallJsFactory, base::Passed(&factory)));
-}
-
 ServiceRegistryJsWrapper::ServiceRegistryJsWrapper(
     base::WeakPtr<ServiceRegistry> service_registry)
     : service_registry_(service_registry) {

content/renderer/mojo/service_registry_js_wrapper.h

@@ -12,11 +12,7 @@
 #include "gin/handle.h"
 #include "gin/object_template_builder.h"
 #include "gin/wrappable.h"
-#include "mojo/public/cpp/system/handle.h"
-
-namespace v8 {
-class Isolate;
-}
+#include "mojo/public/cpp/system/core.h"
 
 namespace content {
 
@@ -37,8 +33,6 @@ class CONTENT_EXPORT ServiceRegistryJsWrapper
       v8::Isolate* isolate) override;
 
   // JS interface implementation.
-  void AddServiceOverrideForTesting(const std::string& service_name,
-                                    v8::Local<v8::Function> service_factory);
   mojo::Handle ConnectToService(const std::string& service_name);
 
   static gin::WrapperInfo kWrapperInfo;

third_party/WebKit/LayoutTests/harness-tests/mojo-helpers.html

@@ -13,59 +13,4 @@ mojo_test(mojo => {
   assert_true(mojo.router instanceof Object);
   assert_true(mojo.serviceRegistry instanceof Object);
 }, 'Mojo system APIs should be available to layout tests.');
-
-mojo_test(mojo => {
-  return new Promise(resolve => {
-    // Complete the test as soon as a request comes in for a Frobinator service.
-    mojo.serviceRegistry.addServiceOverrideForTesting('Frobinator', resolve);
-
-    // Try to connect to the browser's Frobinator service. This should be
-    // intercepted by the above override.
-    mojo.serviceRegistry.connectToService('Frobinator');
-  });
-}, 'Service registry overrides should be properly intercepted.');
-
-mojo_test(mojo => {
-  return new Promise(resolve => {
-    let TEST_REQUEST = new Uint8Array([42, 0, 2, 3, 5, 7, 11, 13, 17, 19, 23]);
-
-    mojo.serviceRegistry.addServiceOverrideForTesting('Frobinator', pipe => {
-      resolve(mojo_wait_for_incoming_message(mojo, pipe)
-        .then(message => {
-          assert_array_equals(new Uint8Array(message.buffer), TEST_REQUEST);
-          assert_array_equals(message.handles, []);
-        }));
-    });
-
-    let pipe = mojo.serviceRegistry.connectToService('Frobinator');
-    assert_equals(mojo.core.writeMessage(pipe, TEST_REQUEST, [], 0),
-        mojo.core.RESULT_OK);
-  });
-}, 'Mock services can receive messages from test code.');
-
-mojo_test(mojo => {
-  let TEST_REQUEST = new Uint8Array([1, 2, 3, 4, 5]);
-  let EXPECTED_RESPONSE = new Uint8Array([5, 4, 3, 2, 1]);
-
-  // Mock service should respond to any message with its reverse.
-  mojo.serviceRegistry.addServiceOverrideForTesting('Reversinator', pipe => {
-    mojo_wait_for_incoming_message(mojo, pipe)
-      .then(message => {
-        let response = new Uint8Array(message.buffer);
-        response.reverse();
-        assert_equals(mojo.core.writeMessage(pipe, response, [], 0),
-            mojo.core.RESULT_OK);
-      });
-  });
-
-  let pipe = mojo.serviceRegistry.connectToService('Reversinator');
-  assert_equals(mojo.core.writeMessage(pipe, TEST_REQUEST, [], 0),
-      mojo.core.RESULT_OK);
-
-  return mojo_wait_for_incoming_message(mojo, pipe)
-    .then(response => {
-      assert_array_equals(new Uint8Array(response.buffer), EXPECTED_RESPONSE);
-      assert_array_equals(response.handles, []);
-    });
-}, 'Test code can receive response messages from mock services.');
 </script>

third_party/WebKit/LayoutTests/resources/mojo-helpers.js

@@ -35,25 +35,3 @@ function mojo_test(func, name, properties) {
     });
   }, name, properties);
 }
-
-// Polls aggressively for a message to become available on a pipe.
-function mojo_wait_for_incoming_message(mojo, pipe) {
-  return new Promise((resolve, reject) => {
-    let wait = () => {
-      let result = mojo.core.readMessage(pipe, 0);
-      if (result.result === mojo.core.RESULT_SHOULD_WAIT) {
-        setTimeout(wait);
-        return;
-      }
-
-      if (result.result !== mojo.core.RESULT_OK) {
-        reject(result.result);
-        return;
-      }
-
-      resolve({ buffer: result.buffer, handles: result.handles });
-    };
-
-    wait();
-  });
-};