[go: up one dir, main page]

Fix popup window repositioning in Desktop Chrome.

We need to use Widget::SetBounds in place of Widget::SetSize as the position of the window
may have changed.

BUG=374705
TEST=Covered by views_unittest DesktopAuraWidgetTest.TopLevelOwnedPopupRepositionTest
TBR=ananta@chromium.org

Review URL: https://codereview.chromium.org/871813003

Cr-Commit-Position: refs/heads/master@{#313146}
(cherry picked from commit 0393576a8c18cb92aecdc78e417aee3df4ec5d94)

Review URL: https://codereview.chromium.org/928303002

Cr-Commit-Position: refs/branch-heads/2272@{#302}
Cr-Branched-From: 827a380cfdb31aa54c8d56e63ce2c3fd8c3ba4d4-refs/heads/master@{#310958}
diff --git a/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc b/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc
index 88c6090..c8a59ba5 100644
--- a/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc
+++ b/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc
@@ -145,8 +145,11 @@
   void OnWindowBoundsChanged(aura::Window* window,
                              const gfx::Rect& old_bounds,
                              const gfx::Rect& new_bounds) override {
+    // The position of the window may have changed. Hence we use SetBounds in
+    // place of SetSize. We need to pass the bounds in screen coordinates to
+    // the Widget::SetBounds function.
     if (top_level_widget_ && window == child_window_)
-      top_level_widget_->SetSize(new_bounds.size());
+      top_level_widget_->SetBounds(window->GetBoundsInScreen());
   }
 
  private:
diff --git a/ui/views/widget/desktop_aura/desktop_native_widget_aura_unittest.cc b/ui/views/widget/desktop_aura/desktop_native_widget_aura_unittest.cc
index 975a217..60eef52 100644
--- a/ui/views/widget/desktop_aura/desktop_native_widget_aura_unittest.cc
+++ b/ui/views/widget/desktop_aura/desktop_native_widget_aura_unittest.cc
@@ -12,6 +12,7 @@
 #include "ui/aura/window.h"
 #include "ui/aura/window_tree_host.h"
 #include "ui/events/test/event_generator.h"
+#include "ui/gfx/screen.h"
 #include "ui/views/test/views_test_base.h"
 #include "ui/views/test/widget_test.h"
 #include "ui/views/widget/widget.h"
@@ -249,7 +250,8 @@
       : top_level_widget_(NULL),
         owned_window_(NULL),
         owner_destroyed_(false),
-        owned_window_destroyed_(false) {}
+        owned_window_destroyed_(false),
+        use_async_mode_(true) {}
 
   ~DesktopAuraTopLevelWindowTest() override {
     EXPECT_TRUE(owner_destroyed_);
@@ -302,6 +304,13 @@
 
   void DestroyOwnedWindow() {
     ASSERT_TRUE(owned_window_ != NULL);
+    // If async mode is off then clean up state here.
+    if (!use_async_mode_) {
+      owned_window_->RemoveObserver(this);
+      owned_window_->parent()->RemoveObserver(this);
+      owner_destroyed_ = true;
+      owned_window_destroyed_ = true;
+    }
     delete owned_window_;
   }
 
@@ -329,6 +338,10 @@
     return top_level_widget_;
   }
 
+  void set_use_async_mode(bool async_mode) {
+    use_async_mode_ = async_mode;
+  }
+
  private:
   views::Widget widget_;
   views::Widget* top_level_widget_;
@@ -336,6 +349,9 @@
   bool owner_destroyed_;
   bool owned_window_destroyed_;
   aura::test::TestWindowDelegate child_window_delegate_;
+  // This flag controls whether we need to wait for the destruction to complete
+  // before finishing the test. Defaults to true.
+  bool use_async_mode_;
 
   DISALLOW_COPY_AND_ASSIGN(DesktopAuraTopLevelWindowTest);
 };
@@ -381,6 +397,9 @@
 TEST_F(DesktopAuraWidgetTest, TopLevelOwnedPopupResizeTest) {
   ViewsDelegate::views_delegate = NULL;
   DesktopAuraTopLevelWindowTest popup_window;
+
+  popup_window.set_use_async_mode(false);
+
   ASSERT_NO_FATAL_FAILURE(popup_window.CreateTopLevelWindow(
       gfx::Rect(0, 0, 200, 200), false));
 
@@ -389,9 +408,31 @@
 
   EXPECT_EQ(popup_window.top_level_widget()->GetNativeView()->bounds().size(),
             new_size.size());
-  RunPendingMessages();
+
   ASSERT_NO_FATAL_FAILURE(popup_window.DestroyOwnedWindow());
-  RunPendingMessages();
+}
+
+// This test validates that when a top level owned popup Aura window is
+// repositioned, the widget is repositioned as well.
+TEST_F(DesktopAuraWidgetTest, TopLevelOwnedPopupRepositionTest) {
+  ViewsDelegate::views_delegate = NULL;
+  DesktopAuraTopLevelWindowTest popup_window;
+
+  popup_window.set_use_async_mode(false);
+
+  ASSERT_NO_FATAL_FAILURE(popup_window.CreateTopLevelWindow(
+      gfx::Rect(0, 0, 200, 200), false));
+
+  gfx::Rect new_pos(10, 10, 400, 400);
+  popup_window.owned_window()->SetBoundsInScreen(
+      new_pos,
+      gfx::Screen::GetScreenFor(
+          popup_window.owned_window())->GetDisplayNearestPoint(gfx::Point()));
+
+  EXPECT_EQ(new_pos,
+            popup_window.top_level_widget()->GetWindowBoundsInScreen());
+
+  ASSERT_NO_FATAL_FAILURE(popup_window.DestroyOwnedWindow());
 }
 
 }  // namespace test