Update occlusion state when "fills" state changed by color animation

This is a follow-up CL for crrev.com/c/2104457.

Bug: 1057024
Test: covered by unittest
Change-Id: I0733a19c08deeec2ca43f5930c3c185451fe39ca
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2113911Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#753421}

ui/aura/window.cc

@@ -1330,7 +1330,8 @@ void Window::OnLayerAlphaShapeChanged() {
     observer.OnWindowAlphaShapeSet(this);
 }
 
-void Window::OnLayerFillsBoundsOpaquelyChanged() {
+void Window::OnLayerFillsBoundsOpaquelyChanged(
+    ui::PropertyChangeReason reason) {
   // Let observers know that this window's transparent status has changed.
   // Transparent status can affect the occlusion computed for windows.
   WindowOcclusionTracker::ScopedPause pause_occlusion_tracking;
@@ -1340,7 +1341,7 @@ void Window::OnLayerFillsBoundsOpaquelyChanged() {
     DCHECK(opaque_regions_for_occlusion_.empty());
 
   for (WindowObserver& observer : observers_)
-    observer.OnWindowTransparentChanged(this);
+    observer.OnWindowTransparentChanged(this, reason);
 }
 
 void Window::OnLayerTransformed(const gfx::Transform& old_transform,

ui/aura/window.h

@@ -601,7 +601,8 @@ class AURA_EXPORT Window : public ui::LayerDelegate,
                           ui::PropertyChangeReason reason) override;
   void OnLayerOpacityChanged(ui::PropertyChangeReason reason) override;
   void OnLayerAlphaShapeChanged() override;
-  void OnLayerFillsBoundsOpaquelyChanged() override;
+  void OnLayerFillsBoundsOpaquelyChanged(
+      ui::PropertyChangeReason reason) override;
 
   // Overridden from ui::EventTarget:
   bool CanAcceptEvent(const ui::Event& event) override;

ui/aura/window_observer.h

@@ -124,9 +124,18 @@ class AURA_EXPORT WindowObserver : public base::CheckedObserver {
   // Invoked when the alpha shape of the |window|'s layer is set.
   virtual void OnWindowAlphaShapeSet(Window* window) {}
 
-  // Invoked when whether |window|'s layer fills its bounds opaquely or not
-  // is changed.
-  virtual void OnWindowTransparentChanged(Window* window) {}
+  // Invoked when whether |window|'s layer fills its bounds opaquely or not is
+  // changed.  |reason| indicates whether the value was set directly or by a
+  // color animation. Color animation happens only on LAYER_SOLID_COLOR type,
+  // and this value will always be NOT_FROM_ANIMATION on other layer types.
+  // This won't necessarily be called at every step of an animation. However, it
+  // will always be called before the first frame of the animation is rendered
+  // and when the animation ends. The client can determine whether the animation
+  // is ending by calling
+  // window->layer()->GetAnimator()->IsAnimatingProperty(
+  // ui::LayerAnimationElement::COLOR).
+  virtual void OnWindowTransparentChanged(Window* window,
+                                          ui::PropertyChangeReason reason) {}
 
   // Invoked when |window|'s position among its siblings in the stacking order
   // has changed.

ui/aura/window_occlusion_change_builder.cc

@@ -27,7 +27,6 @@ class DefaultWindowOcclusionChangeBuilder
       auto it = changes_.find(window);
       if (it == changes_.end())
         continue;
-
       window->SetOcclusionInfo(it->second.occlusion_state,
                                it->second.occluded_region);
     }

ui/aura/window_occlusion_tracker.cc

@@ -369,6 +369,10 @@ bool WindowOcclusionTracker::RecomputeOcclusionImpl(
     return false;
   }
 
+  // TODO: While considering that a window whose color is animated doesn't
+  // occlude other windows helps reduce the number of times that occlusion is
+  // recomputed, it isn't necessary to consider that the window whose color is
+  // animated itself is non-occluded.
   if (WindowIsAnimated(window) || WindowIsExcluded(window)) {
     SetWindowAndDescendantsAreOccluded(window, /* is_occluded */ false,
                                        /* is_parent_visible */ true);
@@ -879,9 +883,17 @@ void WindowOcclusionTracker::OnWindowAlphaShapeSet(Window* window) {
   });
 }
 
-void WindowOcclusionTracker::OnWindowTransparentChanged(Window* window) {
+void WindowOcclusionTracker::OnWindowTransparentChanged(
+    Window* window,
+    ui::PropertyChangeReason reason) {
+  // Call MaybeObserveAnimatedWindow() outside the lambda so that the window can
+  // be marked as animated even when its root is dirty.
+  const bool animation_started =
+      (reason == ui::PropertyChangeReason::FROM_ANIMATION) &&
+      MaybeObserveAnimatedWindow(window);
   MarkRootWindowAsDirtyAndMaybeComputeOcclusionIf(window, [=]() {
-    return WindowOpacityChangeMayAffectOcclusionStates(window);
+    return animation_started ||
+           WindowOpacityChangeMayAffectOcclusionStates(window);
   });
 }
 

ui/aura/window_occlusion_tracker.h

@@ -331,7 +331,8 @@ class AURA_EXPORT WindowOcclusionTracker : public ui::LayerAnimationObserver,
   void OnWindowOpacitySet(Window* window,
                           ui::PropertyChangeReason reason) override;
   void OnWindowAlphaShapeSet(Window* window) override;
-  void OnWindowTransparentChanged(Window* window) override;
+  void OnWindowTransparentChanged(Window* window,
+                                  ui::PropertyChangeReason reason) override;
   void OnWindowTransformed(Window* window,
                            ui::PropertyChangeReason reason) override;
   void OnWindowStackingChanged(Window* window) override;

ui/aura/window_occlusion_tracker_unittest.cc

@@ -1464,9 +1464,7 @@ TEST_P(WindowOcclusionTrackerOpacityTest,
 // Verify that no crash occurs if an animation completes on a non-tracked
 // window's layer after the window has been removed from a root with a tracked
 // window and deleted.
-// TODO(crbug.com/1057024): Alpha in SOLID_COLOR layer doesn't work with
-// animation. Fix it and use WindowOcclusionTrackerOpacityTest.
-TEST_F(WindowOcclusionTrackerTest,
+TEST_P(WindowOcclusionTrackerOpacityTest,
        DeleteNonTrackedAnimatedWindowRemovedFromTrackedRoot) {
   ui::ScopedAnimationDurationScaleMode scoped_animation_duration_scale_mode(
       ui::ScopedAnimationDurationScaleMode::NORMAL_DURATION);
@@ -1486,7 +1484,8 @@ TEST_F(WindowOcclusionTrackerTest,
   // stops being animated.
   delegate->set_expectation(Window::OcclusionState::VISIBLE,
                             SkRegion(SkIRect::MakeXYWH(10, 0, 10, 10)));
-  Window* window = CreateUntrackedWindow(gfx::Rect(10, 0, 10, 10));
+  Window* window =
+      CreateUntrackedWindow(gfx::Rect(10, 0, 10, 10), nullptr, layer_type());
   EXPECT_FALSE(delegate->is_expecting_call());
 
   window->layer()->SetAnimator(test_controller.animator());
@@ -1497,7 +1496,9 @@ TEST_F(WindowOcclusionTrackerTest,
   // of its LayerAnimator (after |observer|). Upon beginning animation, the
   // window should no longer affect the occluded region.
   delegate->set_expectation(Window::OcclusionState::VISIBLE, SkRegion());
-  window->layer()->SetOpacity(0.5f);
+  SetOpacity(window, 0.1);
+  // Drive the animation so that the color's alpha value changes.
+  test_controller.Step(kTransitionDuration / 2);
   EXPECT_FALSE(delegate->is_expecting_call());
 
   // Remove the non-tracked window from its root. WindowOcclusionTracker should
@@ -1512,6 +1513,50 @@ TEST_F(WindowOcclusionTrackerTest,
   window->layer()->GetAnimator()->StopAnimating();
 }
 
+TEST_P(WindowOcclusionTrackerOpacityTest,
+       OpacityAnimationShouldNotOccludeWindow) {
+  ui::ScopedAnimationDurationScaleMode scoped_animation_duration_scale_mode(
+      ui::ScopedAnimationDurationScaleMode::NORMAL_DURATION);
+  ui::LayerAnimatorTestController test_controller(
+      ui::LayerAnimator::CreateImplicitAnimator());
+  ui::ScopedLayerAnimationSettings layer_animation_settings(
+      test_controller.animator());
+  layer_animation_settings.SetTransitionDuration(kTransitionDuration);
+
+  // Create a tracked window. Expect it to be non-occluded.
+  MockWindowDelegate* delegate = new MockWindowDelegate();
+  delegate->set_expectation(Window::OcclusionState::VISIBLE, SkRegion());
+  auto* foo = CreateTrackedWindow(delegate, gfx::Rect(0, 0, 10, 10));
+  foo->SetName("A");
+  EXPECT_FALSE(delegate->is_expecting_call());
+
+  // Create a non-tracked window which occludes the tracked window.
+  delegate->set_expectation(Window::OcclusionState::OCCLUDED, SkRegion());
+  Window* window =
+      CreateUntrackedWindow(gfx::Rect(0, 0, 10, 10), nullptr, layer_type());
+  window->SetName("B");
+  EXPECT_FALSE(delegate->is_expecting_call());
+
+  // Set the opacity to make the tracked window VISIBLE.
+  delegate->set_expectation(Window::OcclusionState::VISIBLE, SkRegion());
+  SetOpacity(window, 0.1);
+  EXPECT_FALSE(delegate->is_expecting_call());
+
+  // Animate the window. WindowOcclusionTracker should add itself as an observer
+  // of its LayerAnimator (after |observer|). Upon beginning animation, the
+  // window should no longer affect the occluded region.
+  window->layer()->SetAnimator(test_controller.animator());
+  delegate->set_expectation(Window::OcclusionState::OCCLUDED, SkRegion());
+  SetOpacity(window, 1.0);
+  EXPECT_TRUE(delegate->is_expecting_call());
+  // Drive the animation so that the color's alpha value changes.
+  test_controller.Step(kTransitionDuration / 2);
+  EXPECT_TRUE(delegate->is_expecting_call());
+  window->layer()->GetAnimator()->StopAnimating();
+  EXPECT_FALSE(delegate->is_expecting_call());
+  window->layer()->SetAnimator(nullptr);
+}
+
 namespace {
 
 class WindowDelegateHidingWindowIfOccluded : public MockWindowDelegate {

ui/compositor/layer.cc

@@ -712,15 +712,8 @@ bool Layer::GetTargetTransformRelativeTo(const Layer* ancestor,
 }
 
 void Layer::SetFillsBoundsOpaquely(bool fills_bounds_opaquely) {
-  if (fills_bounds_opaquely_ == fills_bounds_opaquely)
-    return;
-
-  fills_bounds_opaquely_ = fills_bounds_opaquely;
-
-  cc_layer_->SetContentsOpaque(fills_bounds_opaquely);
-
-  if (delegate_)
-    delegate_->OnLayerFillsBoundsOpaquelyChanged();
+  SetFillsBoundsOpaquelyWithReason(fills_bounds_opaquely,
+                                   PropertyChangeReason::NOT_FROM_ANIMATION);
 }
 
 void Layer::SetFillsBoundsCompletely(bool fills_bounds_completely) {
@@ -1446,7 +1439,7 @@ void Layer::SetColorFromAnimation(SkColor color, PropertyChangeReason reason) {
   DCHECK_EQ(type_, LAYER_SOLID_COLOR);
   cc_layer_->SetBackgroundColor(color);
   cc_layer_->SetSafeOpaqueBackgroundColor(color);
-  SetFillsBoundsOpaquely(SkColorGetA(color) == 0xFF);
+  SetFillsBoundsOpaquelyWithReason(SkColorGetA(color) == 0xFF, reason);
 }
 
 void Layer::SetClipRectFromAnimation(const gfx::Rect& clip_rect,
@@ -1656,4 +1649,17 @@ void Layer::GetFlattenedWeakList(
     child->GetFlattenedWeakList(flattened_list);
 }
 
+void Layer::SetFillsBoundsOpaquelyWithReason(bool fills_bounds_opaquely,
+                                             PropertyChangeReason reason) {
+  if (fills_bounds_opaquely_ == fills_bounds_opaquely)
+    return;
+
+  fills_bounds_opaquely_ = fills_bounds_opaquely;
+
+  cc_layer_->SetContentsOpaque(fills_bounds_opaquely);
+
+  if (delegate_)
+    delegate_->OnLayerFillsBoundsOpaquelyChanged(reason);
+}
+
 }  // namespace ui

ui/compositor/layer.h

@@ -598,6 +598,10 @@ class COMPOSITOR_EXPORT Layer : public LayerAnimationDelegate,
   // rooted from |this|.
   void GetFlattenedWeakList(std::vector<base::WeakPtr<Layer>>* flattened_list);
 
+  // Same as SetFillsBoundsOpaque but with a reason how it's changed.
+  void SetFillsBoundsOpaquelyWithReason(bool fills_bounds_opaquely,
+                                        PropertyChangeReason reason);
+
   const LayerType type_;
 
   Compositor* compositor_;

ui/compositor/layer_delegate.cc

@@ -16,7 +16,8 @@ void LayerDelegate::OnLayerOpacityChanged(PropertyChangeReason reason) {}
 
 void LayerDelegate::OnLayerAlphaShapeChanged() {}
 
-void LayerDelegate::OnLayerFillsBoundsOpaquelyChanged() {}
+void LayerDelegate::OnLayerFillsBoundsOpaquelyChanged(
+    PropertyChangeReason reason) {}
 
 void LayerDelegate::UpdateVisualState() {}
 

ui/compositor/layer_delegate.h

@@ -44,7 +44,9 @@ class COMPOSITOR_EXPORT LayerDelegate {
   virtual void OnLayerAlphaShapeChanged();
 
   // Invoked when whether the layer fills its bounds opaquely or not changed.
-  virtual void OnLayerFillsBoundsOpaquelyChanged();
+  // |reason| indicates whether the property was changed directly or by an
+  // animation.
+  virtual void OnLayerFillsBoundsOpaquelyChanged(PropertyChangeReason reason);
 
   // Called when it is a good opportunity for the delegate to update any visual
   // state or schedule any additional regions to be painted. Soon after this is