Makes Window not change focus and send out notifications when moving

to a new parent.

BUG=137342
TEST=covered by tests
R=ben@chromium.org


Review URL: https://chromiumcodereview.appspot.com/10831281

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

ui/aura/window.cc

@@ -349,10 +349,12 @@ void Window::StackChildBelow(Window* child, Window* target) {
 }
 
 void Window::AddChild(Window* child) {
+  RootWindow* old_root = child->GetRootWindow();
+
   DCHECK(std::find(children_.begin(), children_.end(), child) ==
       children_.end());
   if (child->parent())
-    child->parent()->RemoveChild(child);
+    child->parent()->RemoveChildImpl(child, this);
   child->parent_ = this;
 
   layer_->Add(child->layer_);
@@ -363,8 +365,8 @@ void Window::AddChild(Window* child) {
   FOR_EACH_OBSERVER(WindowObserver, observers_, OnWindowAdded(child));
   child->OnParentChanged();
 
-  RootWindow* root_window = child->GetRootWindow();
-  if (root_window) {
+  RootWindow* root_window = GetRootWindow();
+  if (root_window && old_root != root_window) {
     root_window->OnWindowAddedToRootWindow(child);
     child->NotifyAddedToRootWindow();
   }
@@ -389,26 +391,7 @@ void Window::RemoveTransientChild(Window* child) {
 }
 
 void Window::RemoveChild(Window* child) {
-  Windows::iterator i = std::find(children_.begin(), children_.end(), child);
-  DCHECK(i != children_.end());
-  if (layout_manager_.get())
-    layout_manager_->OnWillRemoveWindowFromLayout(child);
-  FOR_EACH_OBSERVER(WindowObserver, observers_, OnWillRemoveWindow(child));
-  RootWindow* root_window = child->GetRootWindow();
-  if (root_window) {
-    root_window->OnWindowRemovedFromRootWindow(child);
-    child->NotifyRemovingFromRootWindow();
-  }
-  child->parent_ = NULL;
-  // We should only remove the child's layer if the child still owns that layer.
-  // Someone else may have acquired ownership of it via AcquireLayer() and may
-  // expect the hierarchy to go unchanged as the Window is destroyed.
-  if (child->layer_owner_.get())
-    layer_->Remove(child->layer_);
-  children_.erase(i);
-  child->OnParentChanged();
-  if (layout_manager_.get())
-    layout_manager_->OnWindowRemovedFromLayout(child);
+  RemoveChildImpl(child, NULL);
 }
 
 bool Window::Contains(const Window* other) const {
@@ -776,6 +759,30 @@ Window* Window::GetWindowForPoint(const gfx::Point& local_point,
   return delegate_ ? this : NULL;
 }
 
+void Window::RemoveChildImpl(Window* child, Window* new_parent) {
+  Windows::iterator i = std::find(children_.begin(), children_.end(), child);
+  DCHECK(i != children_.end());
+  if (layout_manager_.get())
+    layout_manager_->OnWillRemoveWindowFromLayout(child);
+  FOR_EACH_OBSERVER(WindowObserver, observers_, OnWillRemoveWindow(child));
+  RootWindow* root_window = child->GetRootWindow();
+  if (root_window &&
+      (!new_parent || new_parent->GetRootWindow() != root_window)) {
+    root_window->OnWindowRemovedFromRootWindow(child);
+    child->NotifyRemovingFromRootWindow();
+  }
+  child->parent_ = NULL;
+  // We should only remove the child's layer if the child still owns that layer.
+  // Someone else may have acquired ownership of it via AcquireLayer() and may
+  // expect the hierarchy to go unchanged as the Window is destroyed.
+  if (child->layer_owner_.get())
+    layer_->Remove(child->layer_);
+  children_.erase(i);
+  child->OnParentChanged();
+  if (layout_manager_.get())
+    layout_manager_->OnWindowRemovedFromLayout(child);
+}
+
 void Window::OnParentChanged() {
   FOR_EACH_OBSERVER(
       WindowObserver, observers_, OnWindowParentChanged(this, parent_));

ui/aura/window.h

@@ -387,6 +387,11 @@ class AURA_EXPORT Window : public ui::LayerDelegate,
                             bool return_tightest,
                             bool for_event_handling);
 
+  // Implementation of RemoveChild(). If |child| is being removed as the result
+  // of an add, |new_parent| is the new parent |child| is going to be parented
+  // to.
+  void RemoveChildImpl(Window* child, Window* new_parent);
+
   // Called when this window's parent has changed.
   void OnParentChanged();
 

ui/aura/window_unittest.cc

@@ -8,6 +8,7 @@
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
+#include "base/string_number_conversions.h"
 #include "base/stringprintf.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/aura/client/capture_client.h"
@@ -2461,5 +2462,53 @@ TEST_F(WindowTest, DelegateNotifiedAsBoundsChangeInHiddenLayer) {
   EXPECT_NE("0,0 100x100", window->bounds().ToString());
 }
 
+namespace {
+
+// Used by AddChildNotifications to track notification counts.
+class AddChildNotificationsObserver : public WindowObserver {
+ public:
+  AddChildNotificationsObserver() : added_count_(0), removed_count_(0) {}
+
+  std::string CountStringAndReset() {
+    std::string result = base::IntToString(added_count_) + " " +
+        base::IntToString(removed_count_);
+    added_count_ = removed_count_ = 0;
+    return result;
+  }
+
+  // WindowObserver overrides:
+  virtual void OnWindowAddedToRootWindow(Window* window) OVERRIDE {
+    added_count_++;
+  }
+  virtual void OnWindowRemovingFromRootWindow(Window* window) OVERRIDE {
+    removed_count_++;
+  }
+
+ private:
+  int added_count_;
+  int removed_count_;
+
+  DISALLOW_COPY_AND_ASSIGN(AddChildNotificationsObserver);
+};
+
+}  // namespace
+
+// Assertions around when root window notifications are sent.
+TEST_F(WindowTest, AddChildNotifications) {
+  AddChildNotificationsObserver observer;
+  scoped_ptr<Window> w1(CreateTestWindowWithId(1, NULL));
+  scoped_ptr<Window> w2(CreateTestWindowWithId(1, NULL));
+  w2->AddObserver(&observer);
+  w2->Focus();
+  EXPECT_TRUE(w2->HasFocus());
+
+  // Move |w2| to be a child of |w1|.
+  w1->AddChild(w2.get());
+  // Sine we moved in the same root, observer shouldn't be notified.
+  EXPECT_EQ("0 0", observer.CountStringAndReset());
+  // |w2| should still have focus after moving.
+  EXPECT_TRUE(w2->HasFocus());
+}
+
 }  // namespace test
 }  // namespace aura