V2: Implement reparenting, bounds and visibility change notifications, and write more tests.

TBR=sky@chromium.org
BUG=none

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@226847 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 320 insertions, 55 deletions

diff --git a/ui/v2/public/view.h b/ui/v2/public/view.h
index 975afbf..016d8d0 100644
--- a/ui/v2/public/view.h
+++ b/ui/v2/public/view.h
@@ -36,6 +36,8 @@ class V2_EXPORT View /* : public EventTarget */ {
   // View takes ownership.
   void SetPainter(Painter* painter);
 
+  // See documentation in layout.h. A layout manager's rules apply to this
+  // View's children.
   // View takes ownership.
   void SetLayout(Layout* layout);
 
@@ -70,9 +72,7 @@ class V2_EXPORT View /* : public EventTarget */ {
 
  private:
   friend class Layout;
-  friend class ViewObserversAccessor;
-
-  void SetBoundsInternal(const gfx::Rect& bounds);
+  friend class ViewPrivate;
 
   // Disposition attributes.
   gfx::Rect bounds_;
diff --git a/ui/v2/public/view_observer.h b/ui/v2/public/view_observer.h
index 9fcceb0..cb26b4b 100644
--- a/ui/v2/public/view_observer.h
+++ b/ui/v2/public/view_observer.h
@@ -52,14 +52,14 @@ class V2_EXPORT ViewObserver {
 
   // Disposition.
 
-  virtual void OnViewDestroying() {}
-  virtual void OnViewDestroyed() {}
+  virtual void OnViewDestroy(View* view, DispositionChangePhase phase) {}
 
-  virtual void OnViewBoundsChanged(const gfx::Rect& old_bounds,
+  virtual void OnViewBoundsChanged(View* view,
+                                   const gfx::Rect& old_bounds,
                                    const gfx::Rect& new_bounds) {}
 
-  virtual void OnViewVisibilityChanging() {}
-  virtual void OnViewVisibilityChanged() {}
+  virtual void OnViewVisibilityChange(View* view,
+                                      DispositionChangePhase phase) {}
 
  protected:
   virtual ~ViewObserver() {}
diff --git a/ui/v2/src/layout.cc b/ui/v2/src/layout.cc
index 567060c..9b213bd 100644
--- a/ui/v2/src/layout.cc
+++ b/ui/v2/src/layout.cc
@@ -5,6 +5,7 @@
 #include "ui/v2/public/layout.h"
 
 #include "ui/v2/public/view.h"
+#include "ui/v2/src/view_private.h"
 
 namespace v2 {
 
@@ -22,7 +23,7 @@ void Layout::SetChildBounds(View* child,
 // Layout, protected:
 
 void Layout::SetChildBoundsDirect(View* child, const gfx::Rect& bounds) {
-  child->SetBoundsInternal(bounds);
+  ViewPrivate(child).set_bounds(bounds);
 }
 
 }  // namespace v2
diff --git a/ui/v2/src/view.cc b/ui/v2/src/view.cc
index bef82e1..ee7242a 100644
--- a/ui/v2/src/view.cc
+++ b/ui/v2/src/view.cc
@@ -7,24 +7,10 @@
 #include <algorithm>
 
 #include "ui/v2/public/view_observer.h"
+#include "ui/v2/src/view_private.h"
 
 namespace v2 {
 
-// Friend of View. Provides a way to get at a View's observer list for
-// notification methods.
-class ViewObserversAccessor {
- public:
-  explicit ViewObserversAccessor(View* view) : view_(view) {}
-  ~ViewObserversAccessor() {}
-
-  ObserverList<ViewObserver>* get() { return &view_->observers_; }
-
- private:
-  View* view_;
-
-  DISALLOW_COPY_AND_ASSIGN(ViewObserversAccessor);
-};
-
 enum StackDirection {
   STACK_ABOVE,
   STACK_BELOW
@@ -41,9 +27,8 @@ void StackChildRelativeTo(View* parent,
   DCHECK_EQ(parent, child->parent());
   DCHECK_EQ(parent, other->parent());
 
-  // Notify stacking changing.
-
-  // TODO(beng): This is simpler than aura::Window's for now.
+  // TODO(beng): Notify stacking changing.
+  // TODO(beng): consult layout manager
   const size_t child_i =
       std::find(children->begin(), children->end(), child) - children->begin();
   const size_t other_i =
@@ -55,9 +40,8 @@ void StackChildRelativeTo(View* parent,
   children->erase(children->begin() + child_i);
   children->insert(children->begin() + destination_i, child);
 
-  // TODO(beng): restack layers.
-
-  // Notify stacking changed.
+  // TODO(beng): update layer.
+  // TODO(beng): Notify stacking changed.
 }
 
 void NotifyViewTreeChangeAtReceiver(
@@ -66,7 +50,7 @@ void NotifyViewTreeChangeAtReceiver(
   ViewObserver::TreeChangeParams local_params = params;
   local_params.receiver = receiver;
   FOR_EACH_OBSERVER(ViewObserver,
-                    *ViewObserversAccessor(receiver).get(),
+                    *ViewPrivate(receiver).observers(),
                     OnViewTreeChange(local_params));
 }
 
@@ -120,16 +104,26 @@ class ScopedTreeNotifier {
   DISALLOW_COPY_AND_ASSIGN(ScopedTreeNotifier);
 };
 
+void RemoveChildImpl(View* child, View::Children* children) {
+  std::vector<View*>::iterator it =
+      std::find(children->begin(), children->end(), child);
+  if (it != children->end()) {
+    children->erase(it);
+    ViewPrivate(child).ClearParent();
+  }
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // View, public:
 
 // Creation, configuration -----------------------------------------------------
 
-View::View() : visible_(false), owned_by_parent_(true), parent_(NULL) {
+View::View() : visible_(true), owned_by_parent_(true), parent_(NULL) {
 }
 
 View::~View() {
-  FOR_EACH_OBSERVER(ViewObserver, observers_, OnViewDestroying());
+  FOR_EACH_OBSERVER(ViewObserver, observers_,
+                    OnViewDestroy(this, ViewObserver::DISPOSITION_CHANGING));
 
   while (!children_.empty()) {
     View* child = children_.front();
@@ -146,7 +140,8 @@ View::~View() {
   if (parent_)
     parent_->RemoveChild(this);
 
-  FOR_EACH_OBSERVER(ViewObserver, observers_, OnViewDestroyed());
+  FOR_EACH_OBSERVER(ViewObserver, observers_,
+                    OnViewDestroy(this, ViewObserver::DISPOSITION_CHANGED));
 }
 
 void View::AddObserver(ViewObserver* observer) {
@@ -168,30 +163,50 @@ void View::SetLayout(Layout* layout) {
 // Disposition -----------------------------------------------------------------
 
 void View::SetBounds(const gfx::Rect& bounds) {
-  if (parent_ && parent_->layout_.get())
-    parent_->layout_->SetChildBounds(this, bounds);
-  else
-    SetBoundsInternal(bounds);
+  gfx::Rect old_bounds = bounds_;
+  // TODO(beng): consult layout manager
+  bounds_ = bounds;
+  // TODO(beng): update layer
+
+  // TODO(beng): write tests for this where layoutmanager prevents a change
+  //             and no changed notification is sent.
+  if (bounds_ != old_bounds) {
+    FOR_EACH_OBSERVER(ViewObserver, observers_, OnViewBoundsChanged(this,
+        old_bounds, bounds_));
+  }
 }
 
-void View::SetVisible(bool visible) {}
+void View::SetVisible(bool visible) {
+  FOR_EACH_OBSERVER(ViewObserver, observers_, OnViewVisibilityChange(this,
+      ViewObserver::DISPOSITION_CHANGING));
+
+  bool old_visible = visible_;
+  // TODO(beng): consult layout manager
+  visible_ = visible;
+  // TODO(beng): update layer
+
+  // TODO(beng): write tests for this where layoutmanager prevents a change
+  //             and no changed notification is sent.
+  if (old_visible != visible_) {
+    FOR_EACH_OBSERVER(ViewObserver, observers_, OnViewVisibilityChange(this,
+        ViewObserver::DISPOSITION_CHANGED));
+  }
+}
 
 // Tree ------------------------------------------------------------------------
 
 void View::AddChild(View* child) {
   ScopedTreeNotifier notifier(child, child->parent(), this);
+  if (child->parent())
+    RemoveChildImpl(child, &child->parent_->children_);
   children_.push_back(child);
   child->parent_ = this;
 }
 
 void View::RemoveChild(View* child) {
+  DCHECK_EQ(this, child->parent());
   ScopedTreeNotifier(child, this, NULL);
-  std::vector<View*>::iterator it =
-      std::find(children_.begin(), children_.end(), child);
-  if (it != children_.end()) {
-    children_.erase(it);
-    child->parent_ = NULL;
-  }
+  RemoveChildImpl(child, &children_);
 }
 
 bool View::Contains(View* child) const {
@@ -222,14 +237,4 @@ void View::StackChildBelow(View* child, View* other) {
   StackChildRelativeTo(this, &children_, child, other, STACK_BELOW);
 }
 
-////////////////////////////////////////////////////////////////////////////////
-// View, private:
-
-void View::SetBoundsInternal(const gfx::Rect& bounds) {
-  bounds_ = bounds;
-
-  // TODO(beng): Update layer.
-  // TODO(beng): Notify changed.
-}
-
 }  // namespace v2
diff --git a/ui/v2/src/view_private.cc b/ui/v2/src/view_private.cc
new file mode 100644
index 0000000..34a49c1
--- /dev/null
+++ b/ui/v2/src/view_private.cc
@@ -0,0 +1,12 @@
+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ui/v2/src/view_private.h"
+
+namespace v2 {
+
+ViewPrivate::ViewPrivate(View* view) : view_(view) {}
+ViewPrivate::~ViewPrivate() {}
+
+}  // namespace v2
diff --git a/ui/v2/src/view_private.h b/ui/v2/src/view_private.h
new file mode 100644
index 0000000..d59a34e
--- /dev/null
+++ b/ui/v2/src/view_private.h
@@ -0,0 +1,40 @@
+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef UI_V2_SRC_VIEW_PRIVATE_H_
+#define UI_V2_SRC_VIEW_PRIVATE_H_
+
+#include "base/observer_list.h"
+#include "ui/v2/public/view.h"
+
+namespace gfx {
+class Rect;
+}
+
+namespace v2 {
+
+class ViewObserver;
+
+// Friend of View. Provides a way to access view state for the implementation
+// of class View.
+class ViewPrivate {
+ public:
+  explicit ViewPrivate(View* view);
+  ~ViewPrivate();
+
+  ObserverList<ViewObserver>* observers() { return &view_->observers_; }
+
+  void ClearParent() { view_->parent_ = NULL; }
+
+  void set_bounds(const gfx::Rect& bounds) { view_->bounds_ = bounds; }
+
+ private:
+  View* view_;
+
+  DISALLOW_COPY_AND_ASSIGN(ViewPrivate);
+};
+
+}  // namespace v2
+
+#endif  // UI_V2_SRC_VIEW_PRIVATE_H_
diff --git a/ui/v2/src/view_unittest.cc b/ui/v2/src/view_unittest.cc
index 73a3837..fa93ca5 100644
--- a/ui/v2/src/view_unittest.cc
+++ b/ui/v2/src/view_unittest.cc
@@ -29,6 +29,17 @@ TEST_F(ViewTest, RemoveChild) {
   EXPECT_EQ(0U, v1.children().size());
 }
 
+TEST_F(ViewTest, Reparent) {
+  View v1;
+  View v2;
+  View* v11 = new View;
+  v1.AddChild(v11);
+  EXPECT_EQ(1U, v1.children().size());
+  v2.AddChild(v11);
+  EXPECT_EQ(1U, v2.children().size());
+  EXPECT_EQ(0U, v1.children().size());
+}
+
 TEST_F(ViewTest, Contains) {
   View v1;
 
@@ -122,6 +133,7 @@ class TreeChangeObserver : public ViewObserver {
   DISALLOW_COPY_AND_ASSIGN(TreeChangeObserver);
 };
 
+// Adds/Removes v11 to v1.
 TEST_F(ViewObserverTest, TreeChange_SimpleAddRemove) {
   View v1;
   TreeChangeObserver o1(&v1);
@@ -178,6 +190,13 @@ TEST_F(ViewObserverTest, TreeChange_SimpleAddRemove) {
   EXPECT_TRUE(TreeChangeParamsMatch(p11, o11.received_params().back()));
 }
 
+// Creates these two trees:
+// v1
+//  +- v11
+// v111
+//  +- v1111
+//  +- v1112
+// Then adds/removes v111 from v11.
 TEST_F(ViewObserverTest, TreeChange_NestedAddRemove) {
   View v1, v11, v111, v1111, v1112;
 
@@ -238,6 +257,191 @@ TEST_F(ViewObserverTest, TreeChange_NestedAddRemove) {
   EXPECT_TRUE(TreeChangeParamsMatch(p1112, o1112.received_params().back()));
 
   // Remove.
+  o1.Reset();
+  o11.Reset();
+  o111.Reset();
+  o1111.Reset();
+  o1112.Reset();
+  EXPECT_TRUE(o1.received_params().empty());
+  EXPECT_TRUE(o11.received_params().empty());
+  EXPECT_TRUE(o111.received_params().empty());
+  EXPECT_TRUE(o1111.received_params().empty());
+  EXPECT_TRUE(o1112.received_params().empty());
+
+  v11.RemoveChild(&v111);
+
+  EXPECT_EQ(1U, o1.received_params().size());
+  p1.target = &v111;
+  p1.receiver = &v1;
+  p1.old_parent = &v11;
+  p1.new_parent = NULL;
+  p1.phase = ViewObserver::DISPOSITION_CHANGING;
+  EXPECT_TRUE(TreeChangeParamsMatch(p1, o1.received_params().back()));
+
+  EXPECT_EQ(1U, o11.received_params().size());
+  p11 = p1;
+  p11.receiver = &v11;
+  EXPECT_TRUE(TreeChangeParamsMatch(p11, o11.received_params().back()));
+
+  EXPECT_EQ(2U, o111.received_params().size());
+  p111 = p11;
+  p111.receiver = &v111;
+  p111.phase = ViewObserver::DISPOSITION_CHANGING;
+  EXPECT_TRUE(TreeChangeParamsMatch(p111, o111.received_params().front()));
+  p111.phase = ViewObserver::DISPOSITION_CHANGED;
+  EXPECT_TRUE(TreeChangeParamsMatch(p111, o111.received_params().back()));
+
+  EXPECT_EQ(2U, o1111.received_params().size());
+  p1111 = p111;
+  p1111.receiver = &v1111;
+  p1111.phase = ViewObserver::DISPOSITION_CHANGING;
+  EXPECT_TRUE(TreeChangeParamsMatch(p1111, o1111.received_params().front()));
+  p1111.phase = ViewObserver::DISPOSITION_CHANGED;
+  EXPECT_TRUE(TreeChangeParamsMatch(p1111, o1111.received_params().back()));
+
+  EXPECT_EQ(2U, o1112.received_params().size());
+  p1112 = p111;
+  p1112.receiver = &v1112;
+  p1112.phase = ViewObserver::DISPOSITION_CHANGING;
+  EXPECT_TRUE(TreeChangeParamsMatch(p1112, o1112.received_params().front()));
+  p1112.phase = ViewObserver::DISPOSITION_CHANGED;
+  EXPECT_TRUE(TreeChangeParamsMatch(p1112, o1112.received_params().back()));
+}
+
+TEST_F(ViewObserverTest, TreeChange_Reparent) {
+  View v1, v11, v12, v111;
+  v11.set_owned_by_parent(false);
+  v111.set_owned_by_parent(false);
+  v12.set_owned_by_parent(false);
+  v1.AddChild(&v11);
+  v1.AddChild(&v12);
+  v11.AddChild(&v111);
+
+  TreeChangeObserver o1(&v1), o11(&v11), o12(&v12), o111(&v111);
+
+  // Reparent.
+  v12.AddChild(&v111);
+
+  // v1 (root) should see both changing and changed notifications.
+  EXPECT_EQ(2U, o1.received_params().size());
+  ViewObserver::TreeChangeParams p1;
+  p1.target = &v111;
+  p1.receiver = &v1;
+  p1.old_parent = &v11;
+  p1.new_parent = &v12;
+  p1.phase = ViewObserver::DISPOSITION_CHANGING;
+  EXPECT_TRUE(TreeChangeParamsMatch(p1, o1.received_params().front()));
+  p1.phase = ViewObserver::DISPOSITION_CHANGED;
+  EXPECT_TRUE(TreeChangeParamsMatch(p1, o1.received_params().back()));
+
+  // v11 should see changing notifications.
+  EXPECT_EQ(1U, o11.received_params().size());
+  ViewObserver::TreeChangeParams p11;
+  p11 = p1;
+  p11.receiver = &v11;
+  p11.phase = ViewObserver::DISPOSITION_CHANGING;
+  EXPECT_TRUE(TreeChangeParamsMatch(p11, o11.received_params().back()));
+
+  // v12 should see changed notifications.
+  EXPECT_EQ(1U, o12.received_params().size());
+  ViewObserver::TreeChangeParams p12;
+  p12 = p1;
+  p12.receiver = &v12;
+  p12.phase = ViewObserver::DISPOSITION_CHANGED;
+  EXPECT_TRUE(TreeChangeParamsMatch(p12, o12.received_params().back()));
+
+  // v111 should see both changing and changed notifications.
+  EXPECT_EQ(2U, o111.received_params().size());
+  ViewObserver::TreeChangeParams p111;
+  p111 = p1;
+  p111.receiver = &v111;
+  p111.phase = ViewObserver::DISPOSITION_CHANGING;
+  EXPECT_TRUE(TreeChangeParamsMatch(p111, o111.received_params().front()));
+  p111.phase = ViewObserver::DISPOSITION_CHANGED;
+  EXPECT_TRUE(TreeChangeParamsMatch(p111, o111.received_params().back()));
+}
+
+class VisibilityObserver : public ViewObserver {
+ public:
+  typedef std::pair<ViewObserver::DispositionChangePhase, bool> LogEntry;
+  typedef std::vector<LogEntry> LogEntries;
+
+  VisibilityObserver(View* view) : view_(view) {
+    view_->AddObserver(this);
+  }
+  virtual ~VisibilityObserver() {
+    view_->RemoveObserver(this);
+  }
+
+  const LogEntries& log_entries() const { return log_entries_; }
+
+ private:
+  // Overridden from ViewObserver:
+  virtual void OnViewVisibilityChange(
+      View* view,
+      ViewObserver::DispositionChangePhase phase) OVERRIDE {
+    DCHECK_EQ(view_, view);
+    log_entries_.push_back(std::make_pair(phase, view->visible()));
+  }
+
+  View* view_;
+  LogEntries log_entries_;
+
+  DISALLOW_COPY_AND_ASSIGN(VisibilityObserver);
+};
+
+TEST_F(ViewObserverTest, VisibilityChange) {
+  View v1;  // Starts out visible.
+
+  VisibilityObserver o1(&v1);
+
+  v1.SetVisible(false);
+
+  EXPECT_EQ(2U, o1.log_entries().size());
+  EXPECT_EQ(ViewObserver::DISPOSITION_CHANGING, o1.log_entries().front().first);
+  EXPECT_EQ(o1.log_entries().front().second, true);
+  EXPECT_EQ(ViewObserver::DISPOSITION_CHANGED, o1.log_entries().back().first);
+  EXPECT_EQ(o1.log_entries().back().second, false);
+}
+
+class BoundsObserver : public ViewObserver {
+ public:
+  typedef std::pair<gfx::Rect, gfx::Rect> BoundsChange;
+  typedef std::vector<BoundsChange> BoundsChanges;
+
+  explicit BoundsObserver(View* view) : view_(view) {
+    view_->AddObserver(this);
+  }
+  virtual ~BoundsObserver() {
+    view_->RemoveObserver(this);
+  }
+
+  const BoundsChanges& bounds_changes() const { return bounds_changes_; }
+
+ private:
+  virtual void OnViewBoundsChanged(View* view,
+                                  const gfx::Rect& old_bounds,
+                                  const gfx::Rect& new_bounds) OVERRIDE {
+    DCHECK_EQ(view_, view);
+    bounds_changes_.push_back(std::make_pair(old_bounds, new_bounds));
+  }
+
+  View* view_;
+  BoundsChanges bounds_changes_;
+
+  DISALLOW_COPY_AND_ASSIGN(BoundsObserver);
+};
+
+TEST_F(ViewObserverTest, BoundsChanged) {
+  View v1;
+  BoundsObserver o1(&v1);
+
+  gfx::Rect new_bounds(0, 0, 10, 10);
+
+  v1.SetBounds(new_bounds);
+  EXPECT_EQ(1U, o1.bounds_changes().size());
+  EXPECT_EQ(gfx::Rect(), o1.bounds_changes().front().first);
+  EXPECT_EQ(new_bounds, o1.bounds_changes().front().second);
 }
 
 }  // namespace v2
diff --git a/ui/v2/v2.gyp b/ui/v2/v2.gyp
index 9f1c6fe..df454ba 100644
--- a/ui/v2/v2.gyp
+++ b/ui/v2/v2.gyp
@@ -42,6 +42,8 @@
         'src/paint_processor.cc',
         'src/view.cc',
         'src/view_observer.cc',
+        'src/view_private.cc',
+        'src/view_private.h',
         'src/window.cc',
       ],
     },
@@ -51,6 +53,7 @@
       'dependencies': [
         '../../base/base.gyp:test_support_base',
         '../../testing/gtest.gyp:gtest',
+        '../gfx/gfx.gyp:gfx',
         'v2',
       ],
       'sources': [