Desktop aura: Break aura::Window::SetParent in two.

ui/views/widget/native_widget_aura.cc

Index: ui/views/widget/native_widget_aura.cc
diff --git a/ui/views/widget/native_widget_aura.cc b/ui/views/widget/native_widget_aura.cc
index c9b9cfda763f18630d39b096a1ea475a8ee8cfe7..2bddce6acc41e413b80c0a3887693317d6aea08c 100644
--- a/ui/views/widget/native_widget_aura.cc
+++ b/ui/views/widget/native_widget_aura.cc
@@ -108,24 +108,18 @@ void NativeWidgetAura::InitNativeWidget(const Widget::InitParams& params) {
   delegate_->OnNativeWidgetCreated();
 
   gfx::Rect window_bounds = params.bounds;
-  if (params.child) {
-    window_->SetParent(params.GetParent());
-  } else {
+  gfx::NativeView parent = params.GetParent();
+  if (!params.child) {
     // Set up the transient child before the window is added. This way the
     // LayoutManager knows the window has a transient parent.
-    gfx::NativeView parent = params.GetParent();
     if (parent && parent->type() != aura::client::WINDOW_TYPE_UNKNOWN) {
       parent->AddTransientChild(window_);
       parent = NULL;
     }
     // SetAlwaysOnTop before SetParent so that always-on-top container is used.
     SetAlwaysOnTop(params.keep_on_top);
-    // If the parent is not specified, find the default parent for
-    // the |window_| using the desired |window_bounds|.
-    if (!parent) {
-      parent = aura::client::GetStackingClient(params.GetParent())->
-          GetDefaultParent(params.context, window_, window_bounds);
-    } else if (window_bounds == gfx::Rect()) {
+    // Make sure we have a real |window_bounds|.
+    if (window_bounds == gfx::Rect()) {
       // If a parent is specified but no bounds are given,
       // use the origin of the parent's display so that the widget
       // will be added to the same display as the parent.
@@ -133,7 +127,16 @@ void NativeWidgetAura::InitNativeWidget(const Widget::InitParams& params) {
           GetDisplayNearestWindow(parent).bounds();
       window_bounds.set_origin(bounds.origin());
     }
-    window_->SetParent(parent);
+  }
+
+  if (parent) {
+    parent->AddChild(window_);
+  } else {
+    // TODO(erg): Remove this NULL check once we've made everything in views
+    // actually pass us a context.
+    aura::RootWindow* root_window =
+        params.context ? params.context->GetRootWindow() : NULL;
+    window_->SetDefaultParentByRoot(root_window, window_bounds);
   }
 
   // Wait to set the bounds until we have a parent. That way we can know our
@@ -1022,7 +1025,18 @@ void NativeWidgetPrivate::ReparentNativeView(gfx::NativeView native_view,
     (*it)->NotifyNativeViewHierarchyChanged(false, previous_parent);
   }
 
-  native_view->SetParent(new_parent);
+  if (new_parent) {
+    new_parent->AddChild(native_view);
+  } else {
+    // I am unsure what we should really be doing here. The following looks
+    // weird, but it's the equivalent of what aura has always done. (The
+    // previous behaviour of aura::Window::SetParent() used NULL as a special
+    // value that meant ask the StackingClient where things should go.) jam@
+    // reasonably thought that was stupid and replaced it with a RemoveChild
+    // call, which broke things hard.
+    native_view->SetDefaultParentByRoot(native_view->GetRootWindow(),
+                                              gfx::Rect());
+  }
 
   // And now, notify them that they have a brand new parent.
   for (Widget::Widgets::iterator it = widgets.begin();