Do not send OnLayerTransformed when the transform doesn't change

On some occasions, ui::Layer gets a transform which is identical
to the original. Setting and handling transform causes some
tasks, and that doesn't make much sense to do so when the
transform doesn't change actually.

It seems a few of unittests expect to get invoked, so this CL
also changes those expectations. This wouldn't change the actual
system behavior.

Bug: 1082562
Test: compositor_unittests, aura_unittests, views_unittests
Change-Id: Ia83b3b2ba87aa9825da6e2862aee3db9c665c72b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2225227
Commit-Queue: Jun Mukai <mukai@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#774331}

ui/aura/window_unittest.cc

@@ -2232,17 +2232,14 @@ TEST_F(WindowObserverTest, SetTransformAnimation) {
   EXPECT_EQ(target_transform,
             window_target_transform_changing_info().new_transform);
 
-  ASSERT_EQ(1, window_transformed_info().changed_count);
-  EXPECT_EQ(window.get(), window_transformed_info().window);
-  EXPECT_EQ(ui::PropertyChangeReason::FROM_ANIMATION,
-            window_transformed_info().reason);
+  ASSERT_EQ(0, window_transformed_info().changed_count);
 
   window->layer()->GetAnimator()->StopAnimatingProperty(
       ui::LayerAnimationElement::TRANSFORM);
 
   EXPECT_EQ(1, window_target_transform_changing_info().changed_count);
 
-  ASSERT_EQ(2, window_transformed_info().changed_count);
+  ASSERT_EQ(1, window_transformed_info().changed_count);
   EXPECT_EQ(window.get(), window_transformed_info().window);
   EXPECT_EQ(ui::PropertyChangeReason::FROM_ANIMATION,
             window_transformed_info().reason);

ui/compositor/layer.cc

@@ -1385,10 +1385,12 @@ void Layer::SetBoundsFromAnimation(const gfx::Rect& bounds,
     reflecting_layer->MatchLayerSize(this);
 }
 
-void Layer::SetTransformFromAnimation(const gfx::Transform& transform,
+void Layer::SetTransformFromAnimation(const gfx::Transform& new_transform,
                                       PropertyChangeReason reason) {
-  const gfx::Transform old_transform = this->transform();
-  cc_layer_->SetTransform(transform);
+  const gfx::Transform old_transform = transform();
+  if (old_transform == new_transform)
+    return;
+  cc_layer_->SetTransform(new_transform);
 
   // Skip recomputing position if the subpixel offset does not need updating
   // which is the case if an explicit offset is set.

ui/compositor/layer.h

@@ -527,7 +527,7 @@ class COMPOSITOR_EXPORT Layer : public LayerAnimationDelegate,
   // Implementation of LayerAnimatorDelegate
   void SetBoundsFromAnimation(const gfx::Rect& bounds,
                               PropertyChangeReason reason) override;
-  void SetTransformFromAnimation(const gfx::Transform& transform,
+  void SetTransformFromAnimation(const gfx::Transform& new_transform,
                                  PropertyChangeReason reason) override;
   void SetOpacityFromAnimation(float opacity,
                                PropertyChangeReason reason) override;

ui/compositor/layer_unittest.cc

@@ -2910,7 +2910,7 @@ TEST(LayerDelegateTest, OnLayerTransformed) {
     layer->SetTransform(target_transform1);
   }
   gfx::Transform target_transform2;
-  target_transform2.Skew(10.0f, 5.0f);
+  target_transform2.Skew(15.0f, 5.0f);
   EXPECT_CALL(delegate,
               OnLayerTransformed(target_transform1,
                                  PropertyChangeReason::NOT_FROM_ANIMATION))
@@ -2923,6 +2923,18 @@ TEST(LayerDelegateTest, OnLayerTransformed) {
   layer->SetTransform(target_transform2);
 }
 
+// Verify that LayerDelegate::OnLayerTransformed() is not called when the
+// transform isn't actually changed.
+TEST(LayerDelegateTest, OnLayerTransformedNotCalledWhenUnchanged) {
+  auto layer = std::make_unique<Layer>(LAYER_TEXTURED);
+  testing::StrictMock<TestLayerDelegate> delegate;
+  layer->set_delegate(&delegate);
+
+  gfx::Transform target_transform;
+  EXPECT_CALL(delegate, OnLayerTransformed).Times(0);
+  layer->SetTransform(target_transform);
+}
+
 // Verify that LayerDelegate::OnLayerTransformed() is called at every step of a
 // non-threaded transform transition.
 TEST(LayerDelegateTest, OnLayerTransformedNonThreadedAnimation) {
@@ -2996,8 +3008,8 @@ TEST(LayerDelegateTest, OnLayerTransformedNonThreadedAnimation) {
   testing::Mock::VerifyAndClear(&delegate);
 }
 
-// Verify that LayerDelegate::OnLayerTransformed() is called at the beginning
-// and at the end of a threaded transform transition.
+// Verify that LayerDelegate::OnLayerTransformed() is called at the end of a
+// threaded transform transition.
 TEST(LayerDelegateTest, OnLayerTransformedThreadedAnimation) {
   ScopedAnimationDurationScaleMode scoped_animation_duration_scale_mode(
       ScopedAnimationDurationScaleMode::NORMAL_DURATION);
@@ -3019,17 +3031,12 @@ TEST(LayerDelegateTest, OnLayerTransformedThreadedAnimation) {
           target_transform, base::TimeDelta::FromSeconds(1));
   ASSERT_TRUE(element->IsThreaded(layer.get()));
   LayerAnimationElement* element_raw = element.get();
+  // At the beginning, setting the transform actually does not change, so
+  // OnLayerTransformed isn't called.
   EXPECT_CALL(delegate,
               OnLayerTransformed(gfx::Transform(),
                                  PropertyChangeReason::FROM_ANIMATION))
-      .WillOnce(testing::Invoke([&](const gfx::Transform& old_transform,
-                                    PropertyChangeReason) {
-        // Verify that |layer->transform()| returns the correct value when the
-        // delegate is notified.
-        EXPECT_EQ(layer->transform(), initial_transform);
-        EXPECT_TRUE(
-            animator->IsAnimatingProperty(LayerAnimationElement::TRANSFORM));
-      }));
+      .Times(0);
   animator->StartAnimation(new LayerAnimationSequence(std::move(element)));
   testing::Mock::VerifyAndClear(&delegate);
   test_controller.StartThreadedAnimationsIfNeeded();