Fix bounds calculation of snapped windows in clamshell mode.

Sometimes the screen width can be odd numbered, and the current
logic will end up with a slight gap between the snapped bounds.

This CL fixes the logic, the width will be based on the mid point
of the work area.

Bug: 1020351
Test: the new test case
Change-Id: I8d4e3a7cac6ecf5ab4b18e5f6f5404223e88c9e2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1902766Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Commit-Queue: Jun Mukai <mukai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713501}

ash/BUILD.gn

@@ -1985,6 +1985,7 @@ test("ash_unittests") {
     "wm/window_mirror_view_unittest.cc",
     "wm/window_modality_controller_unittest.cc",
     "wm/window_positioner_unittest.cc",
+    "wm/window_positioning_utils_unittest.cc",
     "wm/window_preview_view_unittest.cc",
     "wm/window_state_unittest.cc",
     "wm/window_transient_descendant_iterator_unittest.cc",

ash/wm/window_positioning_utils.cc

@@ -31,16 +31,11 @@ namespace ash {
 
 namespace {
 
-// Returns the default width of a snapped window.
-int GetDefaultSnappedWindowWidth(aura::Window* window) {
-  const float kSnappedWidthWorkspaceRatio = 0.5f;
-
-  int work_area_width =
+int GetSnappedWindowWidth(int ideal_width, aura::Window* window) {
+  const int work_area_width =
       screen_util::GetDisplayWorkAreaBoundsInParent(window).width();
-  int min_width =
+  const int min_width =
       window->delegate() ? window->delegate()->GetMinimumSize().width() : 0;
-  int ideal_width =
-      static_cast<int>(work_area_width * kSnappedWidthWorkspaceRatio);
   return base::ClampToRange(ideal_width, min_width, work_area_width);
 }
 
@@ -93,15 +88,19 @@ void AdjustBoundsToEnsureMinimumWindowVisibility(const gfx::Rect& visible_area,
 gfx::Rect GetDefaultLeftSnappedWindowBoundsInParent(aura::Window* window) {
   gfx::Rect work_area_in_parent(
       screen_util::GetDisplayWorkAreaBoundsInParent(window));
-  return gfx::Rect(work_area_in_parent.x(), work_area_in_parent.y(),
-                   GetDefaultSnappedWindowWidth(window),
-                   work_area_in_parent.height());
+  const int middle = work_area_in_parent.CenterPoint().x();
+  return gfx::Rect(
+      work_area_in_parent.x(), work_area_in_parent.y(),
+      GetSnappedWindowWidth(middle - work_area_in_parent.x(), window),
+      work_area_in_parent.height());
 }
 
 gfx::Rect GetDefaultRightSnappedWindowBoundsInParent(aura::Window* window) {
   gfx::Rect work_area_in_parent(
       screen_util::GetDisplayWorkAreaBoundsInParent(window));
-  int width = GetDefaultSnappedWindowWidth(window);
+  const int middle = work_area_in_parent.CenterPoint().x();
+  const int width =
+      GetSnappedWindowWidth(work_area_in_parent.right() - middle, window);
   return gfx::Rect(work_area_in_parent.right() - width, work_area_in_parent.y(),
                    width, work_area_in_parent.height());
 }

ash/wm/window_positioning_utils_unittest.cc

+// Copyright 2019 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 "ash/wm/window_positioning_utils.h"
+
+#include "ash/test/ash_test_base.h"
+#include "ui/aura/test/test_window_delegate.h"
+#include "ui/aura/window.h"
+
+namespace ash {
+
+using WindowPositioningUtilsTest = AshTestBase;
+
+TEST_F(WindowPositioningUtilsTest, SnapBoundsWithOddNumberedScreenWidth) {
+  UpdateDisplay("999x700");
+
+  auto window = CreateToplevelTestWindow();
+  gfx::Rect left_bounds =
+      GetDefaultLeftSnappedWindowBoundsInParent(window.get());
+  gfx::Rect right_bounds =
+      GetDefaultRightSnappedWindowBoundsInParent(window.get());
+  EXPECT_EQ(left_bounds.x(), 0);
+  EXPECT_EQ(left_bounds.y(), 0);
+  EXPECT_EQ(right_bounds.right(), 999);
+  EXPECT_EQ(right_bounds.y(), 0);
+  EXPECT_EQ(left_bounds.right(), right_bounds.x());
+  EXPECT_NEAR(left_bounds.width(), 499, 1);
+  EXPECT_NEAR(right_bounds.width(), 499, 1);
+}
+
+TEST_F(WindowPositioningUtilsTest, SnapBoundsWithMinimumSize) {
+  UpdateDisplay("800x600");
+
+  auto window = CreateToplevelTestWindow();
+  auto* test_delegate =
+      static_cast<aura::test::TestWindowDelegate*>(window->delegate());
+  test_delegate->set_minimum_size(gfx::Size(300, 200));
+  gfx::Rect left_bounds =
+      GetDefaultLeftSnappedWindowBoundsInParent(window.get());
+  EXPECT_EQ(left_bounds.width(), 400);
+  gfx::Rect right_bounds =
+      GetDefaultRightSnappedWindowBoundsInParent(window.get());
+  EXPECT_EQ(right_bounds.width(), 400);
+  EXPECT_EQ(right_bounds.right(), 800);
+
+  test_delegate->set_minimum_size(gfx::Size(600, 200));
+  left_bounds = GetDefaultLeftSnappedWindowBoundsInParent(window.get());
+  EXPECT_EQ(left_bounds.width(), 600);
+  right_bounds = GetDefaultRightSnappedWindowBoundsInParent(window.get());
+  EXPECT_EQ(right_bounds.width(), 600);
+  EXPECT_EQ(right_bounds.right(), 800);
+
+  test_delegate->set_minimum_size(gfx::Size(1200, 200));
+  left_bounds = GetDefaultLeftSnappedWindowBoundsInParent(window.get());
+  EXPECT_EQ(left_bounds.width(), 800);
+  right_bounds = GetDefaultRightSnappedWindowBoundsInParent(window.get());
+  EXPECT_EQ(right_bounds.width(), 800);
+  EXPECT_EQ(right_bounds.right(), 800);
+}
+
+}  // namespace ash