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