Add a new way of using ServiceProviderImpl without InterfaceFactory.
Instead, just take a factory function. This has a number of advantages:
* Classes don't have to implement InterfaceFactory<I> in order to
"provide" services.
* Classes can (easily) instantiate impls in different ways (or
instantiate different impls for the same service).
For now, the new method is called |AddServiceNew<I>()|, but I plan to
get rid of |AddService<I>()| and rename |AddServiceNew| to |AddService|.
Also, add a bunch of tests.
R=vardhan@google.com
Review URL: https://codereview.chromium.org/1973653002 .
diff --git a/mojo/public/cpp/application/application_connection.h b/mojo/public/cpp/application/application_connection.h
index 3507974..8979d0c 100644
--- a/mojo/public/cpp/application/application_connection.h
+++ b/mojo/public/cpp/application/application_connection.h
@@ -12,6 +12,7 @@
namespace mojo {
+struct ConnectionContext;
class ServiceConnector;
// Represents a connection to another application. An instance of this class is
@@ -34,6 +35,8 @@
// connection->AddService<Bar>(&my_foo_and_bar_factory_);
//
// The InterfaceFactory must outlive the ApplicationConnection.
+//
+// TODO(vtl): Don't get too attached to this class. I'm going to remove it.
class ApplicationConnection {
public:
virtual ~ApplicationConnection();
@@ -47,6 +50,8 @@
Interface::Name_);
}
+ virtual const ConnectionContext& GetConnectionContext() const = 0;
+
// Returns the URL that was used by the source application to establish a
// connection to the destination application.
//
diff --git a/mojo/public/cpp/application/lib/service_provider_impl.cc b/mojo/public/cpp/application/lib/service_provider_impl.cc
index 5e5da65..e7978b6 100644
--- a/mojo/public/cpp/application/lib/service_provider_impl.cc
+++ b/mojo/public/cpp/application/lib/service_provider_impl.cc
@@ -14,15 +14,15 @@
}
ServiceProviderImpl::ServiceProviderImpl(
- InterfaceRequest<ServiceProvider> request)
- : binding_(this, request.Pass()), fallback_service_provider_(nullptr) {
-}
+ InterfaceRequest<ServiceProvider> service_provider_request)
+ : binding_(this, service_provider_request.Pass()),
+ fallback_service_provider_(nullptr) {}
-ServiceProviderImpl::~ServiceProviderImpl() {
-}
+ServiceProviderImpl::~ServiceProviderImpl() {}
-void ServiceProviderImpl::Bind(InterfaceRequest<ServiceProvider> request) {
- binding_.Bind(request.Pass());
+void ServiceProviderImpl::Bind(
+ InterfaceRequest<ServiceProvider> service_provider_request) {
+ binding_.Bind(service_provider_request.Pass());
}
void ServiceProviderImpl::Close() {
diff --git a/mojo/public/cpp/application/lib/service_registry.cc b/mojo/public/cpp/application/lib/service_registry.cc
index 6be0e10..3c34628 100644
--- a/mojo/public/cpp/application/lib/service_registry.cc
+++ b/mojo/public/cpp/application/lib/service_registry.cc
@@ -34,6 +34,10 @@
service_connector_registry_.RemoveServiceConnectorForName(interface_name);
}
+const ConnectionContext& ServiceRegistry::GetConnectionContext() const {
+ return connection_context_;
+}
+
const std::string& ServiceRegistry::GetConnectionURL() {
return connection_context_.connection_url;
}
diff --git a/mojo/public/cpp/application/lib/service_registry.h b/mojo/public/cpp/application/lib/service_registry.h
index 50e5a43..1389d37 100644
--- a/mojo/public/cpp/application/lib/service_registry.h
+++ b/mojo/public/cpp/application/lib/service_registry.h
@@ -29,6 +29,7 @@
// ApplicationConnection overrides.
void SetServiceConnectorForName(ServiceConnector* service_connector,
const std::string& interface_name) override;
+ const ConnectionContext& GetConnectionContext() const override;
const std::string& GetConnectionURL() override;
const std::string& GetRemoteApplicationURL() override;
diff --git a/mojo/public/cpp/application/service_provider_impl.h b/mojo/public/cpp/application/service_provider_impl.h
index 1afa14b..b885924 100644
--- a/mojo/public/cpp/application/service_provider_impl.h
+++ b/mojo/public/cpp/application/service_provider_impl.h
@@ -5,27 +5,57 @@
#ifndef MOJO_PUBLIC_APPLICATION_SERVICE_PROVIDER_IMPL_H_
#define MOJO_PUBLIC_APPLICATION_SERVICE_PROVIDER_IMPL_H_
+#include <functional>
#include <string>
+#include "mojo/public/cpp/application/application_connection.h"
+#include "mojo/public/cpp/application/connection_context.h"
#include "mojo/public/cpp/application/lib/interface_factory_connector.h"
#include "mojo/public/cpp/application/lib/service_connector_registry.h"
+#include "mojo/public/cpp/application/service_connector.h"
#include "mojo/public/cpp/bindings/binding.h"
#include "mojo/public/interfaces/application/service_provider.mojom.h"
namespace mojo {
-// Implements a registry that can be used to expose services to another app.
+// An implementation of |ServiceProvider|, which can be customized appropriately
+// (to select what services it provides).
class ServiceProviderImpl : public ServiceProvider {
public:
+ // A |InterfaceRequestHandler<Interface>| is simply a function that handles an
+ // interface request for |Interface|. If it determines (e.g., based on the
+ // given |ConnectionContext|) that the request should be "accepted", then it
+ // should "connect" ("take ownership of") request. Otherwise, it can simply
+ // drop |interface_request| (as implied by the interface).
+ template <typename Interface>
+ using InterfaceRequestHandler =
+ std::function<void(const ConnectionContext& connection_context,
+ InterfaceRequest<Interface> interface_request)>;
+
+ // Constructs this service provider implementation in an unbound state.
ServiceProviderImpl();
- explicit ServiceProviderImpl(InterfaceRequest<ServiceProvider> request);
+
+ // Constructs this service provider implementation, binding it to the given
+ // interface request.
+ // TODO(vtl): This should take a |ConnectionContext|, to provide
+ // |InterfaceRequestHandler<I>|s.
+ explicit ServiceProviderImpl(
+ InterfaceRequest<ServiceProvider> service_provider_request);
+
~ServiceProviderImpl() override;
- void Bind(InterfaceRequest<ServiceProvider> request);
- // Disconnect this service provider and put it in a state where it can be
- // rebound to a new request.
+ // Binds this service provider implementation to the given interface request.
+ // This may only be called if this object is unbound.
+ // TODO(vtl): This should take a |ConnectionContext|, to provide
+ // |InterfaceRequestHandler<I>|s.
+ void Bind(InterfaceRequest<ServiceProvider> service_provider_request);
+
+ // Disconnect this service provider implementation and put it in a state where
+ // it can be rebound to a new request (i.e., restores this object to an
+ // unbound state). This may be called even if this object is already unbound.
void Close();
+ // TODO(vtl): Remove this.
template <typename Interface>
void AddService(InterfaceFactory<Interface>* factory,
const std::string& interface_name = Interface::Name_) {
@@ -35,18 +65,70 @@
interface_name);
}
- // ServiceProviderImpl uses the fallback_service_provider_ whenever someone
- // asks a service that doesn't exist in the service_connector_registry_.
+ // Adds a supported service with the given |name|, using the given
+ // |interface_request_handler| (see above for information about
+ // |InterfaceRequestHandler<Interface>|). |interface_request_handler| should
+ // remain valid for the lifetime of this object.
//
- // Note: ServiceProviderImpl does not take ownership of |fallback|. The caller
- // must ensure that |fallback| outlives the ServiceProviderImpl.
+ // A typical usage may be:
//
- void set_fallback_service_provider(ServiceProvider* fallback) {
- fallback_service_provider_ = fallback;
+ // service_provider_impl_->AddServiceNew<Foobar>(
+ // [](const ConnectionContext& connection_context,
+ // InterfaceRequest<FooBar> foobar_request) {
+ // // |FoobarImpl| owns itself.
+ // new FoobarImpl(std::move(foobar_request));
+ // });
+ // TODO(vtl): Remove the AddService() above and rename this to AddService().
+ template <typename Interface>
+ void AddServiceNew(
+ InterfaceRequestHandler<Interface> interface_request_handler,
+ const std::string& name = Interface::Name_) {
+ service_connector_registry_.SetServiceConnectorForName(
+ std::unique_ptr<ServiceConnector>(new ServiceConnectorImpl<Interface>(
+ std::move(interface_request_handler))),
+ name);
+ }
+
+ // This uses the provided |fallback_service_provider| for connection requests
+ // for services that are not known (haven't been added). (Set it to null to
+ // not have any fallback.) A fallback must outlive this object (or until it is
+ // "cleared" or replaced by a different fallback.
+ void set_fallback_service_provider(
+ ServiceProvider* fallback_service_provider) {
+ fallback_service_provider_ = fallback_service_provider;
}
private:
- // Overridden from ServiceProvider:
+ // Objects of this class are used to adapt a generic (untyped) connection
+ // request (i.e., |ServiceConnector::ConnectToService()|) to the type-safe
+ // |InterfaceRequestHandler<Interface>|.
+ template <typename Interface>
+ class ServiceConnectorImpl : public ServiceConnector {
+ public:
+ explicit ServiceConnectorImpl(
+ InterfaceRequestHandler<Interface> interface_request_handler)
+ : interface_request_handler_(std::move(interface_request_handler)) {}
+ ~ServiceConnectorImpl() override {}
+
+ void ConnectToService(ApplicationConnection* application_connection,
+ const std::string& interface_name,
+ ScopedMessagePipeHandle client_handle) override {
+ // TODO(vtl): This should be given a |const ConnectionContext&|, instead
+ // of an |ApplicationConnection*| -- which may be null!
+ interface_request_handler_(
+ application_connection
+ ? application_connection->GetConnectionContext()
+ : ConnectionContext(),
+ InterfaceRequest<Interface>(client_handle.Pass()));
+ }
+
+ private:
+ const InterfaceRequestHandler<Interface> interface_request_handler_;
+
+ MOJO_DISALLOW_COPY_AND_ASSIGN(ServiceConnectorImpl);
+ };
+
+ // Overridden from |ServiceProvider|:
void ConnectToService(const String& service_name,
ScopedMessagePipeHandle client_handle) override;
diff --git a/mojo/public/cpp/application/tests/BUILD.gn b/mojo/public/cpp/application/tests/BUILD.gn
index 7766a7d..284f54d 100644
--- a/mojo/public/cpp/application/tests/BUILD.gn
+++ b/mojo/public/cpp/application/tests/BUILD.gn
@@ -8,6 +8,7 @@
testonly = true
sources = [
+ "service_provider_impl_unittest.cc",
"service_registry_unittest.cc",
]
@@ -18,6 +19,9 @@
mojo_sdk_deps = [
"mojo/public/cpp/application:standalone",
"mojo/public/cpp/environment:standalone",
+ "mojo/public/cpp/system",
"mojo/public/cpp/utility",
+ "mojo/public/interfaces/application",
+ "mojo/public/interfaces/bindings/tests:test_interfaces",
]
}
diff --git a/mojo/public/cpp/application/tests/service_provider_impl_unittest.cc b/mojo/public/cpp/application/tests/service_provider_impl_unittest.cc
new file mode 100644
index 0000000..e0a4f12
--- /dev/null
+++ b/mojo/public/cpp/application/tests/service_provider_impl_unittest.cc
@@ -0,0 +1,257 @@
+// Copyright 2016 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 "mojo/public/cpp/application/service_provider_impl.h"
+
+#include <utility>
+
+#include "mojo/public/cpp/application/connect.h"
+#include "mojo/public/cpp/bindings/strong_binding.h"
+#include "mojo/public/cpp/environment/environment.h"
+#include "mojo/public/cpp/system/macros.h"
+#include "mojo/public/cpp/utility/run_loop.h"
+#include "mojo/public/interfaces/application/service_provider.mojom.h"
+#include "mojo/public/interfaces/bindings/tests/ping_service.mojom.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace mojo {
+namespace {
+
+class ServiceProviderImplTest : public testing::Test {
+ public:
+ ServiceProviderImplTest() {}
+ ~ServiceProviderImplTest() override { loop_.RunUntilIdle(); }
+
+ RunLoop& loop() { return loop_; }
+
+ protected:
+ void QuitLoop(bool ok) {
+ EXPECT_TRUE(ok);
+ loop_.Quit();
+ }
+
+ private:
+ Environment env_;
+ RunLoop loop_;
+
+ MOJO_DISALLOW_COPY_AND_ASSIGN(ServiceProviderImplTest);
+};
+
+class PingServiceImpl : public test::PingService {
+ public:
+ PingServiceImpl(InterfaceRequest<test::PingService> ping_service_request)
+ : strong_binding_(this, std::move(ping_service_request)) {}
+ ~PingServiceImpl() override {}
+
+ // |test::PingService|:
+ void Ping(const PingCallback& callback) override { callback.Run(); }
+
+ private:
+ StrongBinding<test::PingService> strong_binding_;
+
+ MOJO_DISALLOW_COPY_AND_ASSIGN(PingServiceImpl);
+};
+
+TEST_F(ServiceProviderImplTest, Basic) {
+ const char kPing1[] = "Ping1";
+ const char kPing2[] = "Ping2";
+ const char kPing3[] = "Ping3";
+
+ ServiceProviderPtr sp;
+ ServiceProviderImpl impl(GetProxy(&sp));
+
+ impl.AddServiceNew<test::PingService>(
+ [](const ConnectionContext& connection_context,
+ InterfaceRequest<test::PingService> ping_service_request) {
+ new PingServiceImpl(std::move(ping_service_request));
+ },
+ kPing1);
+
+ impl.AddServiceNew<test::PingService>(
+ [](const ConnectionContext& connection_context,
+ InterfaceRequest<test::PingService> ping_service_request) {
+ new PingServiceImpl(std::move(ping_service_request));
+ },
+ kPing2);
+
+ {
+ test::PingServicePtr ping1;
+ ConnectToService(sp.get(), GetProxy(&ping1), kPing1);
+ ping1.set_connection_error_handler([this] { QuitLoop(false); });
+ ping1->Ping([this] { QuitLoop(true); });
+ loop().Run();
+ }
+ loop().RunUntilIdle(); // Run stuff caused by destructors.
+
+ {
+ test::PingServicePtr ping2;
+ ConnectToService(sp.get(), GetProxy(&ping2), kPing2);
+ ping2.set_connection_error_handler([this] { QuitLoop(false); });
+ ping2->Ping([this] { QuitLoop(true); });
+ loop().Run();
+ }
+ loop().RunUntilIdle(); // Run stuff caused by destructors.
+
+ // "Ping3" isn't actually registered!
+ {
+ test::PingServicePtr ping3;
+ ConnectToService(sp.get(), GetProxy(&ping3), kPing3);
+ ping3.set_connection_error_handler([this] { QuitLoop(true); });
+ ping3->Ping([this] { QuitLoop(false); });
+ loop().Run();
+ }
+ loop().RunUntilIdle(); // Run stuff caused by destructors.
+
+ sp.reset();
+ loop().RunUntilIdle();
+}
+
+TEST_F(ServiceProviderImplTest, CloseAndRebind) {
+ const char kPing[] = "Ping";
+
+ ServiceProviderPtr sp1;
+ ServiceProviderImpl impl(GetProxy(&sp1));
+
+ impl.AddServiceNew<test::PingService>(
+ [](const ConnectionContext& connection_context,
+ InterfaceRequest<test::PingService> ping_service_request) {
+ new PingServiceImpl(std::move(ping_service_request));
+ },
+ kPing);
+
+ {
+ test::PingServicePtr ping;
+ ConnectToService(sp1.get(), GetProxy(&ping), kPing);
+ ping.set_connection_error_handler([this] { QuitLoop(false); });
+ ping->Ping([this] { QuitLoop(true); });
+ loop().Run();
+ }
+ loop().RunUntilIdle(); // Run stuff caused by destructors.
+
+ impl.Close();
+ sp1.reset();
+ loop().RunUntilIdle();
+
+ ServiceProviderPtr sp2;
+ impl.Bind(GetProxy(&sp2));
+
+ {
+ test::PingServicePtr ping;
+ ConnectToService(sp2.get(), GetProxy(&ping), kPing);
+ ping.set_connection_error_handler([this] { QuitLoop(false); });
+ ping->Ping([this] { QuitLoop(true); });
+ loop().Run();
+ }
+ loop().RunUntilIdle(); // Run stuff caused by destructors.
+
+ // Can close multiple times.
+ impl.Close();
+ impl.Close();
+ sp2.reset();
+ loop().RunUntilIdle();
+}
+
+TEST_F(ServiceProviderImplTest, Bind) {
+ const char kPing[] = "Ping";
+
+ ServiceProviderPtr sp;
+ ServiceProviderImpl impl;
+
+ impl.Bind(GetProxy(&sp));
+
+ impl.AddServiceNew<test::PingService>(
+ [](const ConnectionContext& connection_context,
+ InterfaceRequest<test::PingService> request) {
+ new PingServiceImpl(std::move(request));
+ },
+ kPing);
+
+ {
+ test::PingServicePtr ping;
+ ConnectToService(sp.get(), GetProxy(&ping), kPing);
+ ping.set_connection_error_handler([this] { QuitLoop(false); });
+ ping->Ping([this] { QuitLoop(true); });
+ loop().Run();
+ }
+ loop().RunUntilIdle(); // Run stuff caused by destructors.
+
+ sp.reset();
+ loop().RunUntilIdle();
+}
+
+class FauxServiceProvider : public ServiceProvider {
+ public:
+ explicit FauxServiceProvider(
+ std::function<void(const std::string& service_name)>
+ on_connect_to_service)
+ : on_connect_to_service_(on_connect_to_service) {}
+ ~FauxServiceProvider() override {}
+
+ // |ServiceProvider|:
+ void ConnectToService(const String& service_name,
+ ScopedMessagePipeHandle client_handle) override {
+ on_connect_to_service_(service_name.get());
+ }
+
+ private:
+ std::function<void(const std::string& service_name)> on_connect_to_service_;
+
+ MOJO_DISALLOW_COPY_AND_ASSIGN(FauxServiceProvider);
+};
+
+TEST_F(ServiceProviderImplTest, FallbackServiceProvider) {
+ const char kWhatever[] = "Whatever";
+
+ ServiceProviderPtr sp;
+ ServiceProviderImpl impl(GetProxy(&sp));
+
+ {
+ test::PingServicePtr ping;
+ ConnectToService(sp.get(), GetProxy(&ping), kWhatever);
+ ping.set_connection_error_handler([this] { QuitLoop(true); });
+ ping->Ping([this] { QuitLoop(false); });
+ loop().Run();
+ }
+ loop().RunUntilIdle(); // Run stuff caused by destructors.
+
+ bool was_run = false;
+ FauxServiceProvider fallback_sp(
+ [this, &kWhatever, &was_run](const std::string& service_name) {
+ EXPECT_EQ(kWhatever, service_name);
+ was_run = true;
+ });
+ impl.set_fallback_service_provider(&fallback_sp);
+
+ {
+ test::PingServicePtr ping;
+ ConnectToService(sp.get(), GetProxy(&ping), kWhatever);
+ ping.set_connection_error_handler([this] { QuitLoop(true); });
+ EXPECT_FALSE(was_run);
+ ping->Ping([this] { QuitLoop(false); });
+ loop().Run();
+ EXPECT_TRUE(was_run);
+ }
+ loop().RunUntilIdle(); // Run stuff caused by destructors.
+
+ // Clear the fallback.
+ impl.set_fallback_service_provider(nullptr);
+ was_run = false;
+
+ {
+ test::PingServicePtr ping;
+ ConnectToService(sp.get(), GetProxy(&ping), kWhatever);
+ ping.set_connection_error_handler([this] { QuitLoop(true); });
+ ping->Ping([this] { QuitLoop(false); });
+ loop().Run();
+ }
+ loop().RunUntilIdle(); // Run stuff caused by destructors.
+
+ sp.reset();
+ loop().RunUntilIdle();
+
+ EXPECT_FALSE(was_run);
+}
+
+} // namespace
+} // namespace mojo
diff --git a/services/log/log_impl_unittest.cc b/services/log/log_impl_unittest.cc
index 2511972..8500ce1 100644
--- a/services/log/log_impl_unittest.cc
+++ b/services/log/log_impl_unittest.cc
@@ -11,6 +11,7 @@
#include "mojo/public/c/environment/logger.h"
#include "mojo/public/cpp/application/application_impl.h"
#include "mojo/public/cpp/application/application_test_base.h"
+#include "mojo/public/cpp/application/connection_context.h"
#include "mojo/public/cpp/system/time.h"
#include "mojo/services/log/interfaces/entry.mojom.h"
#include "mojo/services/log/interfaces/log.mojom.h"
@@ -27,18 +28,28 @@
// We need to supply a ApplicationConnection to LogImpl::Create().
class TestApplicationConnection : public ApplicationConnection {
public:
+ TestApplicationConnection()
+ : connection_context_(ConnectionContext::Type::INCOMING,
+ "mojo:log_impl_unittest",
+ "mojo:log") {}
+
+ const ConnectionContext& GetConnectionContext() const override {
+ return connection_context_;
+ }
+
const std::string& GetConnectionURL() override {
- static std::string kConnectionURL = "mojo:log";
- return kConnectionURL;
+ return connection_context_.connection_url;
}
const std::string& GetRemoteApplicationURL() override {
- static std::string kRemoteApplicationURL = "mojo:log_impl_unittest";
- return kRemoteApplicationURL;
+ return connection_context_.remote_url;
}
void SetServiceConnectorForName(ServiceConnector* service_connector,
const std::string& name) override {}
+
+ private:
+ const ConnectionContext connection_context_;
};
// Tests the Log service implementation by calling its AddEntry and verifying
