Correctly update the popup window position

ui/views/widget/desktop_aura/desktop_screen_position_client.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/desktop_aura/desktop_screen_position_client.h"
 
 #include "ui/views/widget/desktop_aura/desktop_native_widget_aura.h"
 
 namespace views {
 
@@ skipping 20 matching lines @@
 DesktopScreenPositionClient::~DesktopScreenPositionClient() {
   aura::client::SetScreenPositionClient(root_window_, NULL);
 }
 
 void DesktopScreenPositionClient::SetBounds(aura::Window* window,
                                             const gfx::Rect& bounds,
                                             const display::Display& display) {
   // 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;

[sadrul, 2016/12/19 18:08:56]

It looks like DesktopNativeWidgetAura::OnBoundsChanged() does not do anything. I
think it would be better if this happened there? That's how NativeWidgetAura
resizes the corresponding views::Widget, looks like, and it'd be good to follow
the same codepath for desktop too, instead of having this special case here.

[oshima, 2016/12/20 23:55:32]

Did you mean DesktopNativeWidgetTopLevelHandler::OnWindowBoundsChannged? I
looked into the code,
and I actually didn't understand how this is supposed to work. The bounds has to
be set here,
before being set on the window, otherwise the window will be moved inside the
widget, and
that's what seems to be happening. (The content_window's bounds is moved inside
root window)

Am I reading wrong?

+  }
+  // The following logic assumes that |window| does not have an associated
+  // DesktopNativeWidgetAura.

[Bret, 2016/12/21 00:04:25]

nit: since you're explicitly handling the dchecked case above, this
comment+dcheck is unnecessary.

   DCHECK(!desktop_native_widget ||
          desktop_native_widget->GetNativeView() != window);
 
   if (PositionWindowInScreenCoordinates(window)) {
     // The caller expects windows we consider "embedded" to be placed in the
     // screen coordinate system. So we need to offset the root window's
     // position (which is in screen coordinates) from these bounds.
 
     gfx::Point origin = bounds.origin();
     aura::Window::ConvertPointToTarget(window->parent(), root, &origin);
 
     gfx::Point host_origin = GetOriginInScreen(root);
     origin.Offset(-host_origin.x(), -host_origin.y());
     window->SetBounds(gfx::Rect(origin, bounds.size()));
     return;
   }
 
   window->SetBounds(bounds);
 }
 
 }  // namespace views