Fix a mistake in computing main thread compositable animations

At this moment, this is how we check whether an animation is main thread
compositable or not:
1. The runtime feature TurnOff2DAndOpacityCompositorAnimationsEnabled is on
2. There is either an opacity or 2d transform animation

There is a problem with this logic. When there is an opacity animation
on an element and the element has "will-change: transform", we will put
this animation as main thread compositable. This is not true, we should
categorize it as composited animation because "will-change: transform"
will create a layer anyways.

This CL fixes this problem. We care about the most common cases:
"will-change: transform" and 3D transform. So step 2 in the above
becomes: when there is no "will-change: transform" and there is no 3D
transform, and there is either opacity or 2D transform, then this is a
main thread compositable animation.

This CL also added a test to verify that an element with
"will-change: transform" + opacity animation will end up having a
composited animation even when the runtime feature is on.

Bug: 818809
Change-Id: Ifcd027223df96c17447955eb52568d74bb5505ff
Reviewed-on: https://chromium-review.googlesource.com/951719
Commit-Queue: Xida Chen <xidachen@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545612}

third_party/WebKit/Source/core/animation/CompositorAnimations.cpp

@@ -152,23 +152,6 @@ CompositorAnimations::CheckCanStartEffectOnCompositor(
     const Animation* animation_to_add,
     const EffectModel& effect,
     double animation_playback_rate) {
-  // Check whether this animation is main thread compositable or not.
-  // If this runtime feature is on + we have either opacity or 2d transform
-  // animation, then this animation is main thread compositable.
-  if (RuntimeEnabledFeatures::
-          TurnOff2DAndOpacityCompositorAnimationsEnabled()) {
-    LayoutObject* layout_object = target_element.GetLayoutObject();
-    if (layout_object) {
-      const ComputedStyle* style = layout_object->Style();
-      if (style && (style->HasCurrentOpacityAnimation() ||
-                    (style->HasCurrentTransformAnimation() &&
-                     !style->Has3DTransform()))) {
-        return FailureCode::AcceleratableAnimNotAccelerated(
-            "Acceleratable animation not accelerated due to an experiment");
-      }
-    }
-  }
-
   const KeyframeEffectModelBase& keyframe_effect =
       ToKeyframeEffectModelBase(effect);
 
@@ -272,6 +255,24 @@ CompositorAnimations::CheckCanStartEffectOnCompositor(
         "animations");
   }
 
+  if (RuntimeEnabledFeatures::
+          TurnOff2DAndOpacityCompositorAnimationsEnabled()) {
+    LayoutObject* layout_object = target_element.GetLayoutObject();
+    if (layout_object && layout_object->PaintingLayer()) {
+      CompositingReasons compositing_reasons =
+          layout_object->PaintingLayer()->GetCompositingReasons();
+      bool has_other_compositing_reasons =
+          compositing_reasons != CompositingReason::kNone;
+      // If we are already compositing the element for other reasons, then not
+      // starting the animation on the compositor will not save memory and will
+      // have worse performance.
+      if (!has_other_compositing_reasons) {
+        return FailureCode::AcceleratableAnimNotAccelerated(
+            "Acceleratable animation not accelerated due to an experiment");
+      }
+    }
+  }
+
   return FailureCode::None();
 }
 

third_party/WebKit/Source/core/animation/CompositorAnimations.h

@@ -164,6 +164,10 @@ class CORE_EXPORT CompositorAnimations {
   FRIEND_TEST_ALL_PREFIXES(
       AnimationCompositorAnimationsTest,
       cannotStartElementOnCompositorEffectWithRuntimeFeature);
+  FRIEND_TEST_ALL_PREFIXES(AnimationCompositorAnimationsTest,
+                           canStartOpacityWithWillChangeWithRuntimeFeature);
+  FRIEND_TEST_ALL_PREFIXES(AnimationCompositorAnimationsTest,
+                           canStartOpacityWith3DTransformWithRuntimeFeature);
   FRIEND_TEST_ALL_PREFIXES(AnimationCompositorAnimationsTest,
                            cancelIncompatibleCompositorAnimations);
 };

third_party/WebKit/Source/core/animation/CompositorAnimationsTest.cpp

@@ -49,6 +49,7 @@
 #include "core/frame/WebLocalFrameImpl.h"
 #include "core/layout/LayoutObject.h"
 #include "core/paint/ObjectPaintProperties.h"
+#include "core/paint/PaintLayer.h"
 #include "core/style/ComputedStyle.h"
 #include "core/style/FilterOperations.h"
 #include "core/testing/CoreUnitTestHelper.h"
@@ -1405,9 +1406,9 @@ TEST_F(AnimationCompositorAnimationsTest, canStartElementOnCompositorEffect) {
   EXPECT_EQ(host->GetCompositedAnimationsCountForTesting(), 1u);
 }
 
-// Regression test for crbug.com/781305. When we have a transform animation
-// on a SVG element, the effect can be started on compositor but the element
-// itself cannot. The animation should not be a main thread compositable
+// Regression test for https://crbug.com/781305. When we have a transform
+// animation on a SVG element, the effect can be started on compositor but the
+// element itself cannot. The animation should not be a main thread compositable
 // animation.
 TEST_F(AnimationCompositorAnimationsTest,
        cannotStartElementOnCompositorEffectSVG) {
@@ -1427,6 +1428,49 @@ TEST_F(AnimationCompositorAnimationsTest,
   EXPECT_EQ(host->GetCompositedAnimationsCountForTesting(), 0u);
 }
 
+// A helper function for the next two tests to avoid duplicate code
+void CanStartOpacityTestHelper(CompositorAnimations::FailureCode code,
+                               Document* document,
+                               size_t composited_animations_count) {
+  EXPECT_EQ(code, CompositorAnimations::FailureCode::None());
+  EXPECT_EQ(document->Timeline().PendingAnimationsCount(), 1u);
+  EXPECT_EQ(document->Timeline().MainThreadCompositableAnimationsCount(), 0u);
+  CompositorAnimationHost* host =
+      document->View()->GetCompositorAnimationHost();
+  EXPECT_EQ(host->GetMainThreadAnimationsCountForTesting(), 0u);
+  EXPECT_EQ(host->GetMainThreadCompositableAnimationsCountForTesting(), 0u);
+  EXPECT_EQ(host->GetCompositedAnimationsCountForTesting(),
+            composited_animations_count);
+}
+
+// Regression tests for https://crbug.com/818809. When an element has an
+// animation that will be composited regardless of an experiment (such as
+// will-change: transform), then an opacity animation on this element will be
+// composited as well.
+TEST_F(AnimationCompositorAnimationsTest,
+       canStartOpacityWithWillChangeWithRuntimeFeature) {
+  ScopedTurnOff2DAndOpacityCompositorAnimationsForTest
+      turn_off_2d_and_opacity_compositors_animation(true);
+  LoadTestData("opacity-with-will-change-transform.html");
+  Document* document = GetFrame()->GetDocument();
+  Element* target = document->getElementById("target");
+  CompositorAnimations::FailureCode code =
+      CompositorAnimations::CheckCanStartElementOnCompositor(*target);
+  CanStartOpacityTestHelper(code, document, 1u);
+}
+
+TEST_F(AnimationCompositorAnimationsTest,
+       canStartOpacityWith3DTransformWithRuntimeFeature) {
+  ScopedTurnOff2DAndOpacityCompositorAnimationsForTest
+      turn_off_2d_and_opacity_compositors_animation(true);
+  LoadTestData("opacity-with-3d-transform.html");
+  Document* document = GetFrame()->GetDocument();
+  Element* target = document->getElementById("target");
+  CompositorAnimations::FailureCode code =
+      CompositorAnimations::CheckCanStartElementOnCompositor(*target);
+  CanStartOpacityTestHelper(code, document, 2u);
+}
+
 TEST_F(AnimationCompositorAnimationsTest,
        cannotStartAnimationOnCompositorWithRuntimeFeature) {
   ScopedTurnOff2DAndOpacityCompositorAnimationsForTest
@@ -1437,16 +1481,15 @@ TEST_F(AnimationCompositorAnimationsTest,
   const ObjectPaintProperties* properties =
       target->GetLayoutObject()->FirstFragment().PaintProperties();
   EXPECT_TRUE(properties->Transform()->HasDirectCompositingReasons());
-  Timing timing;
-  KeyframeEffectModelBase* effect =
-      StringKeyframeEffectModel::Create(StringKeyframeVector());
-  CompositorAnimations::FailureCode code =
-      CompositorAnimations::CheckCanStartAnimationOnCompositor(
-          timing, *target, nullptr, *effect, 1.0);
-  EXPECT_EQ(
-      code,
-      CompositorAnimations::FailureCode::AcceleratableAnimNotAccelerated(
-          "Acceleratable animation not accelerated due to an experiment"));
+  LayoutObject* layout_object = target->GetLayoutObject();
+  DCHECK(layout_object && layout_object->PaintingLayer());
+  // LoadTestData calls ForceFullCompositingUpdate, which have called
+  // CheckCanStartAnimationOnCompositor on the target element. In this test,
+  // there is one single transform animation, and it should not be composited
+  // because the runtime flag is on.
+  CompositingReasons compositing_reasons =
+      layout_object->PaintingLayer()->GetCompositingReasons();
+  EXPECT_EQ(compositing_reasons, CompositingReason::kNone);
   EXPECT_EQ(document->Timeline().PendingAnimationsCount(), 1u);
   EXPECT_EQ(document->Timeline().MainThreadCompositableAnimationsCount(), 1u);
   CompositorAnimationHost* host =

third_party/WebKit/Source/core/animation/test_data/opacity-with-3d-transform.html

+<!DOCTYPE html>
+<style>
+@keyframes test {
+  0% { opacity: 0; transform: rotateX(0deg); }
+  100% { opacity: 1; transform: rotateX(90deg); }
+}
+
+.animate {
+  animation-name: test;
+  animation-duration: 1s;
+}
+</style>
+<div id="target" class="animate"></div>;

third_party/WebKit/Source/core/animation/test_data/opacity-with-will-change-transform.html

+<!DOCTYPE html>
+<style>
+@keyframes test {
+  0% { opacity: 0; }
+  100% { opacity: 1; }
+}
+
+.animate {
+  animation-name: test;
+  animation-duration: 1s;
+}
+</style>
+<div id="target" style="will-change: transform;" class="animate"></div>;