views: GetRestoreBounds shouldn't be affected by window transform.

Some cros windows have a transform applied to them when they're closed.
When reopened they are placed where the transformed window was. The
widget bounds saving uses Window::GetBoundsInScreen which returns bounds
that are affected by the transform on the window. This CL alters that.

Test: added test
Bug: 1109161
Change-Id: Ia577b2372fa78be4cbf1ae4fbe2236fca615dd19
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2373660
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819095}

ash/display/mirror_window_controller.cc

@@ -44,6 +44,7 @@ class MirroringScreenPositionClient
   explicit MirroringScreenPositionClient(MirrorWindowController* controller)
       : controller_(controller) {}
 
+  // aura::client::ScreenPositionClient:
   void ConvertPointToScreen(const aura::Window* window,
                             gfx::PointF* point) override {
     const aura::Window* root = window->GetRootWindow();
@@ -79,6 +80,16 @@ class MirroringScreenPositionClient
     NOTREACHED();
   }
 
+ protected:
+  // aura::client::ScreenPositionClient:
+  gfx::Point GetRootWindowOriginInScreen(
+      const aura::Window* root_window) override {
+    DCHECK(root_window->IsRootWindow());
+    const display::Display& display =
+        controller_->GetDisplayForRootWindow(root_window);
+    return display.bounds().origin();
+  }
+
  private:
   MirrorWindowController* controller_;  // not owned.
 

ash/display/screen_position_controller.cc

@@ -9,6 +9,7 @@
 #include "ash/wm/window_positioning_utils.h"
 #include "ash/wm/window_properties.h"
 #include "ash/wm/window_state.h"
+#include "ui/aura/client/screen_position_client.h"
 #include "ui/aura/window.h"
 #include "ui/aura/window_tree_host.h"
 #include "ui/display/display.h"
@@ -120,4 +121,13 @@ void ScreenPositionController::SetBounds(aura::Window* window,
   SetBoundsInScreen(window, bounds, display);
 }
 
+gfx::Point ScreenPositionController::GetRootWindowOriginInScreen(
+    const aura::Window* root_window) {
+  DCHECK(root_window->IsRootWindow());
+  const display::Display& display =
+      display::Screen::GetScreen()->GetDisplayNearestWindow(
+          const_cast<aura::Window*>(root_window));
+  return display.bounds().origin();
+}
+
 }  // namespace ash

ash/display/screen_position_controller.h

@@ -42,6 +42,11 @@ class ASH_EXPORT ScreenPositionController
                  const gfx::Rect& bounds,
                  const display::Display& display) override;
 
+ protected:
+  // aura::client::ScreenPositionClient:
+  gfx::Point GetRootWindowOriginInScreen(
+      const aura::Window* root_window) override;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(ScreenPositionController);
 };

extensions/shell/browser/root_window_controller.cc

@@ -82,7 +82,7 @@ class ScreenPositionClient : public wm::DefaultScreenPositionClient {
     aura::Window::ConvertPointToTarget(window->parent(), root_window, &origin);
 
     // Translate the origin by the root window's offset in screen coordinates.
-    gfx::Point host_origin = GetOriginInScreen(root_window);
+    gfx::Point host_origin = GetRootWindowOriginInScreen(root_window);
     origin.Offset(-host_origin.x(), -host_origin.y());
     window->SetBounds(gfx::Rect(origin, bounds.size()));
   }

ui/aura/BUILD.gn

@@ -322,6 +322,7 @@ test("aura_unittests") {
 
   sources = [
     "../compositor_extra/shadow_unittest.cc",
+    "client/screen_position_client_unittest.cc",
     "gestures/gesture_recognizer_unittest.cc",
     "test/aura_test_suite.h",
     "test/run_all_unittests.cc",

ui/aura/client/screen_position_client.cc

@@ -30,6 +30,28 @@ void ScreenPositionClient::ConvertPointFromScreen(const Window* window,
   *point = gfx::ToFlooredPoint(point_float);
 }
 
+void ScreenPositionClient::ConvertPointToRootWindowIgnoringTransforms(
+    const Window* window,
+    gfx::Point* point) {
+  DCHECK(window);
+  DCHECK(window->GetRootWindow());
+  Window* ancestor = const_cast<Window*>(window);
+  while (ancestor && !ancestor->IsRootWindow()) {
+    const gfx::Point origin = ancestor->bounds().origin();
+    point->Offset(origin.x(), origin.y());
+    ancestor = ancestor->parent();
+  }
+}
+
+void ScreenPositionClient::ConvertPointToScreenIgnoringTransforms(
+    const aura::Window* window,
+    gfx::Point* point) {
+  const aura::Window* root_window = window->GetRootWindow();
+  ConvertPointToRootWindowIgnoringTransforms(window, point);
+  gfx::Point origin = GetRootWindowOriginInScreen(root_window);
+  point->Offset(origin.x(), origin.y());
+}
+
 void SetScreenPositionClient(Window* root_window,
                              ScreenPositionClient* client) {
   DCHECK_EQ(root_window->GetRootWindow(), root_window);

ui/aura/client/screen_position_client.h

@@ -45,6 +45,18 @@ class AURA_EXPORT ScreenPositionClient {
   virtual void SetBounds(Window* window,
                          const gfx::Rect& bounds,
                          const display::Display& display) = 0;
+  // Converts |point| from |window|'s coordinate space into screen coordinate
+  // space. Ignores any transforms that may be applied on |window| or its window
+  // hieraichy.
+  void ConvertPointToScreenIgnoringTransforms(const Window* window,
+                                              gfx::Point* point);
+  void ConvertPointToRootWindowIgnoringTransforms(const Window* window,
+                                                  gfx::Point* point);
+
+ protected:
+  // Returns the origin of the host platform-window in system DIP coordinates.
+  virtual gfx::Point GetRootWindowOriginInScreen(
+      const aura::Window* root_window) = 0;
 };
 
 // Sets/Gets the activation client on the Window.

ui/aura/client/screen_position_client_unittest.cc

+// Copyright 2020 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/aura/client/screen_position_client.h"
+
+#include <memory>
+
+#include "ui/aura/test/aura_test_base.h"
+
+namespace aura {
+namespace client {
+
+using ScreenPositionClientTest = test::AuraTestBase;
+
+class TestScreenPositionClient : public ScreenPositionClient {
+ public:
+  TestScreenPositionClient() = default;
+  TestScreenPositionClient(const TestScreenPositionClient&) = delete;
+  TestScreenPositionClient& operator=(const TestScreenPositionClient&) = delete;
+  ~TestScreenPositionClient() override = default;
+
+  // ScreenPositionClient:
+  void ConvertPointToScreen(const Window* window, gfx::PointF* point) override {
+  }
+  void ConvertPointFromScreen(const Window* window,
+                              gfx::PointF* point) override {}
+  void ConvertHostPointToScreen(Window* root_window,
+                                gfx::Point* point) override {}
+  void SetBounds(Window* window,
+                 const gfx::Rect& bounds,
+                 const display::Display& display) override {}
+
+ protected:
+  // ScreenPositionClient:
+  gfx::Point GetRootWindowOriginInScreen(
+      const aura::Window* root_window) override {
+    return gfx::Point();
+  }
+};
+
+TEST_F(ScreenPositionClientTest, ConvertPointToRootWindowIgnoringTransforms) {
+  std::unique_ptr<Window> parent(CreateNormalWindow(1, root_window(), nullptr));
+  std::unique_ptr<Window> child(CreateNormalWindow(2, parent.get(), nullptr));
+
+  parent->SetBounds(gfx::Rect(50, 50, 200, 200));
+  child->SetBounds(gfx::Rect(150, 150, 200, 200));
+
+  TestScreenPositionClient test_client;
+  gfx::Point point(100, 100);
+  test_client.ConvertPointToRootWindowIgnoringTransforms(parent.get(), &point);
+  EXPECT_EQ(gfx::Point(150, 150), point);
+
+  point = gfx::Point(100, 100);
+  test_client.ConvertPointToRootWindowIgnoringTransforms(child.get(), &point);
+  EXPECT_EQ(gfx::Point(300, 300), point);
+
+  point = gfx::Point(100, 100);
+  child->SetTransform(gfx::Transform(1, 0, 0, 1, 100, 100));
+  test_client.ConvertPointToRootWindowIgnoringTransforms(child.get(), &point);
+  EXPECT_EQ(gfx::Point(300, 300), point);
+}
+
+}  // namespace client
+}  // namespace aura

ui/views/widget/desktop_aura/desktop_screen_position_client.cc

@@ -46,7 +46,7 @@ void DesktopScreenPositionClient::SetBounds(aura::Window* window,
     gfx::Point origin = bounds.origin();
     aura::Window::ConvertPointToTarget(window->parent(), root, &origin);
 
-    gfx::Point host_origin = GetOriginInScreen(root);
+    gfx::Point host_origin = GetRootWindowOriginInScreen(root);
     origin.Offset(-host_origin.x(), -host_origin.y());
     window->SetBounds(gfx::Rect(origin, bounds.size()));
     return;

ui/views/widget/native_widget_aura.cc

@@ -472,6 +472,21 @@ gfx::Rect NativeWidgetAura::GetRestoredBounds() const {
     if (restore_bounds)
       return *restore_bounds;
   }
+
+  // Prefer getting the window bounds and converting them to screen bounds since
+  // Window::GetBoundsInScreen takes into the account the window transform.
+  auto* screen_position_client =
+      aura::client::GetScreenPositionClient(window_->GetRootWindow());
+  if (screen_position_client) {
+    // |window_|'s bounds are in parent's coordinate system so use that when
+    // converting.
+    gfx::Rect bounds = window_->bounds();
+    gfx::Point origin = bounds.origin();
+    screen_position_client->ConvertPointToScreenIgnoringTransforms(
+        window_->parent(), &origin);
+    return gfx::Rect(origin, bounds.size());
+  }
+
   return window_->GetBoundsInScreen();
 }
 

ui/views/widget/native_widget_aura_unittest.cc

@@ -149,6 +149,25 @@ TEST_F(NativeWidgetAuraTest, CreateMinimized) {
   widget->CloseNow();
 }
 
+// Tests that GetRestoreBounds returns the window bounds even if the window is
+// transformed.
+TEST_F(NativeWidgetAuraTest, RestoreBounds) {
+  Widget::InitParams params(Widget::InitParams::TYPE_WINDOW);
+  params.ownership = Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
+  params.parent = nullptr;
+  params.context = root_window();
+  params.bounds.SetRect(0, 0, 400, 400);
+  auto widget = std::make_unique<Widget>();
+  widget->Init(std::move(params));
+  widget->Show();
+  EXPECT_EQ(gfx::Rect(400, 400), widget->GetRestoredBounds());
+
+  gfx::Transform transform;
+  transform.Translate(100.f, 100.f);
+  widget->GetNativeWindow()->SetTransform(transform);
+  EXPECT_EQ(gfx::Rect(400, 400), widget->GetRestoredBounds());
+}
+
 // A WindowObserver that counts kShowStateKey property changes.
 class TestWindowObserver : public aura::WindowObserver {
  public:

ui/wm/core/default_screen_position_client.cc

@@ -27,7 +27,7 @@ void DefaultScreenPositionClient::ConvertPointToScreen(
     gfx::PointF* point) {
   const aura::Window* root_window = window->GetRootWindow();
   aura::Window::ConvertPointToTarget(window, root_window, point);
-  gfx::Point origin = GetOriginInScreen(root_window);
+  gfx::Point origin = GetRootWindowOriginInScreen(root_window);
   point->Offset(origin.x(), origin.y());
 }
 
@@ -35,7 +35,7 @@ void DefaultScreenPositionClient::ConvertPointFromScreen(
     const aura::Window* window,
     gfx::PointF* point) {
   const aura::Window* root_window = window->GetRootWindow();
-  gfx::Point origin = GetOriginInScreen(root_window);
+  gfx::Point origin = GetRootWindowOriginInScreen(root_window);
   point->Offset(-origin.x(), -origin.y());
   aura::Window::ConvertPointToTarget(root_window, window, point);
 }
@@ -52,7 +52,7 @@ void DefaultScreenPositionClient::SetBounds(aura::Window* window,
   window->SetBounds(bounds);
 }
 
-gfx::Point DefaultScreenPositionClient::GetOriginInScreen(
+gfx::Point DefaultScreenPositionClient::GetRootWindowOriginInScreen(
     const aura::Window* root_window) {
   aura::Window* window = const_cast<aura::Window*>(root_window);
   display::Screen* screen = display::Screen::GetScreen();

ui/wm/core/default_screen_position_client.h

@@ -31,8 +31,9 @@ class WM_CORE_EXPORT DefaultScreenPositionClient
                  const display::Display& display) override;
 
  protected:
-  // Returns the origin of the host platform-window in system DIP coordinates.
-  virtual gfx::Point GetOriginInScreen(const aura::Window* root_window);
+  // aura::client::ScreenPositionClient:
+  gfx::Point GetRootWindowOriginInScreen(
+      const aura::Window* root_window) override;
 
  private:
   aura::Window* root_window_;