Correctly update the popup window position

It was updating the child window instead.
This CL hooks up the bounds change to the desktop widget correctly.

BUG=612270
TEST=fixed test harness to do the correct check.

Review-Url: https://codereview.chromium.org/2583873002
Cr-Commit-Position: refs/heads/master@{#442741}

content/browser/renderer_host/render_widget_host_view_aura.cc

@@ -581,6 +581,10 @@ void RenderWidgetHostViewAura::Hide() {
 #endif
 }
 
+aura::Window* RenderWidgetHostViewAura::GetToplevelWindow() {
+  return window_->GetToplevelWindow();
+}
+
 void RenderWidgetHostViewAura::SetSize(const gfx::Size& size) {
   // For a SetSize operation, we don't care what coordinate system the origin
   // of the window is in, it's only important to make sure that the origin
@@ -589,21 +593,12 @@ void RenderWidgetHostViewAura::SetSize(const gfx::Size& size) {
 }
 
 void RenderWidgetHostViewAura::SetBounds(const gfx::Rect& rect) {
-  gfx::Point relative_origin(rect.origin());
-
-  // RenderWidgetHostViewAura::SetBounds() takes screen coordinates, but
-  // Window::SetBounds() takes parent coordinates, so do the conversion here.
-  aura::Window* root = window_->GetRootWindow();
-  if (root) {
-    aura::client::ScreenPositionClient* screen_position_client =
-        aura::client::GetScreenPositionClient(root);
-    if (screen_position_client) {
-      screen_position_client->ConvertPointFromScreen(
-          window_->parent(), &relative_origin);
-    }
-  }
-
-  InternalSetBounds(gfx::Rect(relative_origin, rect.size()));
+  display::Display display =
+      popup_parent_host_view_
+          ? display::Screen::GetScreen()->GetDisplayNearestWindow(
+                popup_parent_host_view_->window_)
+          : display::Screen::GetScreen()->GetDisplayMatching(rect);
+  GetToplevelWindow()->SetBoundsInScreen(rect, display);
 }
 
 gfx::Vector2dF RenderWidgetHostViewAura::GetLastScrollOffset() const {

content/browser/renderer_host/render_widget_host_view_aura.h

@@ -332,6 +332,9 @@ class CONTENT_EXPORT RenderWidgetHostViewAura
     return delegated_frame_host_.get();
   }
 
+  // Returns the top level window that is hosting the renderwidget.
+  virtual aura::Window* GetToplevelWindow();
+
  private:
   friend class DelegatedFrameHostClientAura;
   friend class InputMethodAuraTestBase;

content/browser/renderer_host/render_widget_host_view_aura_unittest.cc

@@ -430,6 +430,10 @@ class FakeRenderWidgetHostViewAura : public RenderWidgetHostViewAura {
     return event_handler()->pointer_state();
   }
 
+  // In this unit test, |window_| is directly added to the root and is
+  // toplevel.
+  aura::Window* GetToplevelWindow() override { return window(); }
+
   gfx::Size last_frame_size_;
   std::unique_ptr<cc::CopyOutputRequest> last_copy_request_;
   FakeWindowEventDispatcher* dispatcher_;
@@ -517,6 +521,20 @@ const WebInputEvent* GetInputEventFromMessage(const IPC::Message& message) {
   return reinterpret_cast<const WebInputEvent*>(data);
 }
 
+class MockRenderWidgetHostViewAura : public RenderWidgetHostViewAura {
+ public:
+  MockRenderWidgetHostViewAura(RenderWidgetHost* host, bool is_guest_view_hack)
+      : RenderWidgetHostViewAura(host, is_guest_view_hack) {}
+
+  ~MockRenderWidgetHostViewAura() override {}
+
+ protected:
+  aura::Window* GetToplevelWindow() override { return window(); }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MockRenderWidgetHostViewAura);
+};
+
 }  // namespace
 
 class RenderWidgetHostViewAuraTest : public testing::Test {
@@ -542,6 +560,8 @@ class RenderWidgetHostViewAuraTest : public testing::Test {
         ImageTransportFactory::GetInstance()->GetContextFactory(),
         ImageTransportFactory::GetInstance()->GetContextFactoryPrivate());
     new wm::DefaultActivationClient(aura_test_helper_->root_window());
+    aura::client::SetScreenPositionClient(aura_test_helper_->root_window(),
+                                          &screen_position_client_);
 
     browser_context_.reset(new TestBrowserContext);
     process_host_ = new MockRenderProcessHost(browser_context_.get());
@@ -554,8 +574,8 @@ class RenderWidgetHostViewAuraTest : public testing::Test {
     parent_host_ = new RenderWidgetHostImpl(delegates_.back().get(),
                                             process_host_, routing_id, false);
     delegates_.back()->set_widget_host(parent_host_);
-    parent_view_ = new RenderWidgetHostViewAura(parent_host_,
-                                                is_guest_view_hack_);
+    parent_view_ =
+        new MockRenderWidgetHostViewAura(parent_host_, is_guest_view_hack_);
     parent_view_->InitAsChild(nullptr);
     aura::client::ParentWindowWithContext(parent_view_->GetNativeView(),
                                           aura_test_helper_->root_window(),
@@ -713,6 +733,7 @@ class RenderWidgetHostViewAuraTest : public testing::Test {
   std::unique_ptr<aura::test::AuraTestHelper> aura_test_helper_;
   std::unique_ptr<BrowserContext> browser_context_;
   std::vector<std::unique_ptr<MockRenderWidgetHostDelegate>> delegates_;
+  wm::DefaultScreenPositionClient screen_position_client_;
   MockRenderProcessHost* process_host_;
 
   // Tests should set these to nullptr if they've already triggered their
@@ -1002,11 +1023,9 @@ TEST_F(RenderWidgetHostViewAuraTest, FocusFullscreen) {
 // Checks that a popup is positioned correctly relative to its parent using
 // screen coordinates.
 TEST_F(RenderWidgetHostViewAuraTest, PositionChildPopup) {
-  wm::DefaultScreenPositionClient screen_position_client;
 
   aura::Window* window = parent_view_->GetNativeView();
   aura::Window* root = window->GetRootWindow();
-  aura::client::SetScreenPositionClient(root, &screen_position_client);
 
   parent_view_->SetBounds(gfx::Rect(10, 10, 800, 600));
   gfx::Rect bounds_in_screen = parent_view_->GetViewBounds();
@@ -4200,9 +4219,8 @@ class RenderWidgetHostViewAuraWithViewHarnessTest
     // the RWHVA as the view.
     delete contents()->GetRenderViewHost()->GetWidget()->GetView();
     // This instance is destroyed in the TearDown method below.
-    view_ = new RenderWidgetHostViewAura(
-        contents()->GetRenderViewHost()->GetWidget(),
-        false);
+    view_ = new MockRenderWidgetHostViewAura(
+        contents()->GetRenderViewHost()->GetWidget(), false);
   }
 
   void TearDown() override {

ui/aura/window.cc

@@ -313,8 +313,11 @@ void Window::SetBoundsInScreen(const gfx::Rect& new_bounds_in_screen,
   if (root) {
     aura::client::ScreenPositionClient* screen_position_client =
         aura::client::GetScreenPositionClient(root);
-    screen_position_client->SetBounds(this, new_bounds_in_screen, dst_display);
-    return;
+    if (screen_position_client) {
+      screen_position_client->SetBounds(this, new_bounds_in_screen,
+                                        dst_display);
+      return;
+    }
   }
   SetBounds(new_bounds_in_screen);
 }

ui/views/widget/desktop_aura/desktop_native_widget_aura_unittest.cc

@@ -614,6 +614,25 @@ TEST_F(WidgetTest, WindowMouseModalityTest) {
   top_level_widget.CloseNow();
 }
 
+TEST_F(WidgetTest, SetScreenBoundsOnNativeWindow) {
+  Widget widget;
+  Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_WINDOW);
+  params.native_widget =
+      CreatePlatformDesktopNativeWidgetImpl(params, &widget, nullptr);
+  params.ownership = Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
+  params.bounds = gfx::Rect(10, 10, 100, 100);
+  widget.Init(params);
+  widget.Show();
+  aura::Window* window = widget.GetNativeWindow();
+  display::Screen* screen = display::Screen::GetScreen();
+
+  const gfx::Rect new_screen_bounds(50, 50, 150, 150);
+  window->SetBoundsInScreen(new_screen_bounds,
+                            screen->GetPrimaryDisplay());
+  EXPECT_EQ(new_screen_bounds, widget.GetWindowBoundsInScreen());
+  widget.CloseNow();
+}
+
 #if defined(OS_WIN)
 // Tests whether we can activate the top level widget when a modal dialog is
 // active.

ui/views/widget/desktop_aura/desktop_screen_position_client.cc

@@ -38,10 +38,17 @@ void DesktopScreenPositionClient::SetBounds(aura::Window* window,
   // TODO(jam): Use the 3rd parameter, |display|.
   aura::Window* root = window->GetRootWindow();
 
-  // This method assumes that |window| does not have an associated
-  // DesktopNativeWidgetAura.
   internal::NativeWidgetPrivate* desktop_native_widget =
       DesktopNativeWidgetAura::ForWindow(root);
+  // The screen bounds request to the content_window_ should be interpreted as
+  // a widget bounds change.
+  if (desktop_native_widget &&
+      desktop_native_widget->GetNativeView() == window) {
+    desktop_native_widget->GetWidget()->SetBounds(bounds);
+    return;
+  }
+  // The following logic assumes that |window| does not have an associated
+  // DesktopNativeWidgetAura.
   DCHECK(!desktop_native_widget ||
          desktop_native_widget->GetNativeView() != window);