Change when 'animation ended' event is generated by AnimatingLayout.

Previously, it could be fired before the final layout of the animation.
Now it will be fired after the final layout is applied, so the child
views are all in their final states.

Bug: 995596
Change-Id: Idbc187e4d4b2bf28cf720845180a427a2d04778a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1762060
Auto-Submit: Dana Fried <dfried@chromium.org>
Commit-Queue: Caroline Rising <corising@chromium.org>
Reviewed-by: Caroline Rising <corising@chromium.org>
Cr-Commit-Position: refs/heads/master@{#688766}

chrome/browser/ui/views/layout/animating_layout_manager.cc

@@ -188,11 +188,8 @@ void AnimatingLayoutManager::ResetLayout() {
   target_layout_ = target_layout_manager_->GetProposedLayout(target_size);
   current_layout_ = target_layout_;
   starting_layout_ = current_layout_;
-  const bool was_animating = current_offset_ < 1.0;
   current_offset_ = 1.0;
   set_cached_layout_size(target_size);
-  if (was_animating)
-    NotifyIsAnimatingChanged();
 }
 
 void AnimatingLayoutManager::AddObserver(Observer* observer) {
@@ -255,6 +252,13 @@ void AnimatingLayoutManager::Layout(views::View* host) {
     ResetLayout();
   }
   ApplyLayout(current_layout_);
+
+  // Send animating stopped events on layout so the current layout during the
+  // event represents the final state instead of an intermediate state.
+  if (is_animating_ && current_offset_ == 1.0) {
+    is_animating_ = false;
+    NotifyIsAnimatingChanged();
+  }
 }
 
 void AnimatingLayoutManager::InvalidateLayout() {
@@ -388,7 +392,10 @@ bool AnimatingLayoutManager::RecalculateTarget() {
     starting_offset_ = 0.0;
     current_offset_ = 0.0;
     animation_delegate_->Animate();
-    NotifyIsAnimatingChanged();
+    if (!is_animating_) {
+      is_animating_ = true;
+      NotifyIsAnimatingChanged();
+    }
   } else {
     starting_offset_ = current_offset_;
   }
@@ -412,8 +419,6 @@ void AnimatingLayoutManager::AnimateTo(double value) {
   current_layout_ = InterpolatingLayoutManager::Interpolate(
       percent, starting_layout_, target_layout_);
   InvalidateHost();
-  if (!is_animating())
-    NotifyIsAnimatingChanged();
 }
 
 void AnimatingLayoutManager::NotifyIsAnimatingChanged() {

chrome/browser/ui/views/layout/animating_layout_manager.h

@@ -79,7 +79,7 @@ class AnimatingLayoutManager : public views::LayoutManagerBase {
   gfx::Tween::Type tween_type() const { return tween_type_; }
   AnimatingLayoutManager& SetTweenType(gfx::Tween::Type tween_type);
 
-  bool is_animating() const { return current_offset_ < 1.0; }
+  bool is_animating() const { return is_animating_; }
 
   // Sets the owned (non-animating) layout manager which defines the target
   // layout that will be animated to when it changes.
@@ -165,6 +165,9 @@ class AnimatingLayoutManager : public views::LayoutManagerBase {
   // operations.
   bool suspend_recalculation_ = false;
 
+  // Used to determine when to fire animation events.
+  bool is_animating_ = false;
+
   // Where in the animation the last layout recalculation happened.
   double starting_offset_ = 0.0;
 

chrome/browser/ui/views/layout/animating_layout_manager_unittest.cc

@@ -225,6 +225,57 @@ TEST_F(AnimatingLayoutManagerSteppingTest,
   EnsureLayout(expected);
 }
 
+TEST_F(AnimatingLayoutManagerSteppingTest, TestEvents) {
+  class EventWatcher : public AnimatingLayoutManager::Observer {
+   public:
+    ~EventWatcher() override {}
+
+    explicit EventWatcher(AnimatingLayoutManager* layout) {
+      scoped_observer_.Add(layout);
+    }
+
+    void OnLayoutIsAnimatingChanged(AnimatingLayoutManager* source,
+                                    bool is_animating) override {
+      events_.push_back(is_animating);
+    }
+
+    const std::vector<bool> events() const { return events_; }
+
+   private:
+    std::vector<bool> events_;
+    ScopedObserver<AnimatingLayoutManager, Observer> scoped_observer_{this};
+  };
+
+  layout()->SetShouldAnimateBounds(true);
+  auto* const test_layout =
+      layout()->SetTargetLayoutManager(std::make_unique<TestLayoutManager>());
+  test_layout->SetLayout(layout1());
+  layout()->ResetLayout();
+  view()->SetSize(view()->GetPreferredSize());
+
+  EXPECT_FALSE(layout()->is_animating());
+  EventWatcher watcher(layout());
+  test_layout->SetLayout(layout2());
+
+  // Invalidating the layout forces a recalculation, which starts the animation.
+  const std::vector<bool> expected1{true};
+  view()->InvalidateLayout();
+  EXPECT_TRUE(layout()->is_animating());
+  EXPECT_EQ(expected1, watcher.events());
+
+  // Advance to completion.
+  animation_api()->IncrementTime(base::TimeDelta::FromMilliseconds(1000));
+  EXPECT_TRUE(layout()->is_animating());
+  EXPECT_EQ(expected1, watcher.events());
+
+  // Final layout clears the |is_animating| state because the views are now in
+  // their final configuration.
+  const std::vector<bool> expected2{true, false};
+  view()->Layout();
+  EXPECT_FALSE(layout()->is_animating());
+  EXPECT_EQ(expected2, watcher.events());
+}
+
 TEST_F(AnimatingLayoutManagerSteppingTest,
        HostInvalidate_NoAnimateBounds_NoAnimation) {
   layout()->SetShouldAnimateBounds(false);
@@ -595,7 +646,7 @@ class ImmediateLayoutManager : public LayoutManager {
   }
 
   void Layout(View* view) override {
-    DCHECK_EQ(host_, view);
+    EXPECT_EQ(host_, view);
     for (View* child : host_->children()) {
       if (use_preferred_size_) {
         const gfx::Size preferred = child->GetPreferredSize();