More work on ServiceProviderImpl.
* Remove the old AddService<I>().
* Rename AddServiceNew<I>() to AddService<I>().
* Add a getter for the ConnectionContext.
* Add a AddServiceForName() that takes a ServiceConnector.
* Add RemoveService<I>() and RemoveServiceForName().
The additions are basically required to replace the current
ServiceProvider implementation in stuff in ServiceRegistry with just a
ServiceProviderImpl.
R=vardhan@google.com
Review URL: https://codereview.chromium.org/1976063002 .
diff --git a/apps/moterm/moterm_view.cc b/apps/moterm/moterm_view.cc
index 413d486..e9f942f 100644
--- a/apps/moterm/moterm_view.cc
+++ b/apps/moterm/moterm_view.cc
@@ -58,7 +58,11 @@
// connection context argument.
service_provider_impl_.Bind(mojo::ConnectionContext(),
service_provider_request.Pass());
- service_provider_impl_.AddService<mojo::terminal::Terminal>(this);
+ service_provider_impl_.AddService<mojo::terminal::Terminal>([this](
+ const mojo::ConnectionContext& connection_context,
+ mojo::InterfaceRequest<mojo::terminal::Terminal> terminal_request) {
+ terminal_bindings_.AddBinding(this, terminal_request.Pass());
+ });
}
regular_typeface_ = skia::AdoptRef(SkTypeface::CreateFromStream(
@@ -133,12 +137,6 @@
}
}
-void MotermView::Create(
- const mojo::ConnectionContext& connection_context,
- mojo::InterfaceRequest<mojo::terminal::Terminal> request) {
- terminal_bindings_.AddBinding(this, request.Pass());
-}
-
void MotermView::Connect(
mojo::InterfaceRequest<mojo::files::File> terminal_file,
bool force,
diff --git a/apps/moterm/moterm_view.h b/apps/moterm/moterm_view.h
index 692670b..3595c4e 100644
--- a/apps/moterm/moterm_view.h
+++ b/apps/moterm/moterm_view.h
@@ -12,7 +12,6 @@
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "mojo/common/binding_set.h"
-#include "mojo/public/cpp/application/interface_factory.h"
#include "mojo/public/cpp/application/service_provider_impl.h"
#include "mojo/public/cpp/bindings/callback.h"
#include "mojo/public/cpp/bindings/interface_request.h"
@@ -30,7 +29,6 @@
public mojo::ui::InputListener,
public MotermModel::Delegate,
public MotermDriver::Client,
- public mojo::InterfaceFactory<mojo::terminal::Terminal>,
public mojo::terminal::Terminal {
public:
MotermView(
@@ -60,11 +58,6 @@
void OnClosed() override;
void OnDestroyed() override;
- // |mojo::InterfaceFactory<mojo::terminal::Terminal>|:
- void Create(
- const mojo::ConnectionContext& connection_context,
- mojo::InterfaceRequest<mojo::terminal::Terminal> request) override;
-
// |mojo::terminal::Terminal| implementation:
void Connect(mojo::InterfaceRequest<mojo::files::File> terminal_file,
bool force,
diff --git a/mojo/public/cpp/application/service_provider_impl.h b/mojo/public/cpp/application/service_provider_impl.h
index dd24472..54adac7 100644
--- a/mojo/public/cpp/application/service_provider_impl.h
+++ b/mojo/public/cpp/application/service_provider_impl.h
@@ -7,10 +7,9 @@
#include <functional>
#include <string>
+#include <utility>
-#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"
@@ -53,14 +52,12 @@
// 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_) {
+ // Adds a supported service with the given |name|, using the given
+ // |service_connector|.
+ void AddServiceForName(std::unique_ptr<ServiceConnector> service_connector,
+ const std::string& interface_name) {
service_connector_registry_.SetServiceConnectorForName(
- std::unique_ptr<ServiceConnector>(
- new internal::InterfaceFactoryConnector<Interface>(factory)),
- interface_name);
+ std::move(service_connector), interface_name);
}
// Adds a supported service with the given |name|, using the given
@@ -70,23 +67,34 @@
//
// A typical usage may be:
//
- // service_provider_impl_->AddServiceNew<Foobar>(
+ // service_provider_impl_->AddService<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(
+ void AddService(InterfaceRequestHandler<Interface> interface_request_handler,
+ const std::string& name = Interface::Name_) {
+ AddServiceForName(
std::unique_ptr<ServiceConnector>(new ServiceConnectorImpl<Interface>(
std::move(interface_request_handler))),
name);
}
+ // Removes support for the service with the given |name|.
+ void RemoveServiceForName(const std::string& name) {
+ service_connector_registry_.RemoveServiceConnectorForName(name);
+ }
+
+ // Like |RemoveServiceForName()| (above), but designed so that it can be used
+ // like |RemoveService<Interface>()| or even |RemoveService<Interface>(name)|
+ // (to parallel |AddService<Interface>()|).
+ template <typename Interface>
+ void RemoveService(const std::string& name = Interface::Name_) {
+ RemoveServiceForName(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
@@ -96,6 +104,12 @@
fallback_service_provider_ = fallback_service_provider;
}
+ // Gets the context for the connection that this object is bound to (if not
+ // bound, the context is just a default-initialized |ConnectionContext|).
+ const ConnectionContext& connection_context() const {
+ return connection_context_;
+ }
+
private:
// Objects of this class are used to adapt a generic (untyped) connection
// request (i.e., |ServiceConnector::ConnectToService()|) to the type-safe
diff --git a/mojo/public/cpp/application/tests/service_provider_impl_unittest.cc b/mojo/public/cpp/application/tests/service_provider_impl_unittest.cc
index a66f182..2483fc3 100644
--- a/mojo/public/cpp/application/tests/service_provider_impl_unittest.cc
+++ b/mojo/public/cpp/application/tests/service_provider_impl_unittest.cc
@@ -65,8 +65,11 @@
ServiceProviderImpl impl(ConnectionContext(ConnectionContext::Type::INCOMING,
kRemoteUrl, kConnectionUrl),
GetProxy(&sp));
+ EXPECT_EQ(ConnectionContext::Type::INCOMING, impl.connection_context().type);
+ EXPECT_EQ(kRemoteUrl, impl.connection_context().remote_url);
+ EXPECT_EQ(kConnectionUrl, impl.connection_context().connection_url);
- impl.AddServiceNew<test::PingService>(
+ impl.AddService<test::PingService>(
[&kRemoteUrl, &kConnectionUrl](
const ConnectionContext& connection_context,
InterfaceRequest<test::PingService> ping_service_request) {
@@ -77,7 +80,7 @@
},
kPing1);
- impl.AddServiceNew<test::PingService>(
+ impl.AddService<test::PingService>(
[&kRemoteUrl, &kConnectionUrl](
const ConnectionContext& connection_context,
InterfaceRequest<test::PingService> ping_service_request) {
@@ -116,6 +119,40 @@
}
loop().RunUntilIdle(); // Run stuff caused by destructors.
+ impl.RemoveService<test::PingService>(kPing2);
+
+ // "Ping2" should no longer work.
+ {
+ test::PingServicePtr ping2;
+ ConnectToService(sp.get(), GetProxy(&ping2), kPing2);
+ ping2.set_connection_error_handler([this] { QuitLoop(true); });
+ ping2->Ping([this] { QuitLoop(false); });
+ loop().Run();
+ }
+ loop().RunUntilIdle(); // Run stuff caused by destructors.
+
+ // But "Ping1" should still work.
+ {
+ 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.
+
+ impl.RemoveServiceForName(kPing1);
+
+ // "Ping1" should no longer work.
+ {
+ test::PingServicePtr ping1;
+ ConnectToService(sp.get(), GetProxy(&ping1), kPing1);
+ ping1.set_connection_error_handler([this] { QuitLoop(true); });
+ ping1->Ping([this] { QuitLoop(false); });
+ loop().Run();
+ }
+ loop().RunUntilIdle(); // Run stuff caused by destructors.
+
sp.reset();
loop().RunUntilIdle();
}
@@ -130,8 +167,11 @@
ServiceProviderImpl impl(ConnectionContext(ConnectionContext::Type::INCOMING,
kRemoteUrl1, kConnectionUrl),
GetProxy(&sp1));
+ EXPECT_EQ(ConnectionContext::Type::INCOMING, impl.connection_context().type);
+ EXPECT_EQ(kRemoteUrl1, impl.connection_context().remote_url);
+ EXPECT_EQ(kConnectionUrl, impl.connection_context().connection_url);
- impl.AddServiceNew<test::PingService>(
+ impl.AddService<test::PingService>(
[&kRemoteUrl1, &kRemoteUrl2, &kConnectionUrl](
const ConnectionContext& connection_context,
InterfaceRequest<test::PingService> ping_service_request) {
@@ -153,6 +193,9 @@
loop().RunUntilIdle(); // Run stuff caused by destructors.
impl.Close();
+ EXPECT_EQ(ConnectionContext::Type::UNKNOWN, impl.connection_context().type);
+ EXPECT_EQ(std::string(), impl.connection_context().remote_url);
+ EXPECT_EQ(std::string(), impl.connection_context().connection_url);
sp1.reset();
loop().RunUntilIdle();
@@ -160,6 +203,9 @@
impl.Bind(ConnectionContext(ConnectionContext::Type::INCOMING, kRemoteUrl2,
kConnectionUrl),
GetProxy(&sp2));
+ EXPECT_EQ(ConnectionContext::Type::INCOMING, impl.connection_context().type);
+ EXPECT_EQ(kRemoteUrl2, impl.connection_context().remote_url);
+ EXPECT_EQ(kConnectionUrl, impl.connection_context().connection_url);
{
test::PingServicePtr ping;
@@ -184,12 +230,15 @@
ServiceProviderPtr sp;
ServiceProviderImpl impl;
+ EXPECT_EQ(ConnectionContext::Type::UNKNOWN, impl.connection_context().type);
+ EXPECT_EQ(std::string(), impl.connection_context().remote_url);
+ EXPECT_EQ(std::string(), impl.connection_context().connection_url);
impl.Bind(ConnectionContext(ConnectionContext::Type::INCOMING, kRemoteUrl,
kConnectionUrl),
GetProxy(&sp));
- impl.AddServiceNew<test::PingService>(
+ impl.AddService<test::PingService>(
[&kRemoteUrl, &kConnectionUrl](
const ConnectionContext& connection_context,
InterfaceRequest<test::PingService> request) {
@@ -289,5 +338,7 @@
EXPECT_FALSE(was_run);
}
+// TODO(vtl): Explicitly test |AddServiceForName()|?
+
} // namespace
} // namespace mojo
