Adjust PIP bounds before saving snap fraction.

This is a follow-up CL of crrev.com/c/2040505, which introduced a
regression where, if the client sends a bounds that is not along
any of the edges of the PIP movement area, a wrong snap fraction
is saved to the window.

To avoid this, this CL ensures to snap the bounds to one of the
edges.

BUG=b:204050
TEST=atest android.server.am.ActivityManagerPinnedStackTests
TEST=#testEnterPictureInPictureSavePosition on kevin

Change-Id: I9d617790fb31c1a9c9bf33cf21ff67d2ae923e02
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2058934Reviewed-by: Stefan Kuhne <skuhne@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Commit-Queue: Kazuki Takise <takise@chromium.org>
Auto-Submit: Kazuki Takise <takise@chromium.org>
Cr-Commit-Position: refs/heads/master@{#742991}

ash/wm/pip/pip_positioner.cc

@@ -19,6 +19,7 @@
 #include "base/logging.h"
 #include "ui/aura/window.h"
 #include "ui/gfx/geometry/insets.h"
+#include "ui/gfx/geometry/safe_integer_conversions.h"
 #include "ui/wm/core/coordinate_conversion.h"
 
 namespace ash {
@@ -86,30 +87,30 @@ gfx::Rect PipPositioner::GetSnapFractionAppliedBounds(
       *(window_state->window()->GetProperty(ash::kPipSnapFractionKey));
 
   if (snap_fraction < 1.) {
-    int offset =
-        movement_area.x() +
-        (int)(snap_fraction * (movement_area.width() - bounds.width()));
+    int offset = movement_area.x() +
+                 gfx::ToRoundedInt(snap_fraction *
+                                   (movement_area.width() - bounds.width()));
     return gfx::Rect(offset, movement_area.y(), bounds.width(),
                      bounds.height());
   } else if (snap_fraction < 2.) {
     snap_fraction -= 1.;
-    int offset =
-        movement_area.y() +
-        (int)(snap_fraction * (movement_area.height() - bounds.height()));
+    int offset = movement_area.y() +
+                 gfx::ToRoundedInt(snap_fraction *
+                                   (movement_area.height() - bounds.height()));
     return gfx::Rect(movement_area.right() - bounds.width(), offset,
                      bounds.width(), bounds.height());
   } else if (snap_fraction < 3.) {
     snap_fraction -= 2.;
-    int offset =
-        movement_area.x() +
-        (int)((1. - snap_fraction) * (movement_area.width() - bounds.width()));
+    int offset = movement_area.x() +
+                 gfx::ToRoundedInt((1. - snap_fraction) *
+                                   (movement_area.width() - bounds.width()));
     return gfx::Rect(offset, movement_area.bottom() - bounds.height(),
                      bounds.width(), bounds.height());
   } else {
     snap_fraction -= 3.;
-    int offset =
-        movement_area.y() + (int)((1. - snap_fraction) *
-                                  (movement_area.height() - bounds.height()));
+    int offset = movement_area.y() +
+                 gfx::ToRoundedInt((1. - snap_fraction) *
+                                   (movement_area.height() - bounds.height()));
     return gfx::Rect(movement_area.x(), offset, bounds.width(),
                      bounds.height());
   }

ash/wm/window_state.cc

@@ -696,6 +696,17 @@ void WindowState::SetBoundsDirect(const gfx::Rect& bounds) {
         std::max(min_size.width(), actual_new_bounds.width()));
     actual_new_bounds.set_height(
         std::max(min_size.height(), actual_new_bounds.height()));
+
+    // Changing the size of the PIP window can detach it from one of the edges
+    // of the screen, which makes the snap fraction logic fail. Ensure to snap
+    // it again.
+    if (IsPip() && !is_dragged()) {
+      ::wm::ConvertRectToScreen(window_->GetRootWindow(), &actual_new_bounds);
+      actual_new_bounds = CollisionDetectionUtils::GetRestingPosition(
+          display, actual_new_bounds,
+          CollisionDetectionUtils::RelativePriority::kPictureInPicture);
+      ::wm::ConvertRectFromScreen(window_->GetRootWindow(), &actual_new_bounds);
+    }
   }
   BoundsSetter().SetBounds(window_, actual_new_bounds);
 }

ash/wm/window_state_unittest.cc

@@ -11,6 +11,7 @@
 #include "ash/public/cpp/shelf_config.h"
 #include "ash/public/cpp/window_properties.h"
 #include "ash/test/ash_test_base.h"
+#include "ash/wm/pip/pip_positioner.h"
 #include "ash/wm/window_state_util.h"
 #include "ash/wm/window_util.h"
 #include "ash/wm/wm_event.h"
@@ -767,6 +768,32 @@ TEST_F(WindowStateTest, SetBoundsUpdatesSizeOfPipRestoreBounds) {
             window_state->GetRestoreBoundsInScreen());
 }
 
+TEST_F(WindowStateTest, SetBoundsSnapsPipBoundsToScreenEdge) {
+  UpdateDisplay("600x900");
+
+  aura::test::TestWindowDelegate delegate;
+  delegate.set_minimum_size(gfx::Size(51, 51));
+  std::unique_ptr<aura::Window> window(CreateTestWindowInShellWithDelegate(
+      &delegate, -1, gfx::Rect(0, 0, 50, 50)));
+  WindowState* window_state = WindowState::Get(window.get());
+  window->Show();
+
+  const WMEvent enter_pip(WM_EVENT_PIP);
+  window_state->OnWMEvent(&enter_pip);
+  window->SetBounds(gfx::Rect(542, 50, 50, 50));
+  EXPECT_TRUE(window_state->IsPip());
+  // Ensure that the PIP window is along the right edge of the screen even when
+  // the new bounds is adjusted by the minimum size.
+  // 541 (left origin) + 51 (PIP width) + 8 (PIP insets) == 600.
+  EXPECT_EQ(gfx::Rect(541, 50, 51, 51),
+            window_state->window()->GetBoundsInScreen());
+
+  PipPositioner::SaveSnapFraction(window_state);
+  EXPECT_TRUE(PipPositioner::HasSnapFraction(window_state));
+  EXPECT_EQ(gfx::Rect(541, 50, 51, 51),
+            PipPositioner::GetPositionAfterMovementAreaChange(window_state));
+}
+
 // TODO(skuhne): Add more unit test to verify the correctness for the restore
 // operation.