Revert 265509 "Gets Add/Remove working correctly in the view man..."

> Gets Add/Remove working correctly in the view manager > > Also adds tests > > BUG=365012 > TEST=covered by tests > R=ben@chromium.org > > Review URL: https://codereview.chromium.org/247363006 TBR=sky@chromium.org Review URL: https://codereview.chromium.org/248713002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@265519 0039d316-1c4b-4281-b951-d872f2087c98
Denger-Network · Apr 23, 2014 · c115146 · c115146
1 parent 0113f8e
commit c115146
Show file tree

Hide file tree

Showing 13 changed files with 29 additions and 631 deletions.
diff --git a/mojo/examples/sample_view_manager_app/sample_view_manager_app.cc b/mojo/examples/sample_view_manager_app/sample_view_manager_app.cc
@@ -47,13 +47,6 @@ class SampleApp : public Application,
   virtual void OnConnectionEstablished(int32_t manager_id) OVERRIDE {
   }
 
-  virtual void OnViewHierarchyChanged(
-      const services::view_manager::ViewId& view,
-      const services::view_manager::ViewId& new_parent,
-      const services::view_manager::ViewId& old_parent,
-      int32_t change_id) OVERRIDE {
-  }
-
  private:
   void OnCreatedView(bool success) {
     DCHECK(success);

diff --git a/mojo/mojo.gyp b/mojo/mojo.gyp
@@ -61,7 +61,6 @@
             'mojo_launcher',
             'mojo_sample_view_manager_app',
             'mojo_view_manager',
-            'mojo_view_manager_unittests',
           ],
         }],
       ]

diff --git a/mojo/mojo_services.gypi b/mojo/mojo_services.gypi
@@ -181,33 +181,9 @@
             'services/view_manager/root_view_manager.h',
             'services/view_manager/view.cc',
             'services/view_manager/view.h',
-            'services/view_manager/view_delegate.h',
-            'services/view_manager/view_manager.cc',
             'services/view_manager/view_manager_connection.cc',
             'services/view_manager/view_manager_connection.h',
-            'services/view_manager/view_manager_export.h',
-          ],
-          'defines': [
-            'MOJO_VIEW_MANAGER_IMPLEMENTATION',
-          ],
-        },
-        {
-          'target_name': 'mojo_view_manager_unittests',
-          'type': 'executable',
-          'dependencies': [
-            '../base/base.gyp:base',
-            '../skia/skia.gyp:skia',
-            '../testing/gtest.gyp:gtest',
-            '../ui/aura/aura.gyp:aura',
-            'mojo_environment_chromium',
-            'mojo_run_all_unittests',
-            'mojo_shell_client',
-            'mojo_system_impl',
-            'mojo_view_manager',
-            'mojo_view_manager_bindings',
-          ],
-          'sources': [
-            'services/view_manager/view_manager_connection_unittest.cc',
+            'services/view_manager/view_manager.cc',
           ],
         },
         {

diff --git a/mojo/services/public/interfaces/view_manager/view_manager.mojom b/mojo/services/public/interfaces/view_manager/view_manager.mojom
@@ -6,48 +6,32 @@ module mojo.services.view_manager {
 
 // Each View is identified by the following pair. The |manager_id| is assigned
 // by the server and uniquely identifies the ViewManager. A value of 0 can be
-// used to indicate 'this' manager. For received notifications a value of 0 for
-// |manager_id| indicates the view is owned by the ViewManager. The |view_id|
-// is assigned by the client. Non-negative values may be used. Server uses
-// negative to identify special values. For example, -1 is the root.
+// used to indicate 'this' manager. The |view_id| is assigned by the client.
+// Non-negative values may be used. Server uses negative to identify special
+// values. For example, -1 is the root.
 struct ViewId {
   int32 manager_id;
   int32 view_id;
 };
 
-
-// Functions that mutate the hierarchy take a |change_id|. |change_id| is an
-// arbitrary value assigned by the client originating the change. It may be
-// used by the client originating the change to later identify the change in
-// an OnViewHierarchyChanged callback. |change_id| is only passed to the client
-// that originated the change. All other clients get a value of 0.
 [Peer=ViewManagerClient]
 interface ViewManager {
   // Creates a new view with the specified id. It is up to the client to ensure
   // the id is unique to the connection (the id need not be globally unique).
   CreateView(int32 view_id) => (bool success);
 
-  // Reparents a view. See description above class for details of |change_id|.
-  AddView(ViewId parent, ViewId child, int32 change_id) => (bool success);
+  // Reparents a view.
+  AddView(ViewId parent, ViewId child) => (bool success);
 
-  // Removes a view from its current parent.  See description above class for
-  // details of |change_id|.
-  RemoveViewFromParent(ViewId view, int32 change_id) => (bool success);
+  // Removes a view from its current parent.
+  RemoveFromParent(ViewId view) => (bool success);
 };
 
 [Peer=ViewManager]
 interface ViewManagerClient {
   // Invoked once the connection has been established. |manager_id| is the id
   // used to uniquely identify the ViewManager.
   OnConnectionEstablished(int32 manager_id);
-
-  // Invoked when a change is done to the hierarchy. |new_parent| and/or
-  // |old_parent| may be NULL. See description above ViewManager for details on
-  // |change_id|.
-  OnViewHierarchyChanged(ViewId view,
-                         ViewId new_parent,
-                         ViewId old_parent,
-                         int32 change_id);
 };
 
 }
diff --git a/mojo/services/view_manager/root_view_manager.cc b/mojo/services/view_manager/root_view_manager.cc
@@ -17,20 +17,9 @@ const int32_t kRootId = -1;
 
 }  // namespace
 
-RootViewManager::ScopedChange::ScopedChange(ViewManagerConnection* connection,
-                                            RootViewManager* root,
-                                            int32_t change_id)
-    : root_(root) {
-  root_->PrepareForChange(connection, change_id);
-}
-
-RootViewManager::ScopedChange::~ScopedChange() {
-  root_->FinishChange();
-}
-
 RootViewManager::RootViewManager()
     : next_connection_id_(1),
-      root_(this, kRootId) {
+      root_(kRootId) {
 }
 
 RootViewManager::~RootViewManager() {
@@ -56,29 +45,6 @@ View* RootViewManager::GetView(int32_t manager_id, int32_t view_id) {
   return i == connection_map_.end() ? NULL : i->second->GetView(view_id);
 }
 
-void RootViewManager::NotifyViewHierarchyChanged(const ViewId& view,
-                                                 const ViewId& new_parent,
-                                                 const ViewId& old_parent) {
-  for (ConnectionMap::iterator i = connection_map_.begin();
-       i != connection_map_.end(); ++i) {
-    const int32_t change_id = (change_ && i->first == change_->connection_id) ?
-        change_->change_id : 0;
-    i->second->NotifyViewHierarchyChanged(
-        view, new_parent, old_parent, change_id);
-  }
-}
-
-void RootViewManager::PrepareForChange(ViewManagerConnection* connection,
-                                       int32_t change_id) {
-  DCHECK(!change_.get());  // Should only ever have one change in flight.
-  change_.reset(new Change(connection->id(), change_id));
-}
-
-void RootViewManager::FinishChange() {
-  DCHECK(change_.get());  // PrepareForChange/FinishChange should be balanced.
-  change_.reset();
-}
-
 void RootViewManager::OnCreated() {
 }
 
@@ -93,19 +59,6 @@ void RootViewManager::OnEvent(const Event& event,
   callback.Run();
 }
 
-ViewId RootViewManager::GetViewId(const View* view) const {
-  ViewId::Builder builder;
-  builder.set_manager_id(0);
-  builder.set_view_id(view->id());
-  return builder.Finish();
-}
-
-void RootViewManager::OnViewHierarchyChanged(const ViewId& view,
-                                             const ViewId& new_parent,
-                                             const ViewId& old_parent) {
-  NotifyViewHierarchyChanged(view, new_parent, old_parent);
-}
-
 }  // namespace view_manager
 }  // namespace services
 }  // namespace mojo
diff --git a/mojo/services/view_manager/root_view_manager.h b/mojo/services/view_manager/root_view_manager.h
@@ -10,8 +10,6 @@
 #include "base/basictypes.h"
 #include "mojo/services/native_viewport/native_viewport.mojom.h"
 #include "mojo/services/view_manager/view.h"
-#include "mojo/services/view_manager/view_delegate.h"
-#include "mojo/services/view_manager/view_manager_export.h"
 
 namespace mojo {
 namespace services {
@@ -23,25 +21,8 @@ class ViewManagerConnection;
 
 // RootViewManager is responsible for managing the set of ViewManagerConnections
 // as well as providing the root of the View hierarchy.
-class MOJO_VIEW_MANAGER_EXPORT RootViewManager
-    : public NativeViewportClient,
-      public ViewDelegate {
+class RootViewManager : public NativeViewportClient {
  public:
-  // Create when a ViewManagerConnection is about to make a change. Ensures
-  // clients are notified of the correct change id.
-  class ScopedChange {
-   public:
-    ScopedChange(ViewManagerConnection* connection,
-                 RootViewManager* root,
-                 int32_t change_id);
-    ~ScopedChange();
-
-   private:
-    RootViewManager* root_;
-
-    DISALLOW_COPY_AND_ASSIGN(ScopedChange);
-  };
-
   RootViewManager();
   virtual ~RootViewManager();
 
@@ -55,49 +36,16 @@ class MOJO_VIEW_MANAGER_EXPORT RootViewManager
   // one.
   View* GetView(int32_t manager_id, int32_t view_id);
 
-  // Notifies all ViewManagerConnections of a hierarchy change.
-  void NotifyViewHierarchyChanged(const ViewId& view,
-                                  const ViewId& new_parent,
-                                  const ViewId& old_parent);
-
  private:
-  // Tracks a change.
-  struct Change {
-    Change(int32_t connection_id, int32_t change_id)
-        : connection_id(connection_id),
-          change_id(change_id) {
-    }
-
-    int32_t connection_id;
-    int32_t change_id;
-  };
-
   typedef std::map<int32_t, ViewManagerConnection*> ConnectionMap;
 
-  // Invoked when a particular connection is about to make a change. Records
-  // the |change_id| so that it can be supplied to the clients by way of
-  // OnViewHierarchyChanged().
-  // Changes should never nest, meaning each PrepareForChange() must be
-  // balanced with a call to FinishChange() with no PrepareForChange()
-  // in between.
-  void PrepareForChange(ViewManagerConnection* connection, int32_t change_id);
-
-  // Balances a call to PrepareForChange().
-  void FinishChange();
-
   // Overridden from NativeViewportClient:
   virtual void OnCreated() OVERRIDE;
   virtual void OnDestroyed() OVERRIDE;
   virtual void OnBoundsChanged(const Rect& bounds) OVERRIDE;
   virtual void OnEvent(const Event& event,
                        const mojo::Callback<void()>& callback) OVERRIDE;
 
-  // Overriden from ViewDelegate:
-  virtual ViewId GetViewId(const View* view) const OVERRIDE;
-  virtual void OnViewHierarchyChanged(const ViewId& view,
-                                      const ViewId& new_parent,
-                                      const ViewId& old_parent) OVERRIDE;
-
   // ID to use for next ViewManagerConnection.
   int32_t next_connection_id_;
 
@@ -107,9 +55,6 @@ class MOJO_VIEW_MANAGER_EXPORT RootViewManager
   // Root of Views that are displayed.
   View root_;
 
-  // If non-null we're processing a change.
-  scoped_ptr<Change> change_;
-
   DISALLOW_COPY_AND_ASSIGN(RootViewManager);
 };
 

diff --git a/mojo/services/view_manager/view.cc b/mojo/services/view_manager/view.cc
@@ -4,8 +4,6 @@
 
 #include "mojo/services/view_manager/view.h"
 
-#include "mojo/public/cpp/bindings/allocation_scope.h"
-#include "mojo/services/view_manager/view_delegate.h"
 #include "ui/aura/window_property.h"
 
 DECLARE_WINDOW_PROPERTY_TYPE(mojo::services::view_manager::View*);
@@ -16,14 +14,9 @@ namespace view_manager {
 
 DEFINE_WINDOW_PROPERTY_KEY(View*, kViewKey, NULL);
 
-View::View(ViewDelegate* delegate, const int32_t id)
-    : delegate_(delegate),
-      id_(id),
-      window_(NULL) {
-  DCHECK(delegate);  // Must provide a delegate.
+// TODO(sky): figure out window delegate.
+View::View(int32_t id) : id_(id), window_(NULL) {
   window_.set_owned_by_parent(false);
-  window_.AddObserver(this);
-  window_.SetProperty(kViewKey, this);
 }
 
 View::~View() {
@@ -43,25 +36,6 @@ void View::Remove(View* child) {
   window_.RemoveChild(&child->window_);
 }
 
-ViewId View::GetViewId() const {
-  return delegate_->GetViewId(this);
-}
-
-void View::OnWindowHierarchyChanged(
-    const aura::WindowObserver::HierarchyChangeParams& params) {
-  if (params.target != &window_ || params.receiver != &window_)
-    return;
-  AllocationScope scope;
-  ViewId new_parent_id;
-  if (params.new_parent)
-    new_parent_id = params.new_parent->GetProperty(kViewKey)->GetViewId();
-  ViewId old_parent_id;
-  if (params.old_parent)
-    old_parent_id = params.old_parent->GetProperty(kViewKey)->GetViewId();
-  delegate_->OnViewHierarchyChanged(delegate_->GetViewId(this), new_parent_id,
-                                    old_parent_id);
-}
-
 }  // namespace view_manager
 }  // namespace services
 }  // namespace mojo
diff --git a/mojo/services/view_manager/view.h b/mojo/services/view_manager/view.h
@@ -6,22 +6,15 @@
 #define MOJO_SERVICES_VIEW_MANAGER_VIEW_H_
 
 #include "base/logging.h"
-#include "mojo/services/public/interfaces/view_manager/view_manager.mojom.h"
-#include "mojo/services/view_manager/view_manager_export.h"
 #include "ui/aura/window.h"
-#include "ui/aura/window_observer.h"
 
 namespace mojo {
 namespace services {
 namespace view_manager {
 
-class RootViewManager;
-class ViewDelegate;
-class ViewId;
-
-class MOJO_VIEW_MANAGER_EXPORT View : public aura::WindowObserver {
+class View {
  public:
-  View(ViewDelegate* delegate, const int32_t id);
+  View(int32_t view_id);
   ~View();
 
   int32 id() const { return id_; }
@@ -32,13 +25,6 @@ class MOJO_VIEW_MANAGER_EXPORT View : public aura::WindowObserver {
   View* GetParent();
 
  private:
-  ViewId GetViewId() const;
-
-  // WindowObserver overrides:
-  virtual void OnWindowHierarchyChanged(
-      const aura::WindowObserver::HierarchyChangeParams& params) OVERRIDE;
-
-  ViewDelegate* delegate_;
   const int32_t id_;
   aura::Window window_;