Cancel new-style tabstrip animations when an old-style animation begins.

Bug: 958173
Change-Id: Iff81fa8dd3ff57cb7121d628cd5e78e8e58c157f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1633350
Commit-Queue: Taylor Bergquist <tbergquist@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#664067}

chrome/browser/ui/views/tabs/tab_animation.cc

@@ -62,14 +62,6 @@ void TabAnimation::CompleteAnimation() {
   duration_ = kZeroDuration;
 }
 
-void TabAnimation::CancelAnimation() {
-  TabAnimationState current_state = GetCurrentState();
-  initial_state_ = current_state;
-  target_state_ = current_state;
-  start_time_ = base::TimeTicks::Now();
-  duration_ = kZeroDuration;
-}
-
 void TabAnimation::NotifyCloseCompleted() {
   std::move(tab_removed_callback_).Run();
 }

chrome/browser/ui/views/tabs/tab_animation.h

@@ -34,7 +34,6 @@ class TabAnimation {
   void RetargetTo(TabAnimationState target_state);
 
   void CompleteAnimation();
-  void CancelAnimation();
 
   // Notifies the owner of the animated tab that the close animation
   // has completed and the tab can be cleaned up.

chrome/browser/ui/views/tabs/tab_animation_unittest.cc

@@ -95,48 +95,6 @@ TEST_F(TabAnimationTest, CompletedAnimationSnapsToTarget) {
             PinnednessOf(animation.GetCurrentState()));
 }
 
-TEST_F(TabAnimationTest, CanceledAnimationStaysAtInitial) {
-  TabAnimationState initial_state = TabAnimationState::ForIdealTabState(
-      TabAnimationState::TabOpenness::kOpen,
-      TabAnimationState::TabPinnedness::kUnpinned,
-      TabAnimationState::TabActiveness::kInactive, 0);
-  TabAnimationState target_state =
-      initial_state.WithPinnedness(TabAnimationState::TabPinnedness::kPinned);
-  TabAnimation animation =
-      TabAnimation::ForStaticState(initial_state, base::BindOnce([]() {}));
-  animation.AnimateTo(target_state);
-
-  animation.CancelAnimation();
-
-  EXPECT_EQ(kZeroDuration, animation.GetTimeRemaining());
-  EXPECT_EQ(base::TimeDelta::FromMilliseconds(0), animation.GetTimeRemaining());
-  EXPECT_EQ(PinnednessOf(initial_state),
-            PinnednessOf(animation.GetCurrentState()));
-}
-
-TEST_F(TabAnimationTest, PartwayFinishedCanceledAnimationStaysPartwayFinished) {
-  TabAnimationState initial_state = TabAnimationState::ForIdealTabState(
-      TabAnimationState::TabOpenness::kOpen,
-      TabAnimationState::TabPinnedness::kUnpinned,
-      TabAnimationState::TabActiveness::kInactive, 0);
-  TabAnimationState target_state =
-      initial_state.WithPinnedness(TabAnimationState::TabPinnedness::kPinned);
-  TabAnimation animation =
-      TabAnimation::ForStaticState(initial_state, base::BindOnce([]() {}));
-  animation.AnimateTo(target_state);
-
-  env_.FastForwardBy(TabAnimation::kAnimationDuration / 2.0);
-
-  animation.CancelAnimation();
-
-  EXPECT_EQ(kZeroDuration, animation.GetTimeRemaining());
-  EXPECT_EQ(base::TimeDelta::FromMilliseconds(0), animation.GetTimeRemaining());
-  EXPECT_LT(PinnednessOf(initial_state),
-            PinnednessOf(animation.GetCurrentState()));
-  EXPECT_LT(PinnednessOf(animation.GetCurrentState()),
-            PinnednessOf(target_state));
-}
-
 TEST_F(TabAnimationTest, ReplacedAnimationRestartsDuration) {
   TabAnimationState initial_state = TabAnimationState::ForIdealTabState(
       TabAnimationState::TabOpenness::kOpen,

chrome/browser/ui/views/tabs/tab_strip.cc

@@ -2105,6 +2105,10 @@ void TabStrip::StartMoveTabAnimation() {
 }
 
 void TabStrip::AnimateToIdealBounds() {
+  // bounds_animator_ and animator_ should not run concurrently.
+  // bounds_animator_ takes precedence, and can finish what animator_ started.
+  animator_->CompleteAnimationsWithoutDestroyingTabs();
+
   for (int i = 0; i < tab_count(); ++i) {
     // If the tab is being dragged manually, skip it.
     Tab* tab = tab_at(i);

chrome/browser/ui/views/tabs/tab_strip_animator.cc

@@ -97,16 +97,13 @@ void TabStripAnimator::SetActiveTab(int prev_active_index,
 }
 
 void TabStripAnimator::CompleteAnimations() {
-  for (size_t i = 0; i < animations_.size(); i++) {
-    animations_[i].CompleteAnimation();
-  }
+  CompleteAnimationsWithoutDestroyingTabs();
   RemoveClosedTabs();
-  timer_.Stop();
 }
 
-void TabStripAnimator::CancelAnimations() {
+void TabStripAnimator::CompleteAnimationsWithoutDestroyingTabs() {
   for (TabAnimation& animation : animations_) {
-    animation.CancelAnimation();
+    animation.CompleteAnimation();
   }
   timer_.Stop();
 }

chrome/browser/ui/views/tabs/tab_strip_animator.h

@@ -58,10 +58,12 @@ class TabStripAnimator {
 
   void CompleteAnimations();
 
-  // TODO(958173): Temporary method that aborts current animations, leaving
-  // tabs where they are. Use to hand off animation responsibilities from
-  // this animator to elsewhere without teleporting tabs.
-  void CancelAnimations();
+  // TODO(958173): Temporary method that completes running animations,
+  // without invoking the callback to destroy removed tabs. Use to hand
+  // off animation (and removed tab destruction) responsibilities from
+  // this animator to elsewhere without teleporting tabs or destroying
+  // the same tab more than once.
+  void CompleteAnimationsWithoutDestroyingTabs();
 
  private:
   void AnimateTabTo(int index, TabAnimationState target_state);

chrome/browser/ui/views/tabs/tab_strip_animator_unittest.cc

@@ -160,20 +160,6 @@ TEST_F(TabStripAnimatorTest, CompleteAnimations) {
   EXPECT_EQ(1.0f, OpennessOf(animator_.GetCurrentTabStates()[0]));
 }
 
-TEST_F(TabStripAnimatorTest, CancelAnimations) {
-  animator_.InsertTabAt(0, base::BindOnce([]() {}),
-                        TabAnimationState::TabActiveness::kActive,
-                        TabAnimationState::TabPinnedness::kUnpinned);
-  EXPECT_TRUE(animator_.IsAnimating());
-  EXPECT_EQ(1u, animator_.GetCurrentTabStates().size());
-  EXPECT_EQ(0.0f, OpennessOf(animator_.GetCurrentTabStates()[0]));
-
-  animator_.CancelAnimations();
-
-  EXPECT_FALSE(animator_.IsAnimating());
-  EXPECT_EQ(0.0f, OpennessOf(animator_.GetCurrentTabStates()[0]));
-}
-
 TEST_F(TabStripAnimatorTest, CompleteAnimationsRemovesClosedTabs) {
   TabClosedDetector second_tab;
   animator_.InsertTabAtNoAnimation(0, base::BindOnce([]() {}),
@@ -200,7 +186,8 @@ TEST_F(TabStripAnimatorTest, CompleteAnimationsRemovesClosedTabs) {
   EXPECT_TRUE(second_tab.was_closed_);
 }
 
-TEST_F(TabStripAnimatorTest, CancelAnimationsDoesNotRemoveClosedTabs) {
+TEST_F(TabStripAnimatorTest,
+       CompleteAnimationsWithoutDestroyingTabsDoesNotRemoveClosedTabs) {
   TabClosedDetector second_tab;
   animator_.InsertTabAtNoAnimation(0, base::BindOnce([]() {}),
                                    TabAnimationState::TabActiveness::kActive,
@@ -219,7 +206,7 @@ TEST_F(TabStripAnimatorTest, CancelAnimationsDoesNotRemoveClosedTabs) {
   EXPECT_EQ(1.0f, OpennessOf(animator_.GetCurrentTabStates()[1]));
   EXPECT_FALSE(second_tab.was_closed_);
 
-  animator_.CancelAnimations();
+  animator_.CompleteAnimationsWithoutDestroyingTabs();
 
   EXPECT_FALSE(animator_.IsAnimating());
   EXPECT_EQ(2u, animator_.GetCurrentTabStates().size());