desks: Fix screenshot not requested quick enough when updating.

This regressed in crrev.com/c/2417491 when we added edge padding, but
did not update the request new screenshot code. This had users seeing
an extra black region for a couple of updates before the new screenshot
was taken and placed. Added a couple more tests to
RootWindowDeskSwitchAnimatorTest for Update/End Swipe.

Test: added
Bug: 1111445
Change-Id: I787ecd2c2603e5626dc3d15f5f4ca7f2d5ded33f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2438572
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812758}

ash/wm/desks/root_window_desk_switch_animator.cc

@@ -43,10 +43,6 @@ constexpr int kMinDistanceBeforeScreenshotDp = 40;
 constexpr base::TimeDelta kAnimationDuration =
     base::TimeDelta::FromMilliseconds(300);
 
-// In touchpad units, a touchpad swipe of this length will correspond to a full
-// desk change.
-constexpr int kTouchpadSwipeLengthForDeskChange = 100;
-
 // The amount, by which the detached old layers of the removed desk's windows,
 // is translated vertically during the for-remove desk switch animation.
 constexpr int kRemovedDeskWindowYTranslation = 20;
@@ -115,7 +111,8 @@ std::string GetScreenshotLayerName(int index) {
 // units. Convert these units so that what is considered a full touchpad swipe
 // shifts the animation layer one entire desk length.
 float TouchpadToXTranslation(float touchpad_x, int desk_length) {
-  return desk_length * touchpad_x / kTouchpadSwipeLengthForDeskChange;
+  return desk_length * touchpad_x /
+         RootWindowDeskSwitchAnimator::kTouchpadSwipeLengthForDeskChange;
 }
 
 }  // namespace
@@ -274,21 +271,23 @@ bool RootWindowDeskSwitchAnimator::UpdateSwipeAnimation(float scroll_delta_x) {
   // the delegate to request a new screenshot if the animating layer is about to
   // slide past the bounds which are visible to the user (root window bounds).
   //
-  //            <--- moving left
-  //   +------------------------------+
-  //   |               +-----------+  |
-  //   |      b        |     a     |  |
-  //   |               +___________+  |
-  //   +______________________________+
+  //              moving right ---->
+  //   +---+------------------------------+---+
+  //   |   |               +-----------+  |   |
+  //   | c |      b        |     a     |  | c |
+  //   |   |               +___________+  |   |
+  //   +___+______________________________+___+
   //
   //  a - root window/visible bounds - (0,0-1000x500)
-  //  b - animating layer with two screenshots - (0,0-2050x500)
-  //    - with translation (-1000, 0)
-  //  We will notify the delegate to request a new screenshot once the right
-  //  of b is within |kMinDistanceBeforeScreenshotDp| of the right of a (i.e.
-  //  translation of (-1040, 0)).
+  //  b - animating layer with two screenshots and edge padding - (0,0-2350x500)
+  //    - current second screenshot is visible (translation (-1200, 0))
+  //  c - Edge padding, equal to |kEdgePaddingRatio| x 1000 - 150 dips wide
+  //  We will notify the delegate to request a new screenshot once the x of b is
+  //  within |kMinDistanceBeforeScreenshotDp| of the x of a, not including the
+  //  edge padding (i.e. translation of (-190, 0)).
   gfx::RectF transformed_animation_layer_bounds(animation_layer->bounds());
   transform.TransformRect(&transformed_animation_layer_bounds);
+  transformed_animation_layer_bounds.Inset(edge_padding_width_dp_, 0);
 
   const bool moving_left = scroll_delta_x < 0.f;
   const bool going_out_of_bounds =

ash/wm/desks/root_window_desk_switch_animator.h

@@ -190,6 +190,10 @@ class ASH_EXPORT RootWindowDeskSwitchAnimator
   // continuously.
   static constexpr float kEdgePaddingRatio = 0.15f;
 
+  // In touchpad units, a touchpad swipe of this length will correspond to a
+  // full desk change.
+  static constexpr int kTouchpadSwipeLengthForDeskChange = 100;
+
   RootWindowDeskSwitchAnimator(aura::Window* root,
                                int starting_desk_index,
                                int ending_desk_index,

ash/wm/desks/root_window_desk_switch_animator_unittest.cc

@@ -9,6 +9,8 @@
 #include "ash/public/cpp/ash_features.h"
 #include "ash/shell.h"
 #include "ash/test/ash_test_base.h"
+#include "ash/wm/desks/desks_controller.h"
+#include "ash/wm/desks/desks_histogram_enums.h"
 #include "ash/wm/desks/root_window_desk_switch_animator_test_api.h"
 #include "base/callback_forward.h"
 #include "base/test/scoped_feature_list.h"
@@ -341,4 +343,107 @@ TEST_F(RootWindowDeskSwitchAnimatorTest, MultipleReplacements) {
                              animation_layer));
 }
 
+// Tests that the update swipe animation api requests a new screenshot when
+// needed.
+TEST_F(RootWindowDeskSwitchAnimatorTest, UpdateSwipeAnimationNewScreenshot) {
+  // Add two more desks as we need three desks for this test.
+  DesksController::Get()->NewDesk(DesksCreationRemovalSource::kButton);
+  DesksController::Get()->NewDesk(DesksCreationRemovalSource::kButton);
+
+  InitAnimator(0, 1);
+  TakeStartingDeskScreenshotAndWait();
+  TakeEndingDeskScreenshotAndWait();
+
+  const int touchpad_swipe_length_for_desk_change =
+      RootWindowDeskSwitchAnimator::kTouchpadSwipeLengthForDeskChange;
+  // Swipe so that half of desk indexed 0 and half of desk indexed 1 is shown.
+  // Verify that a new screenshot is not needed, as the screenshots of both desk
+  // 0 and desk 1 were taken when initializing.
+  EXPECT_FALSE(animator()->UpdateSwipeAnimation(
+      -touchpad_swipe_length_for_desk_change / 2));
+
+  // Swipe so that desk indexed 1 is fully shown. Verify that a new screenshot
+  // is needed as we expect the user to continue swiping to show desk indexed 2.
+  EXPECT_TRUE(animator()->UpdateSwipeAnimation(
+      -touchpad_swipe_length_for_desk_change / 2));
+}
+
+// Tests that additional swipes do not shift the animation layer if it has
+// reached its limit.
+TEST_F(RootWindowDeskSwitchAnimatorTest, UpdateSwipeAnimationLimit) {
+  // Add one more desk as we need two desks for this test.
+  DesksController::Get()->NewDesk(DesksCreationRemovalSource::kButton);
+
+  InitAnimator(0, 1);
+  TakeStartingDeskScreenshotAndWait();
+  TakeEndingDeskScreenshotAndWait();
+
+  // Do a large right swipe; this will ensure we reach the limit on the left of
+  // the animation layer.
+  const int touchpad_swipe_length_for_desk_change =
+      RootWindowDeskSwitchAnimator::kTouchpadSwipeLengthForDeskChange;
+  animator()->UpdateSwipeAnimation(5 * touchpad_swipe_length_for_desk_change);
+  auto* animation_layer = test_api()->GetAnimationLayer();
+  int x_translation = animation_layer->transform().To2dTranslation().x();
+
+  // Test that an additional small right swipe will not shift the animation
+  // layer.
+  animator()->UpdateSwipeAnimation(5);
+  EXPECT_EQ(x_translation, animation_layer->transform().To2dTranslation().x());
+
+  // Swipe back to desk indexed 1.
+  animator()->UpdateSwipeAnimation(-2 * touchpad_swipe_length_for_desk_change);
+
+  // Do a large right swipe; this will ensure we reach the limit on the left of
+  // the animation layer.
+  animator()->UpdateSwipeAnimation(-5 * touchpad_swipe_length_for_desk_change);
+  x_translation = animation_layer->transform().To2dTranslation().x();
+
+  // Test that an additional small left swipe will not shift the animation
+  // layer.
+  animator()->UpdateSwipeAnimation(-5);
+  EXPECT_EQ(x_translation, animation_layer->transform().To2dTranslation().x());
+}
+
+// Tests the when ending the swipe animation, we animate to the expected desk.
+TEST_F(RootWindowDeskSwitchAnimatorTest, EndSwipeAnimation) {
+  // Add one more desk as we need two desks for this test.
+  DesksController::Get()->NewDesk(DesksCreationRemovalSource::kButton);
+
+  InitAnimator(0, 1);
+  TakeStartingDeskScreenshotAndWait();
+  TakeEndingDeskScreenshotAndWait();
+  auto* animation_layer = test_api()->GetAnimationLayer();
+
+  const int touchpad_swipe_length_for_desk_change =
+      RootWindowDeskSwitchAnimator::kTouchpadSwipeLengthForDeskChange;
+  // Make a small left swipe headed towards desk indexed 1. Desk indexed 0
+  // should still be the most visible desk, so on ending the swipe animation,
+  // desk indexed 0 is the target desk.
+  animator()->UpdateSwipeAnimation(-touchpad_swipe_length_for_desk_change / 10);
+  animator()->EndSwipeAnimation();
+  EXPECT_EQ(
+      Shell::GetPrimaryRootWindow()->bounds(),
+      GetTargetVisibleBounds(test_api()->GetScreenshotLayerOfDeskWithIndex(0),
+                             animation_layer));
+
+  // Reinitialize the animator as each animator only supports one
+  // EndSwipeAnimation during its lifetime.
+  InitAnimator(0, 1);
+  TakeStartingDeskScreenshotAndWait();
+  TakeEndingDeskScreenshotAndWait();
+  animation_layer = test_api()->GetAnimationLayer();
+
+  // Make a big left swipe headed towards desk indexed 1. Desk indexed 1 should
+  // be the new most visible desk, so on ending the swipe animation, desk
+  // indexed 1 is the target desk.
+  animator()->UpdateSwipeAnimation(-9 * touchpad_swipe_length_for_desk_change /
+                                   10);
+  animator()->EndSwipeAnimation();
+  EXPECT_EQ(
+      Shell::GetPrimaryRootWindow()->bounds(),
+      GetTargetVisibleBounds(test_api()->GetScreenshotLayerOfDeskWithIndex(1),
+                             animation_layer));
+}
+
 }  // namespace ash