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

content/browser/renderer_host/render_widget_host_view_aura.cc

Index: content/browser/renderer_host/render_widget_host_view_aura.cc
diff --git a/content/browser/renderer_host/render_widget_host_view_aura.cc b/content/browser/renderer_host/render_widget_host_view_aura.cc
index b4a1bc62abaf62b7195a822e272d31a2f9467576..dc8ff0a896314ed5f09cbc30e701c112d7de7d9a 100644
--- a/content/browser/renderer_host/render_widget_host_view_aura.cc
+++ b/content/browser/renderer_host/render_widget_host_view_aura.cc
@@ -322,18 +322,20 @@ void RenderWidgetHostViewAura::InitAsPopup(
   window_->Init(ui::LAYER_TEXTURED);
   window_->SetName("RenderWidgetHostViewAura");
 
-  aura::Window* parent = NULL;
   aura::RootWindow* root = popup_parent_host_view_->window_->GetRootWindow();
+
+  // TODO(erg): While I could make sure details of the StackingClient are
+  // hidden behind aura, hiding the details of the ScreenPositionClient will
+  // take another effort.
+  gfx::Point origin_in_screen(bounds_in_display.origin());

[oshima, 2012/11/26 21:24:46]

bounds_in_display is in screen coordinates, so you shouldn't have to convert it
to screen. 

SetBounds, on the other hand, takes the bounds in the parent's coordinates,
so you need to convert it to the parent's coordinate.

Please see https://chromiumcodereview.appspot.com/11299119.

   aura::client::ScreenPositionClient* screen_position_client =
       aura::client::GetScreenPositionClient(root);
-  if (screen_position_client) {
-    gfx::Point origin_in_screen(bounds_in_display.origin());
+  if (screen_position_client)
     screen_position_client->ConvertPointToScreen(root, &origin_in_screen);
-    parent = aura::client::GetStackingClient()->GetDefaultParent(
-        window_, window_,
-        gfx::Rect(origin_in_screen, bounds_in_display.size()));
-  }
-  window_->SetParent(parent);
+
+  window_->SetDefaultParentByRoot(

[oshima, 2012/11/26 21:24:46]

Does this take screen coordinate? If so, SetDefaultParentByScreen or
SetDefaultParentInScreen would be better. "Root" can mean aura's root, ash's
root or platform's root which are all different unfortunately.

[Elliot Glaysher, 2012/11/26 23:02:30]

Root in this case is referring to RootWindow, which is the important parameter.
The bounds are optional, even in ash most places. This is one of the few places
where we previously passed in some set of bounds.

A question: Once I plumb aura::RootWindow context through the rest of ash and
views, will it be safe to remove this piece of location context? Are there cases
where a window is spawned from one RootWindow onto another by coordinates?

[oshima, 2012/11/27 00:14:30]

I see, my mistake.
We still need it because there is no context when you restore window with the
stored location, and you can open a window at given location using
window.open().
(This looks broken now though)

+      root,
+      gfx::Rect(origin_in_screen, bounds_in_display.size()));
   SetBounds(bounds_in_display);
   Show();
 }
@@ -345,7 +347,9 @@ void RenderWidgetHostViewAura::InitAsFullscreen(
   window_->Init(ui::LAYER_TEXTURED);
   window_->SetName("RenderWidgetHostViewAura");
   window_->SetProperty(aura::client::kShowStateKey, ui::SHOW_STATE_FULLSCREEN);
-  aura::Window* parent = NULL;
+
+  aura::RootWindow* parent = NULL;
+  gfx::Rect bounds;
   if (reference_host_view) {
     aura::Window* reference_window =
         static_cast<RenderWidgetHostViewAura*>(reference_host_view)->window_;
@@ -355,13 +359,11 @@ void RenderWidgetHostViewAura::InitAsFullscreen(
     }
     gfx::Display display = gfx::Screen::GetScreenFor(window_)->
         GetDisplayNearestWindow(reference_window);
-    aura::client::StackingClient* stacking_client =
-        aura::client::GetStackingClient();
-    if (stacking_client)
-      parent = stacking_client->GetDefaultParent(
-          window_, window_, display.bounds());
+    parent = reference_window->GetRootWindow();
+    bounds = display.bounds();
   }
-  window_->SetParent(parent);
+  window_->SetDefaultParentByRoot(parent, bounds);
+
   Show();
   Focus();
 }