Reset AnimationRunner when a view is removed from widget

We were depending on view_->GetWidget() to see if the view is
removed from a widget or not. But in ViewObserver::OnViewRemovedFromWidget(),
GetWidget() may return valud widget. It's because View's parent pointer
is still valid at this point. Please see GetWidget() implementation.

So explicitly clear animation runner without checking GetWidget().

note: tried invalidating parent pointer before the propagation, but caused crash.
it seems some classes are using GetWidget() in the OnViewRemovedFromWidget().

Bug: 1082633
Change-Id: I607f1845002398fa24311dcf697f52f19f25aa8e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2212109
Commit-Queue: Sang Woo Ko <sangwoo108@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#772115}

ui/views/animation/animation_delegate_views.cc

@@ -41,7 +41,7 @@ void AnimationDelegateViews::OnViewAddedToWidget(View* observed_view) {
 }
 
 void AnimationDelegateViews::OnViewRemovedFromWidget(View* observed_view) {
-  UpdateAnimationRunner();
+  ClearAnimationRunner();
 }
 
 void AnimationDelegateViews::OnViewIsDeleting(View* observed_view) {
@@ -53,7 +53,7 @@ void AnimationDelegateViews::OnViewIsDeleting(View* observed_view) {
 void AnimationDelegateViews::AnimationContainerShuttingDown(
     gfx::AnimationContainer* container) {
   container_ = nullptr;
-  compositor_animation_runner_ = nullptr;
+  ClearAnimationRunner();
 }
 
 base::TimeDelta AnimationDelegateViews::GetAnimationDurationForReporting()
@@ -76,18 +76,12 @@ void AnimationDelegateViews::SetAnimationMetricsReporter(
 }
 
 void AnimationDelegateViews::UpdateAnimationRunner() {
-  if (!container_)
-    return;
-
   if (!view_ || !view_->GetWidget() || !view_->GetWidget()->GetCompositor()) {
-    // TODO(https://crbug.com/960621): make sure the container has a correct
-    // compositor-assisted runner.
-    container_->SetAnimationRunner(nullptr);
-    compositor_animation_runner_ = nullptr;
+    ClearAnimationRunner();
     return;
   }
 
-  if (container_->has_custom_animation_runner())
+  if (!container_ || container_->has_custom_animation_runner())
     return;
 
   auto compositor_animation_runner =
@@ -98,4 +92,12 @@ void AnimationDelegateViews::UpdateAnimationRunner() {
   container_->SetAnimationRunner(std::move(compositor_animation_runner));
 }
 
+void AnimationDelegateViews::ClearAnimationRunner() {
+  // TODO(https://crbug.com/960621): make sure the container has a correct
+  // compositor-assisted runner.
+  if (container_)
+    container_->SetAnimationRunner(nullptr);
+  compositor_animation_runner_ = nullptr;
+}
+
 }  // namespace views

ui/views/animation/animation_delegate_views.h

@@ -60,10 +60,10 @@ class VIEWS_EXPORT AnimationDelegateViews
   // Sets CompositorAnimationRunner to |container_| if possible. Otherwise,
   // clears AnimationRunner of |container_|.
   void UpdateAnimationRunner();
+  void ClearAnimationRunner();
 
   View* view_;
   gfx::AnimationContainer* container_ = nullptr;
-
   ui::AnimationMetricsReporter* animation_metrics_reporter_ = nullptr;
 
   // The animation runner that |container_| uses.