Remove [Client=] annotation from ServiceProvider
This removes the symmetrical nature of ServiceProvider and consistently
passes and uses a ServiceProvider + ServiceProvider& pair in places that
wish to bidirectionally expose services, such as the view manager.
The
view manager library now deals with InterfaceRequest<ServiceProvider>
and ServiceProviderPtr objects (i.e. c++ wrappers for handles) instead
of a concrete implementation of ServiceProvider to make it easier for
callers.
A number of places that were assuming a particular
ServiceProvider would always exist are updated to reflect the nullability
of the parameters in mojom and places that do not wish to ever look up
or provide services now pass nullptr instead of doomed pipe handles.
The JS application startup classes are reworked a bit to accomodate
exposing services on the third ConnectToApplication/AcceptConnection
parameter.
BUG=449432
R=abarth@chromium.org, sky@chromium.org
Review URL: https://codereview.chromium.org/858103002
diff --git a/examples/browser/browser.cc b/examples/browser/browser.cc
index 194614d..94c2ef5 100644
--- a/examples/browser/browser.cc
+++ b/examples/browser/browser.cc
@@ -36,7 +36,9 @@
public examples::BrowserHost,
public mojo::InterfaceFactory<examples::BrowserHost> {
public:
- Browser() : shell_(nullptr), root_(NULL), binding_(this) {}
+ Browser() : shell_(nullptr), root_(NULL), binding_(this) {
+ browser_host_services_impl_.AddService(this);
+ }
virtual ~Browser() {
if (root_)
@@ -63,10 +65,10 @@
// ViewManagerDelegate:
void OnEmbed(View* root,
- ServiceProviderImpl* exported_services,
- scoped_ptr<ServiceProvider> imported_services) override {
+ InterfaceRequest<ServiceProvider> services,
+ ServiceProviderPtr exposed_services) override {
// TODO: deal with OnEmbed() being invoked multiple times.
- ConnectToService(imported_services.get(), &navigator_host_);
+ ConnectToService(exposed_services.get(), &navigator_host_);
root_ = root;
root_->AddObserver(this);
root_->SetFocus();
@@ -83,13 +85,11 @@
view->SetVisible(true);
root->SetVisible(true);
- scoped_ptr<mojo::ServiceProviderImpl> browser_host_services(
- new mojo::ServiceProviderImpl());
- browser_host_services->AddService(this);
+ ServiceProviderPtr browser_host_services;
+ browser_host_services_impl_.Bind(GetProxy(&browser_host_services));
GURL frame_url = url_.Resolve("/examples/browser/browser.sky");
- browser_input_ =
- view->Embed(frame_url.spec(), browser_host_services.Pass());
+ view->Embed(frame_url.spec(), nullptr, browser_host_services.Pass());
}
void OnViewManagerDisconnected(ViewManager* view_manager) override {
@@ -118,12 +118,11 @@
Shell* shell_;
- scoped_ptr<mojo::ServiceProvider> browser_input_;
-
scoped_ptr<ViewManagerClientFactory> view_manager_client_factory_;
View* root_;
NavigatorHostPtr navigator_host_;
IWindowManagerPtr window_manager_;
+ ServiceProviderImpl browser_host_services_impl_;
GURL url_;
diff --git a/examples/embedded_app/embedded_app.cc b/examples/embedded_app/embedded_app.cc
index e08e9fa..4e07c7b 100644
--- a/examples/embedded_app/embedded_app.cc
+++ b/examples/embedded_app/embedded_app.cc
@@ -33,9 +33,7 @@
SK_ColorMAGENTA};
struct Window {
- Window(View* root,
- scoped_ptr<ServiceProvider> embedder_service_provider,
- Shell* shell)
+ Window(View* root, ServiceProviderPtr embedder_service_provider, Shell* shell)
: root(root),
embedder_service_provider(embedder_service_provider.Pass()),
bitmap_uploader(root) {
@@ -43,7 +41,7 @@
}
View* root;
- scoped_ptr<ServiceProvider> embedder_service_provider;
+ ServiceProviderPtr embedder_service_provider;
BitmapUploader bitmap_uploader;
};
@@ -72,10 +70,10 @@
// Overridden from ViewManagerDelegate:
virtual void OnEmbed(View* root,
- ServiceProviderImpl* exported_services,
- scoped_ptr<ServiceProvider> imported_services) override {
+ InterfaceRequest<ServiceProvider> services,
+ ServiceProviderPtr exposed_services) override {
root->AddObserver(this);
- Window* window = new Window(root, imported_services.Pass(), shell_);
+ Window* window = new Window(root, exposed_services.Pass(), shell_);
windows_[root->id()] = window;
window->bitmap_uploader.SetColor(
kColors[next_color_++ % arraysize(kColors)]);
diff --git a/examples/ganesh_app/ganesh_app.cc b/examples/ganesh_app/ganesh_app.cc
index 215a0b2..0896fb4 100644
--- a/examples/ganesh_app/ganesh_app.cc
+++ b/examples/ganesh_app/ganesh_app.cc
@@ -34,8 +34,8 @@
}
void OnEmbed(mojo::View* root,
- mojo::ServiceProviderImpl* exported_services,
- scoped_ptr<mojo::ServiceProvider> imported_services) override {
+ mojo::InterfaceRequest<mojo::ServiceProvider> services,
+ mojo::ServiceProviderPtr exposed_services) override {
new GaneshView(shell_, root);
}
diff --git a/examples/nesting_app/nesting_app.cc b/examples/nesting_app/nesting_app.cc
index 6a22f37..88cdded 100644
--- a/examples/nesting_app/nesting_app.cc
+++ b/examples/nesting_app/nesting_app.cc
@@ -60,8 +60,8 @@
// Overridden from ViewManagerDelegate:
virtual void OnEmbed(View* root,
- ServiceProviderImpl* exported_services,
- scoped_ptr<ServiceProvider> imported_services) override {
+ InterfaceRequest<ServiceProvider> services,
+ ServiceProviderPtr exposed_services) override {
root->AddObserver(this);
bitmap_uploader_.reset(new BitmapUploader(root));
bitmap_uploader_->Init(shell_);
diff --git a/examples/pdf_viewer/pdf_viewer.cc b/examples/pdf_viewer/pdf_viewer.cc
index 4c3f15c..f0944f8 100644
--- a/examples/pdf_viewer/pdf_viewer.cc
+++ b/examples/pdf_viewer/pdf_viewer.cc
@@ -83,8 +83,8 @@
// Overridden from ViewManagerDelegate:
virtual void OnEmbed(View* root,
- ServiceProviderImpl* exported_services,
- scoped_ptr<ServiceProvider> imported_services) override {
+ InterfaceRequest<ServiceProvider> services,
+ ServiceProviderPtr exposed_services) override {
DCHECK(embedder_for_roots_.find(root) == embedder_for_roots_.end());
root->AddObserver(this);
EmbedderData* embedder_data = new EmbedderData(app_->shell(), root);
diff --git a/examples/png_viewer/png_viewer.cc b/examples/png_viewer/png_viewer.cc
index 949cb11..5a51c50 100644
--- a/examples/png_viewer/png_viewer.cc
+++ b/examples/png_viewer/png_viewer.cc
@@ -91,8 +91,8 @@
// Overridden from ViewManagerDelegate:
virtual void OnEmbed(View* root,
- ServiceProviderImpl* exported_services,
- scoped_ptr<ServiceProvider> imported_services) override {
+ InterfaceRequest<ServiceProvider> services,
+ ServiceProviderPtr exposed_services) override {
// TODO(qsr): The same view should be embeddable on multiple views.
DCHECK(embedder_for_roots_.find(root) == embedder_for_roots_.end());
root->AddObserver(this);
diff --git a/examples/recipes/window_manager/main.cc b/examples/recipes/window_manager/main.cc
index 2d7f8e7..ec63f8b 100644
--- a/examples/recipes/window_manager/main.cc
+++ b/examples/recipes/window_manager/main.cc
@@ -35,8 +35,7 @@
window_manager_app_->Initialize(impl);
for (size_t i = 1; i < impl->args().size(); ++i) {
- mojo::InterfaceRequest<mojo::ServiceProvider> empty_request;
- window_manager_app_->Embed(impl->args()[i], empty_request.Pass());
+ window_manager_app_->Embed(impl->args()[i], nullptr, nullptr);
}
}
bool ConfigureIncomingConnection(
@@ -46,10 +45,9 @@
}
// Overridden from mojo::ViewManagerDelegate:
- void OnEmbed(
- mojo::View* root,
- mojo::ServiceProviderImpl* exported_services,
- scoped_ptr<mojo::ServiceProvider> remote_service_provider) override {
+ void OnEmbed(mojo::View* root,
+ mojo::InterfaceRequest<mojo::ServiceProvider> services,
+ mojo::ServiceProviderPtr exposed_services) override {
window_manager_.reset(new WindowManager(root));
window_manager_app_->InitFocus(
@@ -60,12 +58,12 @@
}
// Overridden from ::window_manager::WindowManagerDelegate:
- void Embed(
- const mojo::String& url,
- mojo::InterfaceRequest<mojo::ServiceProvider> service_provider) override {
+ void Embed(const mojo::String& url,
+ mojo::InterfaceRequest<mojo::ServiceProvider> services,
+ mojo::ServiceProviderPtr exposed_services) override {
DCHECK(window_manager_.get());
mojo::View* view = window_manager_->Create();
- view->Embed(url, service_provider.Pass());
+ view->Embed(url, services.Pass(), exposed_services.Pass());
}
scoped_ptr<::window_manager::WindowManagerApp> window_manager_app_;
diff --git a/examples/sky_compositor_app/sky_compositor_app.cc b/examples/sky_compositor_app/sky_compositor_app.cc
index 6ad298e..1a72095 100644
--- a/examples/sky_compositor_app/sky_compositor_app.cc
+++ b/examples/sky_compositor_app/sky_compositor_app.cc
@@ -50,8 +50,8 @@
}
void OnEmbed(mojo::View* root,
- mojo::ServiceProviderImpl* exported_services,
- scoped_ptr<mojo::ServiceProvider> imported_services) override {
+ mojo::InterfaceRequest<mojo::ServiceProvider> services,
+ mojo::ServiceProviderPtr exposed_services) override {
view_ = root;
base_time_ = base::TimeTicks::Now();
diff --git a/examples/window_manager/window_manager.cc b/examples/window_manager/window_manager.cc
index 09226ba..a2d53e4 100644
--- a/examples/window_manager/window_manager.cc
+++ b/examples/window_manager/window_manager.cc
@@ -152,17 +152,19 @@
class Window : public InterfaceFactory<NavigatorHost> {
public:
Window(WindowManager* window_manager, View* view)
- : window_manager_(window_manager), view_(view) {}
+ : window_manager_(window_manager), view_(view) {
+ exposed_services_impl_.AddService<NavigatorHost>(this);
+ }
virtual ~Window() {}
View* view() const { return view_; }
void Embed(const std::string& url) {
- scoped_ptr<ServiceProviderImpl> service_provider_impl(
- new ServiceProviderImpl());
- service_provider_impl->AddService<NavigatorHost>(this);
- view_->Embed(url, service_provider_impl.Pass());
+ // TODO: Support embedding multiple times?
+ ServiceProviderPtr exposed_services;
+ exposed_services_impl_.Bind(GetProxy(&exposed_services));
+ view_->Embed(url, nullptr, exposed_services.Pass());
}
private:
@@ -175,6 +177,7 @@
WindowManager* window_manager_;
View* view_;
+ ServiceProviderImpl exposed_services_impl_;
};
class WindowManager : public ApplicationDelegate,
@@ -266,8 +269,8 @@
// Overridden from ViewManagerDelegate:
virtual void OnEmbed(View* root,
- ServiceProviderImpl* exported_services,
- scoped_ptr<ServiceProvider> imported_services) override {
+ InterfaceRequest<ServiceProvider> services,
+ ServiceProviderPtr exposed_services) override {
DCHECK(!view_manager_);
view_manager_ = root->view_manager();
@@ -303,7 +306,8 @@
// Overridden from WindowManagerDelegate:
void Embed(const String& url,
- InterfaceRequest<ServiceProvider> service_provider) override {
+ InterfaceRequest<ServiceProvider> services,
+ ServiceProviderPtr exposed_services) override {
const Id kInvalidSourceViewId = 0;
OnLaunch(kInvalidSourceViewId, TARGET_DEFAULT, url);
}
@@ -399,12 +403,12 @@
view->SetBounds(bounds);
view->SetVisible(true);
- scoped_ptr<mojo::ServiceProviderImpl> exported_services(
- new mojo::ServiceProviderImpl());
- exported_services->AddService(this);
+ ServiceProviderPtr exposed_services;
+ control_panel_exposed_services_impl_.Bind(GetProxy(&exposed_services));
+ control_panel_exposed_services_impl_.AddService(this);
GURL frame_url = url_.Resolve("/examples/window_manager/debug_panel.sky");
- debug_panel_ = view->Embed(frame_url.spec(), exported_services.Pass());
+ view->Embed(frame_url.spec(), nullptr, exposed_services.Pass());
return view->id();
}
@@ -425,11 +429,11 @@
InterfaceFactoryImplWithContext<WindowManagerConnection, WindowManager>
window_manager_factory_;
- scoped_ptr<mojo::ServiceProvider> debug_panel_;
Window* launcher_ui_;
WindowVector windows_;
ViewManager* view_manager_;
scoped_ptr<RootLayoutManager> root_layout_manager_;
+ ServiceProviderImpl control_panel_exposed_services_impl_;
scoped_ptr<window_manager::WindowManagerApp> window_manager_app_;
diff --git a/examples/wm_flow/BUILD.gn b/examples/wm_flow/BUILD.gn
index 9853c58..b069b82 100644
--- a/examples/wm_flow/BUILD.gn
+++ b/examples/wm_flow/BUILD.gn
@@ -28,6 +28,7 @@
"//base",
"//mojo/application",
"//mojo/converters/geometry",
+ "//mojo/public/interfaces/application",
"//mojo/services/input_events/public/interfaces",
"//mojo/services/view_manager/public/cpp",
"//services/window_manager:lib",
@@ -50,6 +51,7 @@
"//base",
"//examples/bitmap_uploader",
"//mojo/application",
+ "//mojo/common",
"//mojo/public/interfaces/application:application",
"//mojo/services/view_manager/public/cpp",
"//services/window_manager:lib",
diff --git a/examples/wm_flow/app/app.cc b/examples/wm_flow/app/app.cc
index 6e4c243..7a1171e 100644
--- a/examples/wm_flow/app/app.cc
+++ b/examples/wm_flow/app/app.cc
@@ -11,6 +11,7 @@
#include "examples/wm_flow/app/embedder.mojom.h"
#include "examples/wm_flow/embedded/embeddee.mojom.h"
#include "mojo/application/application_runner_chromium.h"
+#include "mojo/common/weak_binding_set.h"
#include "mojo/public/c/system/main.h"
#include "mojo/public/cpp/application/application_connection.h"
#include "mojo/public/cpp/application/application_delegate.h"
@@ -47,6 +48,30 @@
DISALLOW_COPY_AND_ASSIGN(EmbedderImpl);
};
+class EmbedderImplProvider : public mojo::ServiceProvider {
+ public:
+ EmbedderImplProvider() {}
+ ~EmbedderImplProvider() override {}
+
+ void AddBinding(mojo::InterfaceRequest<mojo::ServiceProvider> request) {
+ embeddee_exposed_services_.AddBinding(this, request.Pass());
+ }
+
+ private:
+ // mojo::ServiceProvider implementation
+ void ConnectToService(const mojo::String& interface_name,
+ mojo::ScopedMessagePipeHandle pipe_handle) override {
+ if (interface_name != Embedder::Name_)
+ return;
+ auto request = mojo::MakeRequest<Embedder>(pipe_handle.Pass());
+ mojo::BindToRequest(new EmbedderImpl, &request);
+ }
+
+ mojo::WeakBindingSet<mojo::ServiceProvider> embeddee_exposed_services_;
+
+ DISALLOW_COPY_AND_ASSIGN(EmbedderImplProvider);
+};
+
} // namespace
// This app starts its life via Connect() rather than by being embed, so it does
@@ -83,10 +108,9 @@
}
// Overridden from mojo::ViewManagerDelegate:
- virtual void OnEmbed(
- mojo::View* root,
- mojo::ServiceProviderImpl* exported_services,
- scoped_ptr<mojo::ServiceProvider> imported_services) override {
+ virtual void OnEmbed(mojo::View* root,
+ mojo::InterfaceRequest<mojo::ServiceProvider> services,
+ mojo::ServiceProviderPtr exposed_services) override {
root->AddObserver(this);
mojo::BitmapUploader* uploader = new mojo::BitmapUploader(root);
uploaders_[root] = uploader;
@@ -106,14 +130,13 @@
embed->SetBounds(bounds);
embed->SetVisible(true);
- scoped_ptr<mojo::ServiceProviderImpl> registry(
- new mojo::ServiceProviderImpl);
- // Expose some services to the embeddee...
- registry->AddService(&embedder_factory_);
+ mojo::ServiceProviderPtr embedee_exposed_services;
+ embeddee_provider_.AddBinding(GetProxy(&embedee_exposed_services));
GURL embedded_app_url = url_.Resolve("wm_flow_embedded.mojo");
- scoped_ptr<mojo::ServiceProvider> imported =
- embed->Embed(embedded_app_url.spec(), registry.Pass());
+ mojo::ServiceProviderPtr imported;
+ embed->Embed(embedded_app_url.spec(), GetProxy(&imported),
+ embedee_exposed_services.Pass());
// FIXME: This is wrong. We're storing per-view state on this per-app
// delegate. Every time a new view is created embedee_ is replaced
// causing the previous one to shut down. This class should not
@@ -154,9 +177,9 @@
mojo::Shell* shell_;
int embed_count_;
scoped_ptr<mojo::ViewManagerClientFactory> view_manager_client_factory_;
- mojo::InterfaceFactoryImpl<EmbedderImpl> embedder_factory_;
scoped_ptr<mojo::ViewManagerContext> view_manager_context_;
EmbeddeePtr embeddee_;
+ EmbedderImplProvider embeddee_provider_;
ViewToUploader uploaders_;
GURL url_;
diff --git a/examples/wm_flow/embedded/embedded.cc b/examples/wm_flow/embedded/embedded.cc
index b2a2bd0..ed9cbb5 100644
--- a/examples/wm_flow/embedded/embedded.cc
+++ b/examples/wm_flow/embedded/embedded.cc
@@ -44,7 +44,9 @@
class WMFlowEmbedded : public mojo::ApplicationDelegate,
public mojo::ViewManagerDelegate {
public:
- WMFlowEmbedded() : shell_(nullptr) {}
+ WMFlowEmbedded() : shell_(nullptr) {
+ embeddee_provider_impl_.AddService(&embeddee_factory_);
+ }
virtual ~WMFlowEmbedded() {}
private:
@@ -61,10 +63,9 @@
}
// Overridden from mojo::ViewManagerDelegate:
- virtual void OnEmbed(
- mojo::View* root,
- mojo::ServiceProviderImpl* exported_services,
- scoped_ptr<mojo::ServiceProvider> imported_services) override {
+ virtual void OnEmbed(mojo::View* root,
+ mojo::InterfaceRequest<mojo::ServiceProvider> services,
+ mojo::ServiceProviderPtr exposed_services) override {
bitmap_uploader_.reset(new mojo::BitmapUploader(root));
bitmap_uploader_->Init(shell_);
// BitmapUploader does not track view size changes, we would
@@ -73,11 +74,11 @@
// object instead of holding per-view state on the ApplicationDelegate.
bitmap_uploader_->SetColor(SK_ColorMAGENTA);
- exported_services->AddService(&embeddee_factory_);
+ embeddee_provider_impl_.Bind(services.Pass());
// FIXME: embedder_ is wrong for the same reason the embedee_ storage is
// wrong in app.cc. We need separate per-instance storage not on the
// application delegate.
- mojo::ConnectToService(imported_services.get(), &embedder_);
+ mojo::ConnectToService(exposed_services.get(), &embedder_);
embedder_->HelloWorld(base::Bind(&WMFlowEmbedded::HelloWorldAck,
base::Unretained(this)));
}
@@ -91,6 +92,7 @@
mojo::Shell* shell_;
scoped_ptr<mojo::ViewManagerClientFactory> view_manager_client_factory_;
EmbedderPtr embedder_;
+ mojo::ServiceProviderImpl embeddee_provider_impl_;
mojo::InterfaceFactoryImpl<EmbeddeeImpl> embeddee_factory_;
scoped_ptr<mojo::BitmapUploader> bitmap_uploader_;
diff --git a/examples/wm_flow/wm/frame_controller.cc b/examples/wm_flow/wm/frame_controller.cc
index 7ddfe6a..e68ad44 100644
--- a/examples/wm_flow/wm/frame_controller.cc
+++ b/examples/wm_flow/wm/frame_controller.cc
@@ -7,7 +7,7 @@
#include "base/macros.h"
#include "base/strings/utf_string_conversions.h"
#include "mojo/converters/geometry/geometry_type_converters.h"
-#include "mojo/public/cpp/application/service_provider_impl.h"
+#include "mojo/public/interfaces/application/service_provider.mojom.h"
#include "mojo/services/view_manager/public/cpp/view.h"
#include "mojo/services/view_manager/public/cpp/view_manager.h"
#include "services/window_manager/capture_controller.h"
@@ -31,12 +31,11 @@
view_->SetVisible(true); // FIXME: This should not be our responsibility?
*app_view = app_view_;
- scoped_ptr<mojo::ServiceProviderImpl> exported_services(
- new mojo::ServiceProviderImpl());
- exported_services->AddService(this);
+ viewer_services_impl_.AddService(this);
+ mojo::ServiceProviderPtr viewer_services;
+ viewer_services_impl_.Bind(GetProxy(&viewer_services));
- viewer_services_ =
- view_->Embed(frame_app_url.spec(), exported_services.Pass());
+ view_->Embed(frame_app_url.spec(), nullptr, viewer_services.Pass());
// We weren't observing when our initial bounds was set:
OnViewBoundsChanged(view, view->bounds(), view->bounds());
diff --git a/examples/wm_flow/wm/frame_controller.h b/examples/wm_flow/wm/frame_controller.h
index 95f781d..e251c76 100644
--- a/examples/wm_flow/wm/frame_controller.h
+++ b/examples/wm_flow/wm/frame_controller.h
@@ -8,6 +8,7 @@
#include "base/memory/scoped_ptr.h"
#include "examples/wm_flow/wm/window_frame_host.mojom.h"
#include "mojo/public/cpp/application/interface_factory.h"
+#include "mojo/public/cpp/application/service_provider_impl.h"
#include "mojo/services/view_manager/public/cpp/view_observer.h"
#include "services/window_manager/focus_controller.h"
#include "ui/gfx/geometry/rect.h"
@@ -59,7 +60,7 @@
bool maximized_;
gfx::Rect restored_bounds_;
window_manager::WindowManagerApp* window_manager_app_;
- scoped_ptr<mojo::ServiceProvider> viewer_services_;
+ mojo::ServiceProviderImpl viewer_services_impl_;
mojo::Binding<examples::WindowFrameHost> binding_;
diff --git a/examples/wm_flow/wm/wm.cc b/examples/wm_flow/wm/wm.cc
index 67008bf..c7dfc52 100644
--- a/examples/wm_flow/wm/wm.cc
+++ b/examples/wm_flow/wm/wm.cc
@@ -46,10 +46,9 @@
}
// Overridden from mojo::ViewManagerDelegate:
- virtual void OnEmbed(
- mojo::View* root,
- mojo::ServiceProviderImpl* exported_services,
- scoped_ptr<mojo::ServiceProvider> remote_service_provider) override {
+ virtual void OnEmbed(mojo::View* root,
+ mojo::InterfaceRequest<mojo::ServiceProvider> services,
+ mojo::ServiceProviderPtr exposed_services) override {
root_ = root;
window_container_ = root->view_manager()->CreateView();
@@ -66,13 +65,13 @@
}
// Overridden from mojo::WindowManagerDelegate:
- virtual void Embed(
- const mojo::String& url,
- mojo::InterfaceRequest<mojo::ServiceProvider> service_provider) override {
+ virtual void Embed(const mojo::String& url,
+ mojo::InterfaceRequest<mojo::ServiceProvider> services,
+ mojo::ServiceProviderPtr exposed_services) override {
DCHECK(root_);
mojo::View* app_view = NULL;
CreateTopLevelWindow(&app_view);
- app_view->Embed(url, service_provider.Pass());
+ app_view->Embed(url, services.Pass(), exposed_services.Pass());
}
// Overridden from mojo::ViewObserver: