Support deleting a window with tracked occlusion from...

Support deleting a window with tracked occlusion from LayerAnimationObserver::OnLayerAnimationEnded.

Previously, if a window with tracked occlusion was deleted from
LayerAnimationObserver::OnLayerAnimationEnded, an invalid memory
access occurred in WindowOcclusionTracker::CleanupAnimatedWindows
(because the window was deleted before being removed from
|animated_windows_|).

This CL fixes this problem by removing the Window* from
|animated_windows_| in WindowOcclusionTracker::OnWindowDestroyed.

Bug: 668690
Change-Id: I77749c79d72d7cebc3a77647552605c8abec3abb
Reviewed-on: https://chromium-review.googlesource.com/806356
Commit-Queue: François Doray <fdoray@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521420}

ui/aura/window_occlusion_tracker.cc

@@ -441,9 +441,15 @@ void WindowOcclusionTracker::OnWindowStackingChanged(Window* window) {
 
 void WindowOcclusionTracker::OnWindowDestroyed(Window* window) {
   DCHECK(!window->GetRootWindow() || (window == window->GetRootWindow()));
-  // Animations should be completed or aborted before a window is destroyed.
-  DCHECK(!WindowIsAnimated(window));
   tracked_windows_.erase(window);
+  // Animations should be completed or aborted before a window is destroyed.
+  DCHECK(!window->layer()->GetAnimator()->IsAnimatingOnePropertyOf(
+      kSkipWindowWhenPropertiesAnimated));
+  // |window| must be removed from |animated_windows_| to prevent an invalid
+  // access in CleanupAnimatedWindows() if |window| is being destroyed from a
+  // LayerAnimationObserver after an animation has ended but before |this| has
+  // been notified.
+  animated_windows_.erase(window);
 }
 
 void WindowOcclusionTracker::OnWindowAddedToRootWindow(Window* window) {

ui/aura/window_occlusion_tracker_unittest.cc

@@ -13,7 +13,9 @@
 #include "ui/aura/test/test_window_delegate.h"
 #include "ui/aura/test/test_windows.h"
 #include "ui/aura/window_observer.h"
+#include "ui/compositor/layer_animation_observer.h"
 #include "ui/compositor/layer_animation_sequence.h"
+#include "ui/compositor/layer_animator.h"
 #include "ui/compositor/scoped_animation_duration_scale_mode.h"
 #include "ui/compositor/scoped_layer_animation_settings.h"
 #include "ui/compositor/test/layer_animator_test_controller.h"
@@ -1160,4 +1162,64 @@ TEST_F(WindowOcclusionTrackerTest, ChangeTrackedWindowBeforeObserveAddToRoot) {
   window->RemoveObserver(&observer);
 }
 
+namespace {
+
+class ObserverDestroyingWindowOnAnimationEnded
+    : public ui::LayerAnimationObserver {
+ public:
+  ObserverDestroyingWindowOnAnimationEnded(Window* window) : window_(window) {}
+
+  ~ObserverDestroyingWindowOnAnimationEnded() override {
+    EXPECT_FALSE(window_);
+  }
+
+  void OnLayerAnimationEnded(ui::LayerAnimationSequence* sequence) override {
+    EXPECT_TRUE(window_);
+    delete window_;
+    window_ = nullptr;
+  }
+
+  void OnLayerAnimationAborted(ui::LayerAnimationSequence* sequence) override {}
+  void OnLayerAnimationScheduled(
+      ui::LayerAnimationSequence* sequence) override {}
+
+ private:
+  Window* window_;
+
+  DISALLOW_COPY_AND_ASSIGN(ObserverDestroyingWindowOnAnimationEnded);
+};
+
+}  // namespace
+
+// Verify that no crash occurs if a LayerAnimationObserver destroys a tracked
+// window before WindowOcclusionTracker is notified that the animation ended.
+TEST_F(WindowOcclusionTrackerTest,
+       DestroyTrackedWindowFromLayerAnimationObserver) {
+  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 window. Expect it to be non-occluded.
+  MockWindowDelegate* delegate = new MockWindowDelegate();
+  delegate->set_expectation(WindowOcclusionChangedExpectation::NOT_OCCLUDED);
+  Window* window = CreateTrackedWindow(delegate, gfx::Rect(0, 0, 10, 10));
+  EXPECT_FALSE(delegate->is_expecting_call());
+  window->layer()->SetAnimator(test_controller.animator());
+
+  // Add a LayerAnimationObserver that destroys the window when an animation
+  // ends.
+  ObserverDestroyingWindowOnAnimationEnded observer(window);
+  window->layer()->GetAnimator()->AddObserver(&observer);
+
+  // Start animating the opacity of the window.
+  window->layer()->SetOpacity(0.5f);
+
+  // Complete the animation. Expect no crash.
+  window->layer()->GetAnimator()->StopAnimating();
+}
+
 }  // namespace aura