Remove OpenWindow from WindowManager API. This time much

simpler than before. Instead the test just waits for the
embed to happen (since it's all URL fakery anyway) and
returns the id of the root view.

Also adds a simple window manager shell implementation
for tests, needed to keep the window manager tests from
crashing.

R=sky@chromium.org
BUG=none

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

Cr-Commit-Position: refs/heads/master@{#289467}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@289467 0039d316-1c4b-4281-b951-d872f2087c98

mojo/public/cpp/application/lib/service_provider_impl.cc

@@ -19,7 +19,7 @@ ServiceProviderImpl::~ServiceProviderImpl() {
 ServiceProvider* ServiceProviderImpl::CreateRemoteServiceProvider() {
   // TODO(beng): it sure would be nice if this method could return a scoped_ptr.
   MOJO_DCHECK(!remote_);
-  remote_ = new internal::WeakServiceProvider(client());
+  remote_ = new internal::WeakServiceProvider(this, client());
   return remote_;
 }
 
@@ -38,10 +38,7 @@ void ServiceProviderImpl::ConnectToService(
 }
 
 void ServiceProviderImpl::OnConnectionError() {
-  if (remote_) {
-    remote_->Clear();
-    remote_ = NULL;
-  }
+  ClearRemote();
 }
 
 void ServiceProviderImpl::AddServiceConnector(
@@ -62,4 +59,11 @@ void ServiceProviderImpl::RemoveServiceConnector(
   service_connectors_.erase(it);
 }
 
+void ServiceProviderImpl::ClearRemote() {
+  if (remote_) {
+    remote_->Clear();
+    remote_ = NULL;
+  }
+}
+
 }  // namespace mojo

mojo/public/cpp/application/lib/weak_service_provider.cc

@@ -3,17 +3,25 @@
 // found in the LICENSE file.
 
 #include "mojo/public/cpp/application/lib/weak_service_provider.h"
+
+#include "mojo/public/cpp/application/service_provider_impl.h"
 #include "mojo/public/interfaces/application/service_provider.mojom.h"
 
 namespace mojo {
 namespace internal {
 
-WeakServiceProvider::WeakServiceProvider(ServiceProvider* service_provider)
-      : service_provider_(service_provider) {}
+WeakServiceProvider::WeakServiceProvider(ServiceProviderImpl* creator,
+                                         ServiceProvider* service_provider)
+      : creator_(creator),
+        service_provider_(service_provider) {}
 
-WeakServiceProvider::~WeakServiceProvider() {}
+WeakServiceProvider::~WeakServiceProvider() {
+  if (creator_)
+    creator_->ClearRemote();
+}
 
 void WeakServiceProvider::Clear() {
+  creator_ = NULL;
   service_provider_ = NULL;
 }
 

mojo/public/cpp/application/lib/weak_service_provider.h

@@ -8,6 +8,7 @@
 #include "mojo/public/interfaces/application/service_provider.mojom.h"
 
 namespace mojo {
+class ServiceProviderImpl;
 namespace internal {
 class ServiceConnectorBase;
 
@@ -17,7 +18,8 @@ class ServiceConnectorBase;
 // Calls to ConnectToService() are silently dropped when the pipe is closed.
 class WeakServiceProvider : public ServiceProvider {
  public:
-  explicit WeakServiceProvider(ServiceProvider* service_provider);
+  WeakServiceProvider(ServiceProviderImpl* creator,
+                      ServiceProvider* service_provider);
   virtual ~WeakServiceProvider();
 
   void Clear();
@@ -28,6 +30,7 @@ class WeakServiceProvider : public ServiceProvider {
       const String& service_name,
       ScopedMessagePipeHandle client_handle) MOJO_OVERRIDE;
 
+  ServiceProviderImpl* creator_;
   ServiceProvider* service_provider_;
 
   MOJO_DISALLOW_COPY_AND_ASSIGN(WeakServiceProvider);

mojo/public/cpp/application/service_provider_impl.h

@@ -39,6 +39,8 @@ class ServiceProviderImpl : public InterfaceImpl<ServiceProvider> {
   typedef std::map<std::string, internal::ServiceConnectorBase*>
       NameToServiceConnectorMap;
 
+  friend class internal::WeakServiceProvider;
+
   // Overridden from ServiceProvider:
   virtual void ConnectToService(
       const String& service_name,
@@ -52,6 +54,8 @@ class ServiceProviderImpl : public InterfaceImpl<ServiceProvider> {
   void RemoveServiceConnector(
       internal::ServiceConnectorBase* service_connector);
 
+  void ClearRemote();
+
   NameToServiceConnectorMap service_connectors_;
 
   internal::WeakServiceProvider* remote_;

mojo/services/public/interfaces/window_manager/window_manager.mojom

@@ -6,11 +6,9 @@ module mojo {
 
 [Client=WindowManagerClient]
 interface WindowManagerService {
-  OpenWindow() => (uint32 node_id);
-  OpenWindowWithURL(string url) => (uint32 node_id);
-  SetCapture(uint32 node_id) => (bool success);
-  FocusWindow(uint32 node_id) => (bool success);
-  ActivateWindow(uint32 node_id) => (bool success);
+  SetCapture(uint32 view_id) => (bool success);
+  FocusWindow(uint32 view_id) => (bool success);
+  ActivateWindow(uint32 view_id) => (bool success);
 };
 
 [Client=WindowManagerService]
@@ -19,9 +17,9 @@ interface WindowManagerClient {
   // connects to it before it has been initialized).
   OnWindowManagerReady();
 
-  // TODO(beng): how is the WM supposed to know if a node is known to a client
+  // TODO(beng): how is the WM supposed to know if a view is known to a client
   //             or not?
-  OnCaptureChanged(uint32 old_capture_node_id, uint32 new_capture_node_id);
+  OnCaptureChanged(uint32 old_capture_view_id, uint32 new_capture_view_id);
 
   OnFocusChanged(uint32 old_focused_node_id, uint32 new_focused_node_id);
   OnActiveWindowChanged(uint32 old_focused_window, uint32 new_focused_window);

mojo/services/window_manager/main.cc

@@ -2,7 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/memory/scoped_ptr.h"
 #include "mojo/public/cpp/application/application_delegate.h"
+#include "mojo/public/cpp/application/service_provider_impl.h"
+#include "mojo/services/public/cpp/view_manager/view_manager.h"
+#include "mojo/services/public/cpp/view_manager/view_manager_delegate.h"
+#include "mojo/services/public/cpp/view_manager/window_manager_delegate.h"
 #include "mojo/services/window_manager/window_manager_app.h"
 
 // ApplicationDelegate implementation file for WindowManager users (e.g.
@@ -11,9 +16,62 @@
 
 namespace mojo {
 
+class DefaultWindowManager : public ApplicationDelegate,
+                             public ViewManagerDelegate,
+                             public WindowManagerDelegate {
+ public:
+  DefaultWindowManager()
+      : window_manager_app_(new WindowManagerApp(this)),
+        view_manager_(NULL),
+        root_(NULL) {}
+  virtual ~DefaultWindowManager() {}
+
+ private:
+  // Overridden from ApplicationDelegate:
+  virtual void Initialize(ApplicationImpl* impl) MOJO_OVERRIDE {
+    window_manager_app_->Initialize(impl);
+  }
+  virtual bool ConfigureIncomingConnection(
+      ApplicationConnection* connection) MOJO_OVERRIDE {
+    window_manager_app_->ConfigureIncomingConnection(connection);
+    return true;
+  }
+
+  // Overridden from ViewManagerDelegate:
+  virtual void OnEmbed(
+      ViewManager* view_manager,
+      View* root,
+      ServiceProviderImpl* exported_services,
+      scoped_ptr<ServiceProvider> imported_services) MOJO_OVERRIDE {
+    view_manager_ = view_manager;
+    root_ = root;
+    view_manager_->SetWindowManagerDelegate(this);
+  }
+  virtual void OnViewManagerDisconnected(
+      ViewManager* view_manager) MOJO_OVERRIDE {}
+
+  // Overridden from WindowManagerDelegate:
+  virtual void Embed(
+      const String& url,
+      InterfaceRequest<ServiceProvider> service_provider) MOJO_OVERRIDE {
+    View* view = View::Create(view_manager_);
+    root_->AddChild(view);
+    view->Embed(url, scoped_ptr<mojo::ServiceProviderImpl>(
+        new mojo::ServiceProviderImpl).Pass());
+  }
+  virtual void DispatchEvent(View* target, EventPtr event) MOJO_OVERRIDE {}
+
+  scoped_ptr<WindowManagerApp> window_manager_app_;
+
+  ViewManager* view_manager_;
+  View* root_;
+
+  MOJO_DISALLOW_COPY_AND_ASSIGN(DefaultWindowManager);
+};
+
 // static
 ApplicationDelegate* ApplicationDelegate::Create() {
-  return new WindowManagerApp(NULL);
+  return new DefaultWindowManager;
 }
 
 }  // namespace mojo

mojo/services/window_manager/window_manager_api_unittest.cc

@@ -47,33 +47,6 @@ bool InitEmbed(ViewManagerInitService* view_manager_init,
   return result;
 }
 
-void OpenWindowCallback(Id* id,
-                        base::RunLoop* run_loop,
-                        Id window_id) {
-  *id = window_id;
-  run_loop->Quit();
-}
-
-Id OpenWindow(WindowManagerService* window_manager) {
-  base::RunLoop run_loop;
-  Id id;
-  window_manager->OpenWindow(
-      base::Bind(&OpenWindowCallback, &id, &run_loop));
-  run_loop.Run();
-  return id;
-}
-
-Id OpenWindowWithURL(WindowManagerService* window_manager,
-                                   const std::string& url) {
-  base::RunLoop run_loop;
-  Id id;
-  window_manager->OpenWindowWithURL(
-      url,
-      base::Bind(&OpenWindowCallback, &id, &run_loop));
-  run_loop.Run();
-  return id;
-}
-
 class TestWindowManagerClient : public WindowManagerClient {
  public:
   typedef base::Callback<void(Id, Id)>
@@ -211,6 +184,15 @@ class WindowManagerApiTest : public testing::Test {
     return old_and_new;
   }
 
+  Id OpenWindow() {
+    return OpenWindowWithURL(kTestServiceURL);
+  }
+
+  Id OpenWindowWithURL(const std::string& url) {
+    InitEmbed(view_manager_init_.get(), url);
+    return WaitForEmbed();
+  }
+
   TestWindowManagerClient* window_manager_client() {
     return window_manager_client_.get();
   }
@@ -282,23 +264,15 @@ class WindowManagerApiTest : public testing::Test {
   DISALLOW_COPY_AND_ASSIGN(WindowManagerApiTest);
 };
 
-TEST_F(WindowManagerApiTest, OpenWindow) {
-  OpenWindow(window_manager_.get());
-  Id created_node =
-      OpenWindowWithURL(window_manager_.get(), kTestServiceURL);
-  Id embed_node = WaitForEmbed();
-  EXPECT_EQ(created_node, embed_node);
-}
-
 TEST_F(WindowManagerApiTest, FocusAndActivateWindow) {
-  Id first_window = OpenWindow(window_manager_.get());
+  Id first_window = OpenWindow();
   window_manager_->FocusWindow(first_window,
                                base::Bind(&EmptyResultCallback));
   TwoIds ids = WaitForFocusChange();
   EXPECT_TRUE(ids.first == 0);
   EXPECT_EQ(ids.second, first_window);
 
-  Id second_window = OpenWindow(window_manager_.get());
+  Id second_window = OpenWindow();
   window_manager_->ActivateWindow(second_window,
                                   base::Bind(&EmptyResultCallback));
   ids = WaitForActiveWindowChange();

mojo/services/window_manager/window_manager_app.cc

@@ -78,15 +78,7 @@ WindowManagerApp::WindowManagerApp(ViewManagerDelegate* delegate)
       root_(NULL) {
 }
 
-WindowManagerApp::~WindowManagerApp() {
-  // TODO(beng): Figure out if this should be done in
-  //             OnViewManagerDisconnected().
-  STLDeleteValues(&view_id_to_window_map_);
-  if (focus_client_.get())
-    focus_client_->RemoveObserver(this);
-  if (activation_client_)
-    activation_client_->RemoveObserver(this);
-}
+WindowManagerApp::~WindowManagerApp() {}
 
 void WindowManagerApp::AddConnection(WindowManagerServiceImpl* connection) {
   DCHECK(connections_.find(connection) == connections_.end());
@@ -98,19 +90,6 @@ void WindowManagerApp::RemoveConnection(WindowManagerServiceImpl* connection) {
   connections_.erase(connection);
 }
 
-Id WindowManagerApp::OpenWindow() {
-  View* view = View::Create(view_manager_);
-  root_->AddChild(view);
-  return view->id();
-}
-
-Id WindowManagerApp::OpenWindowWithURL(const String& url) {
-  View* view = View::Create(view_manager_);
-  root_->AddChild(view);
-  view->Embed(url);
-  return view->id();
-}
-
 void WindowManagerApp::SetCapture(Id view) {
   capture_client_->capture_client()->SetCapture(GetWindowForViewId(view));
   // TODO(beng): notify connected clients that capture has changed, probably
@@ -218,6 +197,11 @@ void WindowManagerApp::OnTreeChanged(
 
 void WindowManagerApp::OnViewDestroyed(View* view) {
   root_ = NULL;
+  STLDeleteValues(&view_id_to_window_map_);
+  if (focus_client_.get())
+    focus_client_->RemoveObserver(this);
+  if (activation_client_)
+    activation_client_->RemoveObserver(this);
   window_tree_host_.reset();
 }
 

mojo/services/window_manager/window_manager_app.h

@@ -52,8 +52,6 @@ class WindowManagerApp
   void AddConnection(WindowManagerServiceImpl* connection);
   void RemoveConnection(WindowManagerServiceImpl* connection);
 
-  Id OpenWindow();
-  Id OpenWindowWithURL(const String& url);
   void SetCapture(Id view);
   void FocusWindow(Id view);
   void ActivateWindow(Id view);

mojo/services/window_manager/window_manager_service_impl.cc

@@ -39,55 +39,30 @@ void WindowManagerServiceImpl::NotifyWindowActivated(Id new_active_id,
 ////////////////////////////////////////////////////////////////////////////////
 // WindowManagerServiceImpl, WindowManager implementation:
 
-void WindowManagerServiceImpl::OpenWindow(
-    const Callback<void(Id)>& callback) {
-  bool success = window_manager_->IsReady();
-  if (success) {
-    Id id = window_manager_->OpenWindow();
-    callback.Run(id);
-  } else {
-    // TODO(beng): perhaps should take an error code for this.
-    callback.Run(0);
-  }
-}
-
-void WindowManagerServiceImpl::OpenWindowWithURL(
-    const String& url,
-    const Callback<void(Id)>& callback) {
-  bool success = window_manager_->IsReady();
-  if (success) {
-    Id id = window_manager_->OpenWindowWithURL(url);
-    callback.Run(id);
-  } else {
-    // TODO(beng): perhaps should take an error code for this.
-    callback.Run(0);
-  }
-}
-
 void WindowManagerServiceImpl::SetCapture(
-    Id node,
+    Id view,
     const Callback<void(bool)>& callback) {
   bool success = window_manager_->IsReady();
   if (success)
-    window_manager_->SetCapture(node);
+    window_manager_->SetCapture(view);
   callback.Run(success);
 }
 
 void WindowManagerServiceImpl::FocusWindow(
-    Id node,
+    Id view,
     const Callback<void(bool)>& callback) {
   bool success = window_manager_->IsReady();
   if (success)
-    window_manager_->FocusWindow(node);
+    window_manager_->FocusWindow(view);
   callback.Run(success);
 }
 
 void WindowManagerServiceImpl::ActivateWindow(
-    Id node,
+    Id view,
     const Callback<void(bool)>& callback) {
   bool success = window_manager_->IsReady();
   if (success)
-    window_manager_->ActivateWindow(node);
+    window_manager_->ActivateWindow(view);
   callback.Run(success);
 }
 

mojo/services/window_manager/window_manager_service_impl.h

@@ -24,16 +24,12 @@ class WindowManagerServiceImpl : public InterfaceImpl<WindowManagerService> {
 
  private:
   // Overridden from WindowManagerService:
-  virtual void OpenWindow(const Callback<void(Id)>& callback) MOJO_OVERRIDE;
-  virtual void OpenWindowWithURL(
-      const String& url,
-      const Callback<void(Id)>& callback) MOJO_OVERRIDE;
-  virtual void SetCapture(Id node,
+  virtual void SetCapture(Id view,
                           const Callback<void(bool)>& callback) MOJO_OVERRIDE;
-  virtual void FocusWindow(Id node,
+  virtual void FocusWindow(Id view,
                            const Callback<void(bool)>& callback) MOJO_OVERRIDE;
   virtual void ActivateWindow(
-      Id node,
+      Id view,
       const Callback<void(bool)>& callback) MOJO_OVERRIDE;
 
   // Overridden from InterfaceImpl: