Fix seg fault crash during device scale factor change

If the device scale factor of a layer changes while it is undergoing a
transform animation, it may result in a crash. This is because during a
device scale factor change operation, all transform animations are
cancelled leading to a call being made to animation observers of the
transform animation. The observers can then delete the layer or some
other layer in the hierarchical traversal. This can result in a seg
fault crash.

Bug: 987300
Test: Added unit test for the crash.
Change-Id: I8842e4b39daf59e4068497a3d662fda7f921b729
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1757126Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Malay Keshav <malaykeshav@chromium.org>
Cr-Commit-Position: refs/heads/master@{#688360}

ui/compositor/layer.cc

@@ -1173,8 +1173,19 @@ void Layer::SuppressPaint() {
 void Layer::OnDeviceScaleFactorChanged(float device_scale_factor) {
   if (device_scale_factor_ == device_scale_factor)
     return;
-  if (animator_)
+
+  base::WeakPtr<Layer> weak_this = weak_ptr_factory_.GetWeakPtr();
+
+  // NOTE: Some animation observers destroy the layer when the animation ends.
+  if (animator_) {
     animator_->StopAnimatingProperty(LayerAnimationElement::TRANSFORM);
+
+    // Do not proceed if the layer was destroyed due to an animation
+    // observer.
+    if (!weak_this)
+      return;
+  }
+
   const float old_device_scale_factor = device_scale_factor_;
   device_scale_factor_ = device_scale_factor;
   RecomputeDrawsContentAndUVRect();
@@ -1189,8 +1200,14 @@ void Layer::OnDeviceScaleFactorChanged(float device_scale_factor) {
     delegate_->OnDeviceScaleFactorChanged(old_device_scale_factor,
                                           device_scale_factor);
   }
-  for (auto* child : children_)
+  for (auto* child : children_) {
     child->OnDeviceScaleFactorChanged(device_scale_factor);
+
+    // A child layer may have triggered a delegate or an observer to delete
+    // |this| layer. In which case return early to avoid crash.
+    if (!weak_this)
+      return;
+  }
   if (layer_mask_)
     layer_mask_->OnDeviceScaleFactorChanged(device_scale_factor);
 }

ui/compositor/layer_unittest.cc

@@ -2654,6 +2654,106 @@ TEST_P(LayerWithRealCompositorTest, SwitchCCLayerMasksToBounds) {
   EXPECT_TRUE(l1->cc_layer_for_testing()->masks_to_bounds());
 }
 
+// An animation observer that deletes the layer when the animation ends.
+class TestAnimationObserver : public ImplicitAnimationObserver {
+ public:
+  TestAnimationObserver() = default;
+
+  Layer* layer() const { return layer_.get(); }
+
+  void SetLayer(std::unique_ptr<Layer> layer) { layer_ = std::move(layer); }
+
+  // ui::ImplicitAnimationObserver overrides:
+  void OnImplicitAnimationsCompleted() override {}
+
+ protected:
+  void OnLayerAnimationEnded(LayerAnimationSequence* sequence) override {
+    layer_.reset();
+  }
+
+ private:
+  std::unique_ptr<Layer> layer_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestAnimationObserver);
+};
+
+// Triggerring a OnDeviceScaleFactorChanged while a layer is undergoing
+// transform animation, may cause a crash. This is because the layer may be
+// deleted by the animation observer leading to a seg fault.
+TEST_P(LayerWithRealCompositorTest, DeletingLayerDuringScaleFactorChange) {
+  TestAnimationObserver animation_observer;
+
+  std::unique_ptr<Layer> root = CreateLayer(LAYER_SOLID_COLOR);
+  animation_observer.SetLayer(CreateLayer(LAYER_SOLID_COLOR));
+
+  Layer* layer_to_delete = animation_observer.layer();
+
+  GetCompositor()->SetRootLayer(root.get());
+  root->Add(layer_to_delete);
+
+  EXPECT_EQ(gfx::Transform(), layer_to_delete->GetTargetTransform());
+
+  gfx::Transform transform;
+  transform.Scale(2, 1);
+  transform.Translate(10, 5);
+
+  auto long_duration_animation =
+      std::make_unique<ui::ScopedAnimationDurationScaleMode>(
+          ui::ScopedAnimationDurationScaleMode::SLOW_DURATION);
+  {
+    ui::ScopedLayerAnimationSettings animation(layer_to_delete->GetAnimator());
+    animation.AddObserver(&animation_observer);
+    animation.SetTransitionDuration(base::TimeDelta::FromMilliseconds(1000));
+    layer_to_delete->SetTransform(transform);
+  }
+
+  // This call should not crash.
+  root->OnDeviceScaleFactorChanged(2.f);
+
+  animation_observer.SetLayer(CreateLayer(LAYER_SOLID_COLOR));
+  layer_to_delete = animation_observer.layer();
+
+  std::unique_ptr<Layer> child = CreateLayer(LAYER_SOLID_COLOR);
+
+  root->Add(layer_to_delete);
+  layer_to_delete->Add(child.get());
+
+  long_duration_animation =
+      std::make_unique<ui::ScopedAnimationDurationScaleMode>(
+          ui::ScopedAnimationDurationScaleMode::SLOW_DURATION);
+  {
+    ui::ScopedLayerAnimationSettings animation(layer_to_delete->GetAnimator());
+    animation.AddObserver(&animation_observer);
+    animation.SetTransitionDuration(base::TimeDelta::FromMilliseconds(1000));
+    layer_to_delete->SetTransform(transform);
+  }
+
+  // This call should not crash.
+  root->OnDeviceScaleFactorChanged(1.5f);
+
+  animation_observer.SetLayer(CreateLayer(LAYER_SOLID_COLOR));
+  layer_to_delete = animation_observer.layer();
+
+  std::unique_ptr<Layer> child2 = CreateLayer(LAYER_SOLID_COLOR);
+
+  root->Add(layer_to_delete);
+  layer_to_delete->Add(child.get());
+  layer_to_delete->Add(child2.get());
+
+  long_duration_animation =
+      std::make_unique<ui::ScopedAnimationDurationScaleMode>(
+          ui::ScopedAnimationDurationScaleMode::SLOW_DURATION);
+  {
+    ui::ScopedLayerAnimationSettings animation(child->GetAnimator());
+    animation.AddObserver(&animation_observer);
+    animation.SetTransitionDuration(base::TimeDelta::FromMilliseconds(1000));
+    child->SetTransform(transform);
+  }
+
+  // This call should not crash.
+  root->OnDeviceScaleFactorChanged(2.f);
+}
+
 // Tests that the animators in the layer tree is added to the
 // animator-collection when the root-layer is set to the compositor.
 TEST_F(LayerWithDelegateTest, RootLayerAnimatorsInCompositor) {