Fix stable insets.

It's regressed in crrev.com/c/1414170.  It's just plain wrong
but I don't remeber why I changed this way.

Bug: 960121
Test: Covered by unittest
Change-Id: I0448648c3d661b9a2a9a3b98104d949c2c5a5573
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1595562Reviewed-by: Eliot Courtney <edcourtney@chromium.org>
Commit-Queue: Mitsuru Oshima (slow - gardening) <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#658983}

ash/wm/work_area_insets.cc

@@ -71,9 +71,9 @@ gfx::Insets WorkAreaInsets::GetAccessibilityInsets() const {
 
 gfx::Rect WorkAreaInsets::ComputeStableWorkArea() const {
   return CalculateWorkAreaBounds(
-      GetAccessibilityInsets(), keyboard_occluded_bounds_,
+      GetAccessibilityInsets(),
       root_window_controller_->shelf()->GetIdealBounds(),
-      root_window_controller_->GetRootWindow());
+      keyboard_occluded_bounds_, root_window_controller_->GetRootWindow());
 }
 
 bool WorkAreaInsets::IsKeyboardShown() const {

components/exo/wayland/BUILD.gn

@@ -204,8 +204,8 @@ source_set("unit_tests") {
 
   if (is_chromeos) {
     sources += [
-      "server_util_unittest.cc",
       "zaura_shell_unittest.cc",
+      "zcr_remote_shell_unittest.cc",
     ]
     deps += [
       "//ash",

components/exo/wayland/server_util.cc

@@ -26,30 +26,6 @@ DEFINE_UI_CLASS_PROPERTY_KEY(wl_resource*, kSurfaceResourceKey, nullptr)
 // data offer object.
 DEFINE_UI_CLASS_PROPERTY_KEY(wl_resource*, kDataOfferResourceKey, nullptr)
 
-// Scale the |child_bounds| in such a way that if it should fill the
-// |parent_size|'s width/height, it returns the |parent_size_in_pixel|'s
-// width/height.
-gfx::Rect ScaleBoundsToPixelSnappedToParent(
-    const gfx::Size& parent_size_in_pixel,
-    const gfx::Size& parent_size,
-    float device_scale_factor,
-    const gfx::Rect& child_bounds) {
-  int right = child_bounds.right();
-  int bottom = child_bounds.bottom();
-
-  int new_x = gfx::ToRoundedInt(child_bounds.x() * device_scale_factor);
-  int new_y = gfx::ToRoundedInt(child_bounds.y() * device_scale_factor);
-
-  int new_right = right == parent_size.width()
-                      ? parent_size_in_pixel.width()
-                      : gfx::ToRoundedInt(right * device_scale_factor);
-
-  int new_bottom = bottom == parent_size.height()
-                       ? parent_size_in_pixel.height()
-                       : gfx::ToRoundedInt(bottom * device_scale_factor);
-  return gfx::Rect(new_x, new_y, new_right - new_x, new_bottom - new_y);
-}
-
 }  // namespace
 
 uint32_t TimeTicksToMilliseconds(base::TimeTicks ticks) {
@@ -77,54 +53,5 @@ void SetDataOfferResource(DataOffer* data_offer,
   data_offer->SetProperty(kDataOfferResourceKey, data_offer_resource);
 }
 
-// Create the insets make sure that work area will be within the chrome's
-// work area when converted to the pixel on client side.
-gfx::Insets GetAdjustedInsets(const display::Display& display) {
-  float scale = display.device_scale_factor();
-  gfx::Size size_in_pixel = display.GetSizeInPixel();
-  gfx::Rect work_area_in_display = display.work_area();
-  work_area_in_display.Offset(-display.bounds().x(), -display.bounds().y());
-  gfx::Rect work_area_in_pixel = ScaleBoundsToPixelSnappedToParent(
-      size_in_pixel, display.bounds().size(), scale, work_area_in_display);
-  gfx::Insets insets_in_pixel =
-      gfx::Rect(size_in_pixel).InsetsFrom(work_area_in_pixel);
-  return gfx::Insets(gfx::ToCeiledInt(insets_in_pixel.top() / scale),
-                     gfx::ToCeiledInt(insets_in_pixel.left() / scale),
-                     gfx::ToCeiledInt(insets_in_pixel.bottom() / scale),
-                     gfx::ToCeiledInt(insets_in_pixel.right() / scale));
-}
-
-gfx::Insets GetWorkAreaInsetsInClientPixel(
-    const display::Display& display,
-    float default_dsf,
-    const gfx::Size& size_in_client_pixel,
-    const gfx::Rect& work_area_in_dp) {
-  gfx::Rect work_area_in_display = display.work_area();
-  work_area_in_display.Offset(-display.bounds().x(), -display.bounds().y());
-  gfx::Rect work_area_in_client_pixel = ScaleBoundsToPixelSnappedToParent(
-      size_in_client_pixel, display.bounds().size(), default_dsf,
-      work_area_in_display);
-  gfx::Insets insets_in_client_pixel =
-      gfx::Rect(size_in_client_pixel).InsetsFrom(work_area_in_client_pixel);
-
-  // TODO(oshima): I think this is more conservative than necessary. The correct
-  // way is to use enclosed rect when converting the work area from dp to
-  // client pixel, but that led to weird buffer size in overlay detection.
-  // (crbug.com/920650). Investigate if we can fix it and use enclosed rect.
-  return gfx::Insets(
-      gfx::ToRoundedInt(
-          gfx::ToCeiledInt(insets_in_client_pixel.top() / default_dsf) *
-          default_dsf),
-      gfx::ToRoundedInt(
-          gfx::ToCeiledInt(insets_in_client_pixel.left() / default_dsf) *
-          default_dsf),
-      gfx::ToRoundedInt(
-          gfx::ToCeiledInt(insets_in_client_pixel.bottom() / default_dsf) *
-          default_dsf),
-      gfx::ToRoundedInt(
-          gfx::ToCeiledInt(insets_in_client_pixel.right() / default_dsf) *
-          default_dsf));
-}
-
 }  // namespace wayland
 }  // namespace exo

components/exo/wayland/server_util.h

@@ -16,16 +16,6 @@
 
 struct wl_resource;
 
-namespace display {
-class Display;
-}
-
-namespace gfx {
-class Insets;
-class Rect;
-class Size;
-}  // namespace gfx
-
 namespace exo {
 
 class DataOffer;
@@ -70,19 +60,6 @@ wl_resource* GetDataOfferResource(const DataOffer* data_offer);
 void SetDataOfferResource(DataOffer* data_offer,
                           wl_resource* data_offer_resource);
 
-// Create the insets make sure that work area will be within the chrome's
-// work area when converted to the pixel on client side.
-// This is deprecatd and replaced with |GetWorkAreaInsetsInClientPixel|.
-gfx::Insets GetAdjustedInsets(const display::Display& display);
-
-// Create the insets in client's pixel coordinates in such way that
-// work area will be within the chrome's work area.
-gfx::Insets GetWorkAreaInsetsInClientPixel(
-    const display::Display& display,
-    float default_dsf,
-    const gfx::Size& size_in_client_pixel,
-    const gfx::Rect& work_area_in_dp);
-
 }  // namespace wayland
 }  // namespace exo
 

components/exo/wayland/zcr_remote_shell.cc

@@ -104,16 +104,51 @@ double GetDefaultDeviceScaleFactor() {
   return WMHelper::GetInstance()->GetDefaultDeviceScaleFactor();
 }
 
-ash::ShelfLayoutManager* GetShelfLayoutManagerForDisplay(
-    const display::Display& display) {
-  auto* root = ash::Shell::GetRootWindowForDisplayId(display.id());
-  return ash::Shelf::ForWindow(root)->shelf_layout_manager();
+// Scale the |child_bounds| in such a way that if it should fill the
+// |parent_size|'s width/height, it returns the |parent_size_in_pixel|'s
+// width/height.
+gfx::Rect ScaleBoundsToPixelSnappedToParent(
+    const gfx::Size& parent_size_in_pixel,
+    const gfx::Size& parent_size,
+    float device_scale_factor,
+    const gfx::Rect& child_bounds) {
+  int right = child_bounds.right();
+  int bottom = child_bounds.bottom();
+
+  int new_x = gfx::ToRoundedInt(child_bounds.x() * device_scale_factor);
+  int new_y = gfx::ToRoundedInt(child_bounds.y() * device_scale_factor);
+
+  int new_right = right == parent_size.width()
+                      ? parent_size_in_pixel.width()
+                      : gfx::ToRoundedInt(right * device_scale_factor);
+
+  int new_bottom = bottom == parent_size.height()
+                       ? parent_size_in_pixel.height()
+                       : gfx::ToRoundedInt(bottom * device_scale_factor);
+  return gfx::Rect(new_x, new_y, new_right - new_x, new_bottom - new_y);
+}
+
+// Create the insets make sure that work area will be within the chrome's
+// work area when converted to the pixel on client side.
+gfx::Insets GetAdjustedInsets(const display::Display& display) {
+  float scale = display.device_scale_factor();
+  gfx::Size size_in_pixel = display.GetSizeInPixel();
+  gfx::Rect work_area_in_display = display.work_area();
+  work_area_in_display.Offset(-display.bounds().x(), -display.bounds().y());
+  gfx::Rect work_area_in_pixel = ScaleBoundsToPixelSnappedToParent(
+      size_in_pixel, display.bounds().size(), scale, work_area_in_display);
+  gfx::Insets insets_in_pixel =
+      gfx::Rect(size_in_pixel).InsetsFrom(work_area_in_pixel);
+  return gfx::Insets(gfx::ToCeiledInt(insets_in_pixel.top() / scale),
+                     gfx::ToCeiledInt(insets_in_pixel.left() / scale),
+                     gfx::ToCeiledInt(insets_in_pixel.bottom() / scale),
+                     gfx::ToCeiledInt(insets_in_pixel.right() / scale));
 }
 
-ash::WorkAreaInsets* GetWorkAreaInsetsForDisplay(
+ash::ShelfLayoutManager* GetShelfLayoutManagerForDisplay(
     const display::Display& display) {
   auto* root = ash::Shell::GetRootWindowForDisplayId(display.id());
-  return ash::WorkAreaInsets::ForWindow(root);
+  return ash::Shelf::ForWindow(root)->shelf_layout_manager();
 }
 
 int Component(uint32_t direction) {
@@ -751,9 +786,10 @@ class WaylandRemoteShell : public ash::TabletModeObserver,
             display, default_dsf, size_in_client_pixel, display.work_area());
 
         gfx::Insets stable_insets_in_client_pixel =
-            GetWorkAreaInsetsInClientPixel(
-                display, default_dsf, size_in_client_pixel,
-                GetWorkAreaInsetsForDisplay(display)->ComputeStableWorkArea());
+            GetWorkAreaInsetsInClientPixel(display, default_dsf,
+                                           size_in_client_pixel,
+                                           GetStableWorkArea(display));
+
         int systemui_visibility =
             shelf_layout_manager->visibility_state() == ash::SHELF_AUTO_HIDE
                 ? ZCR_REMOTE_SURFACE_V1_SYSTEMUI_VISIBILITY_STATE_AUTOHIDE_NON_STICKY
@@ -1082,5 +1118,44 @@ void bind_remote_shell(wl_client* client,
                         static_cast<Display*>(data), resource));
 }
 
+gfx::Insets GetWorkAreaInsetsInClientPixel(
+    const display::Display& display,
+    float default_dsf,
+    const gfx::Size& size_in_client_pixel,
+    const gfx::Rect& work_area_in_dp) {
+  gfx::Rect work_area_in_display = display.work_area();
+  work_area_in_display.Offset(-display.bounds().x(), -display.bounds().y());
+  gfx::Rect local_work_area_in_dp = work_area_in_dp;
+  local_work_area_in_dp.Offset(-display.bounds().x(), -display.bounds().y());
+  gfx::Rect work_area_in_client_pixel = ScaleBoundsToPixelSnappedToParent(
+      size_in_client_pixel, display.bounds().size(), default_dsf,
+      local_work_area_in_dp);
+  gfx::Insets insets_in_client_pixel =
+      gfx::Rect(size_in_client_pixel).InsetsFrom(work_area_in_client_pixel);
+
+  // TODO(oshima): I think this is more conservative than necessary. The correct
+  // way is to use enclosed rect when converting the work area from dp to
+  // client pixel, but that led to weird buffer size in overlay detection.
+  // (crbug.com/920650). Investigate if we can fix it and use enclosed rect.
+  return gfx::Insets(
+      gfx::ToRoundedInt(
+          gfx::ToCeiledInt(insets_in_client_pixel.top() / default_dsf) *
+          default_dsf),
+      gfx::ToRoundedInt(
+          gfx::ToCeiledInt(insets_in_client_pixel.left() / default_dsf) *
+          default_dsf),
+      gfx::ToRoundedInt(
+          gfx::ToCeiledInt(insets_in_client_pixel.bottom() / default_dsf) *
+          default_dsf),
+      gfx::ToRoundedInt(
+          gfx::ToCeiledInt(insets_in_client_pixel.right() / default_dsf) *
+          default_dsf));
+}
+
+gfx::Rect GetStableWorkArea(const display::Display& display) {
+  auto* root = ash::Shell::GetRootWindowForDisplayId(display.id());
+  return ash::WorkAreaInsets::ForWindow(root)->ComputeStableWorkArea();
+}
+
 }  // namespace wayland
 }  // namespace exo

components/exo/wayland/zcr_remote_shell.h

@@ -9,6 +9,16 @@
 
 struct wl_client;
 
+namespace gfx {
+class Rect;
+class Insets;
+class Size;
+}  // namespace gfx
+
+namespace display {
+class Display;
+}
+
 namespace exo {
 namespace wayland {
 
@@ -19,6 +29,17 @@ void bind_remote_shell(wl_client* client,
                        uint32_t version,
                        uint32_t id);
 
+// Create the insets in client's pixel coordinates in such way that
+// work area will be within the chrome's work area.
+gfx::Insets GetWorkAreaInsetsInClientPixel(
+    const display::Display& display,
+    float default_dsf,
+    const gfx::Size& size_in_client_pixel,
+    const gfx::Rect& work_area_in_dp);
+
+// Returns a work area where the shelf is considered visible.
+gfx::Rect GetStableWorkArea(const display::Display& display);
+
 }  // namespace wayland
 }  // namespace exo
 

components/exo/wayland/server_util_unittest.cc → components/exo/wayland/zcr_remote_shell_unittest.cc

@@ -2,20 +2,21 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "components/exo/wayland/server_util.h"
+#include "components/exo/wayland/zcr_remote_shell.h"
+
 #include "components/exo/test/exo_test_base.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
 #include "ui/gfx/geometry/insets.h"
+#include "ui/views/widget/widget.h"
 
 namespace exo {
 
-using ServerUtilTest = test::ExoTestBase;
+using ZcrRemoteShellTest = test::ExoTestBase;
 
-TEST_F(ServerUtilTest, GetWorkAreaInsetsInClientPixel) {
+TEST_F(ZcrRemoteShellTest, GetWorkAreaInsetsInClientPixel) {
   UpdateDisplay("3000x2000*2.25,1920x1080");
   auto display = display::Screen::GetScreen()->GetPrimaryDisplay();
-
   gfx::Insets insets = wayland::GetWorkAreaInsetsInClientPixel(
       display, 2.25f, gfx::Size(3000, 2000), display.work_area());
   EXPECT_EQ(gfx::Insets(0, 0, 128, 0), insets);
@@ -27,7 +28,22 @@ TEST_F(ServerUtilTest, GetWorkAreaInsetsInClientPixel) {
   gfx::Insets secondary_insets = wayland::GetWorkAreaInsetsInClientPixel(
       secondary_display, 2.25f, secondary_size_in_client_pixel,
       secondary_display.work_area());
-  EXPECT_EQ(gfx::Insets(0, 0, 126, 0), secondary_insets);
+  EXPECT_EQ(gfx::Insets(0, 0, 126, 0).ToString(), secondary_insets.ToString());
+
+  // Stable Insets
+  auto widget = CreateTestWidget();
+  widget->SetFullscreen(true);
+  display = display::Screen::GetScreen()->GetPrimaryDisplay();
+  ASSERT_EQ(display.bounds(), display.work_area());
+  gfx::Insets stable_insets = wayland::GetWorkAreaInsetsInClientPixel(
+      display, 2.25f, gfx::Size(3000, 2000),
+      wayland::GetStableWorkArea(display));
+  EXPECT_EQ(gfx::Insets(0, 0, 128, 0).ToString(), stable_insets.ToString());
+  gfx::Insets secondary_stable_insets = wayland::GetWorkAreaInsetsInClientPixel(
+      secondary_display, 2.25f, secondary_size_in_client_pixel,
+      wayland::GetStableWorkArea(secondary_display));
+  EXPECT_EQ(gfx::Insets(0, 0, 126, 0).ToString(),
+            secondary_stable_insets.ToString());
 }
 
 }  // namespace exo