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

content/browser/renderer_host/render_widget_host_view_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 "content/browser/renderer_host/render_widget_host_view_aura.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
@@ skipping 304 matching lines @@
     // similar mechanism to ensure a second popup doesn't cause the first one
     // to never get a chance to filter events. See crbug.com/160589.
     DCHECK(old_child->popup_parent_host_view_ == popup_parent_host_view_);
     old_child->popup_parent_host_view_ = NULL;
   }
   popup_parent_host_view_->popup_child_host_view_ = this;
   window_->SetType(aura::client::WINDOW_TYPE_MENU);
   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();
 }
 
 void RenderWidgetHostViewAura::InitAsFullscreen(
     RenderWidgetHostView* reference_host_view) {
   is_fullscreen_ = true;
   window_->SetType(aura::client::WINDOW_TYPE_NORMAL);
   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_;
     if (reference_window) {
       host_tracker_.reset(new aura::WindowTracker);
       host_tracker_->Add(reference_window);
     }
     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();
 }
 
 RenderWidgetHost* RenderWidgetHostViewAura::GetRenderWidgetHost() const {
   return host_;
 }
 
 void RenderWidgetHostViewAura::WasShown() {
   if (!host_->is_hidden())
@@ skipping 1616 matching lines @@
     RenderWidgetHost* widget) {
   return new RenderWidgetHostViewAura(widget);
 }
 
 // static
 void RenderWidgetHostViewPort::GetDefaultScreenInfo(WebScreenInfo* results) {
   GetScreenInfoForWindow(results, NULL);
 }
 
 }  // namespace content