Fixes to animating layout manager.

In a few caes, fading out a view would not actually cause it to become
invisible. These can be listed as:
 - In some cases, FadeInOutMode::kHide would not hide the view
 - If the layout were reset during a fade-out of a view hidden by
   calling SetVisible() or FadeOut() the view might fail to hide

This change ensures that a view that is supposed to fade out always does
- and adds appropriate unit tests.

Change-Id: I9713f9806e0aa13ad8b35982ac47c2112c91d348
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1979635Reviewed-by: Caroline Rising <corising@chromium.org>
Commit-Queue: Dana Fried <dfried@chromium.org>
Cr-Commit-Position: refs/heads/master@{#727068}

ui/views/layout/animating_layout_manager.cc

@@ -433,6 +433,8 @@ void AnimatingLayoutManager::ResetLayoutToSize(const gfx::Size& target_size) {
   if (animation_delegate_)
     animation_delegate_->Reset();
 
+  ResolveFades();
+
   target_layout_ = target_layout_manager()->GetProposedLayout(target_size);
   current_layout_ = target_layout_;
   starting_layout_ = current_layout_;
@@ -493,6 +495,7 @@ bool AnimatingLayoutManager::RecalculateTarget() {
   }
   CalculateFadeInfos();
   UpdateCurrentLayout(0.0);
+
   return true;
 }
 
@@ -527,7 +530,7 @@ void AnimatingLayoutManager::UpdateCurrentLayout(double percent) {
   current_layout_ =
       ProposedLayoutBetween(percent, starting_layout_, target_layout_);
 
-  if (default_fade_mode_ == FadeInOutMode::kHide || fade_infos_.empty())
+  if (fade_infos_.empty())
     return;
 
   std::map<const View*, size_t> view_indices;
@@ -550,7 +553,9 @@ void AnimatingLayoutManager::UpdateCurrentLayout(double percent) {
       } else {
         child_layout.visible = false;
       }
-
+    } else if (default_fade_mode_ == FadeInOutMode::kHide) {
+      child_layout.child_view = fade_info.child_view;
+      child_layout.visible = false;
     } else {
       const double scale_percent =
           fade_info.fading_in ? percent : 1.0 - percent;
@@ -719,6 +724,22 @@ void AnimatingLayoutManager::CalculateFadeInfos() {
   }
 }
 
+void AnimatingLayoutManager::ResolveFades() {
+  // Views that need faded out are views which were were fading out previously
+  // because they were set to not be visible, either by calling SetVisible() or
+  // FadeOut(). Those views will not be included in the new layout but may not
+  // have been allowed to become invisible yet because of the fade-out
+  // animation. Even in the case of FadeInOutMode::kHide, if no frames of the
+  // animation have run, the relevant view may still be visible.
+  for (const LayoutFadeInfo& fade_info : fade_infos_) {
+    View* const child = fade_info.child_view;
+    if (!fade_info.fading_in && host_view()->GetIndexOf(child) >= 0 &&
+        !IsChildViewIgnoredByLayout(child) && !IsChildIncludedInLayout(child)) {
+      SetViewVisibility(child, false);
+    }
+  }
+}
+
 ChildLayout AnimatingLayoutManager::CalculateScaleFade(
     const LayoutFadeInfo& fade_info,
     base::Optional<size_t> prev_index,

ui/views/layout/animating_layout_manager.h

@@ -227,6 +227,10 @@ class VIEWS_EXPORT AnimatingLayoutManager : public LayoutManagerBase {
   // current animation.
   void CalculateFadeInfos();
 
+  // Called when resetting the layout; resolves any in-progress fades so that a
+  // view that should be rendered invisible actually is.
+  void ResolveFades();
+
   // Calculates a kScaleFrom[Minimum|Zero] fade and returns the resulting child
   // layout info.
   ChildLayout CalculateScaleFade(const LayoutFadeInfo& fade_info,

ui/views/layout/animating_layout_manager_unittest.cc

@@ -827,6 +827,116 @@ TEST_F(AnimatingLayoutManagerTest,
   EnsureLayout(final_layout);
 }
 
+TEST_F(AnimatingLayoutManagerTest,
+       FadeInOutMode_Hide_HidesViewDuringAnimation) {
+  layout()->SetShouldAnimateBounds(false);
+  layout()->SetDefaultFadeMode(AnimatingLayoutManager::FadeInOutMode::kHide);
+  layout()->SetOrientation(LayoutOrientation::kVertical);
+  FlexLayout* const flex_layout =
+      layout()->SetTargetLayoutManager(std::make_unique<FlexLayout>());
+  flex_layout->SetDefault(kMarginsKey, gfx::Insets(5))
+      .SetCollapseMargins(true)
+      .SetOrientation(LayoutOrientation::kVertical)
+      .SetDefault(kFlexBehaviorKey, kDropOut);
+  view()->SetSize({20, 35});
+  layout()->ResetLayout();
+
+  // Sanity check...
+  const ProposedLayout initial_layout{{20, 50},
+                                      {{child(0), true, {5, 5, 10, 10}},
+                                       {child(1), true, {5, 20, 10, 10}},
+                                       {child(2), false}}};
+  EnsureLayout(initial_layout);
+
+  // Hide middle view.
+  layout()->FadeOut(child(1));
+  EXPECT_TRUE(layout()->is_animating());
+
+  animation_api()->IncrementTime(base::TimeDelta::FromMilliseconds(500));
+  view()->Layout();
+
+  const ProposedLayout middle_layout{
+      {20, 35},
+      {{child(0), true, {5, 5, 10, 10}}, {child(1), false}, {child(2), false}}};
+  EnsureLayout(middle_layout);
+
+  animation_api()->IncrementTime(base::TimeDelta::FromMilliseconds(500));
+  view()->Layout();
+  const ProposedLayout final_layout{{20, 35},
+                                    {{child(0), true, {5, 5, 10, 10}},
+                                     {child(1), false},
+                                     {child(2), true, {5, 20, 10, 10}}}};
+  EnsureLayout(final_layout);
+}
+
+TEST_F(AnimatingLayoutManagerTest,
+       FadeInOutMode_Hide_HidesViewDuringAnimation_OneFrame) {
+  layout()->SetShouldAnimateBounds(false);
+  layout()->SetDefaultFadeMode(AnimatingLayoutManager::FadeInOutMode::kHide);
+  layout()->SetOrientation(LayoutOrientation::kVertical);
+  FlexLayout* const flex_layout =
+      layout()->SetTargetLayoutManager(std::make_unique<FlexLayout>());
+  flex_layout->SetDefault(kMarginsKey, gfx::Insets(5))
+      .SetCollapseMargins(true)
+      .SetOrientation(LayoutOrientation::kVertical)
+      .SetDefault(kFlexBehaviorKey, kDropOut);
+  view()->SetSize({20, 35});
+  layout()->ResetLayout();
+
+  // Sanity check...
+  const ProposedLayout initial_layout{{20, 50},
+                                      {{child(0), true, {5, 5, 10, 10}},
+                                       {child(1), true, {5, 20, 10, 10}},
+                                       {child(2), false}}};
+  EnsureLayout(initial_layout);
+
+  // Hide middle view.
+  layout()->FadeOut(child(1));
+  EXPECT_TRUE(layout()->is_animating());
+
+  animation_api()->IncrementTime(base::TimeDelta::FromMilliseconds(1000));
+  view()->Layout();
+
+  const ProposedLayout final_layout{{20, 35},
+                                    {{child(0), true, {5, 5, 10, 10}},
+                                     {child(1), false},
+                                     {child(2), true, {5, 20, 10, 10}}}};
+  EnsureLayout(final_layout);
+}
+
+TEST_F(AnimatingLayoutManagerTest,
+       FadeInOutMode_Hide_AnimationResetDuringHide) {
+  layout()->SetShouldAnimateBounds(false);
+  layout()->SetDefaultFadeMode(AnimatingLayoutManager::FadeInOutMode::kHide);
+  layout()->SetOrientation(LayoutOrientation::kVertical);
+  FlexLayout* const flex_layout =
+      layout()->SetTargetLayoutManager(std::make_unique<FlexLayout>());
+  flex_layout->SetDefault(kMarginsKey, gfx::Insets(5))
+      .SetCollapseMargins(true)
+      .SetOrientation(LayoutOrientation::kVertical)
+      .SetDefault(kFlexBehaviorKey, kDropOut);
+  view()->SetSize({20, 35});
+  layout()->ResetLayout();
+
+  // Sanity check...
+  const ProposedLayout initial_layout{{20, 50},
+                                      {{child(0), true, {5, 5, 10, 10}},
+                                       {child(1), true, {5, 20, 10, 10}},
+                                       {child(2), false}}};
+  EnsureLayout(initial_layout);
+
+  // Hide middle view.
+  layout()->FadeOut(child(1));
+  layout()->ResetLayout();
+  view()->Layout();
+
+  const ProposedLayout final_layout{{20, 35},
+                                    {{child(0), true, {5, 5, 10, 10}},
+                                     {child(1), false},
+                                     {child(2), true, {5, 20, 10, 10}}}};
+  EnsureLayout(final_layout);
+}
+
 TEST_F(AnimatingLayoutManagerTest, FadeInOutMode_SlideFromLeading_LastView) {
   const ProposedLayout initial_layout{{35, 20},
                                       {{child(0), true, {5, 5, 10, 10}},