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..a43dadef7affd47dd9df4df94ead09e4da657540 100644
--- a/ui/views/widget/native_widget_aura.cc
+++ b/ui/views/widget/native_widget_aura.cc
@@ -108,12 +108,10 @@ 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;
@@ -133,8 +131,11 @@ void NativeWidgetAura::InitNativeWidget(const Widget::InitParams& params) {
           GetDisplayNearestWindow(parent).bounds();
       window_bounds.set_origin(bounds.origin());
     }
-    window_->SetParent(parent);
+  } else if (!parent) {
+    parent = aura::client::GetStackingClient(params.context)->

[Ben Goodger (Google), 2012/11/20 23:46:38]

is there a reason we can't just follow the pattern you use elsewhere of:

if (parent)
  SetParentTo()
else
  SetDefault..()

.. I'm OK with the SetDefault etc. function taking window bounds if that helps.

+             GetDefaultParent(params.context, window_, window_bounds);
   }
+  window_->SetParentTo(parent);
 
   // Wait to set the bounds until we have a parent. That way we can know our
   // true state/bounds (the LayoutManager may enforce a particular
@@ -1022,7 +1023,17 @@ void NativeWidgetPrivate::ReparentNativeView(gfx::NativeView native_view,
     (*it)->NotifyNativeViewHierarchyChanged(false, previous_parent);
   }
 
-  native_view->SetParent(new_parent);
+  if (new_parent) {
+    native_view->SetParentTo(new_parent);
+  } 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->SetDefaultParentByTargetRoot(native_view->GetRootWindow());
+  }
 
   // And now, notify them that they have a brand new parent.
   for (Widget::Widgets::iterator it = widgets.begin();