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

ui/views/widget/native_widget_aura.cc

 // Copyright (c) 2012 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/views/widget/native_widget_aura.h"
 
 #include "base/bind.h"
 #include "base/string_util.h"
 #include "third_party/skia/include/core/SkRegion.h"
 #include "ui/aura/client/activation_change_observer.h"
@@ skipping 90 matching lines @@
   if (params.type == Widget::InitParams::TYPE_BUBBLE)
     aura::client::SetHideOnDeactivate(window_, true);
   window_->SetTransparent(params.transparent);
   window_->Init(params.layer_type);
   if (params.type == Widget::InitParams::TYPE_CONTROL)
     window_->Show();
 
   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()) {
       // 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.
       gfx::Rect bounds = gfx::Screen::GetScreenFor(parent)->
           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
   // state/bounds).
   if (IsMaximized())
     SetRestoreBounds(window_, window_bounds);
   else
     SetBounds(window_bounds);
   window_->set_ignore_events(!params.accept_events);
   can_activate_ =
@@ skipping 867 matching lines @@
   Widget::Widgets widgets;
   GetAllChildWidgets(native_view, &widgets);
 
   // First notify all the widgets that they are being disassociated
   // from their previous parent.
   for (Widget::Widgets::iterator it = widgets.begin();
       it != widgets.end(); ++it) {
     (*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();
       it != widgets.end(); ++it) {
     (*it)->NotifyNativeViewHierarchyChanged(true, new_parent);
   }
 }
 
 // static
 bool NativeWidgetPrivate::IsMouseButtonDown() {
   return aura::Env::GetInstance()->is_mouse_button_down();
 }
 
 // static
 bool NativeWidgetPrivate::IsTouchDown() {
   return aura::Env::GetInstance()->is_touch_down();
 }
 
 }  // namespace internal
 }  // namespace views