[SPv2] Decomposite otherwise-compositable animations that paint nothing.

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

Index: third_party/WebKit/Source/core/animation/Animation.cpp
diff --git a/third_party/WebKit/Source/core/animation/Animation.cpp b/third_party/WebKit/Source/core/animation/Animation.cpp
index 6fa80f83fe2cbd13ad14a62a28f32f07f826147f..68149b79e8dcbaec85678dd460499dc4fc634b77 100644
--- a/third_party/WebKit/Source/core/animation/Animation.cpp
+++ b/third_party/WebKit/Source/core/animation/Animation.cpp
@@ -34,6 +34,7 @@
 #include "core/animation/CompositorPendingAnimations.h"
 #include "core/animation/KeyframeEffectReadOnly.h"
 #include "core/animation/css/CSSAnimations.h"
+#include "core/dom/DOMNodeIds.h"
 #include "core/dom/Document.h"
 #include "core/dom/ExceptionCode.h"
 #include "core/dom/StyleChangeReason.h"
@@ -253,7 +254,10 @@ double Animation::unlimitedCurrentTimeInternal() const {
              : calculateCurrentTime();
 }
 
-bool Animation::preCommit(int compositorGroup, bool startOnCompositor) {
+bool Animation::preCommit(
+    int compositorGroup,
+    const Optional<CompositorElementIdSet>& compositedElementIds,
+    bool startOnCompositor) {
   PlayStateUpdateScope updateScope(*this, TimingUpdateOnDemand,
                                    DoNotSetCompositorPending);
 
@@ -291,10 +295,10 @@ bool Animation::preCommit(int compositorGroup, bool startOnCompositor) {
   if (shouldStart) {
     m_compositorGroup = compositorGroup;
     if (startOnCompositor) {
-      if (isCandidateForAnimationOnCompositor())
+      if (isCandidateForAnimationOnCompositor(compositedElementIds))
         createCompositorPlayer();
 
-      if (maybeStartAnimationOnCompositor())
+      if (maybeStartAnimationOnCompositor(compositedElementIds))
         m_compositorState = WTF::wrapUnique(new CompositorState(*this));
       else
         cancelIncompatibleAnimationsOnCompositor();
@@ -722,7 +726,8 @@ void Animation::forceServiceOnNextFrame() {
   m_timeline->wake();
 }
 
-bool Animation::canStartAnimationOnCompositor() const {
+bool Animation::canStartAnimationOnCompositor(
+    const Optional<CompositorElementIdSet>& compositedElementIds) const {
   if (m_isCompositedAnimationDisabledForTesting || effectSuppressed())
     return false;
 
@@ -730,21 +735,34 @@ bool Animation::canStartAnimationOnCompositor() const {
   if (m_playbackRate == 0 || (std::isinf(effectEnd()) && m_playbackRate < 0) ||
       (timeline() && timeline()->playbackRate() != 1))
     return false;
+  // If the optional element id set has no value we must be in SPv1 mode in
+  // which case we trust the compositing logic will create a layer if needed.

[alancutter (OOO until 2018), 2017/03/20 06:21:09]

We should assert the relevant RuntimeEnabledFeature status if it determines
has_value().

[wkorman, 2017/03/20 22:32:33]

Done.

+  if (compositedElementIds.has_value()) {
+    const KeyframeEffectReadOnly* effect =
+        toKeyframeEffectReadOnly(m_content.get());
+    CompositorElementId targetElementId =
+        createCompositorElementId(DOMNodeIds::idForNode(effect->target()),

[alancutter (OOO until 2018), 2017/03/20 06:21:09]

Need null check for target.

[wkorman, 2017/03/20 22:32:33]

What cases might lead it to be null, and if it's null, would we ever want to
allow compositing? I'm assuming not.

Most callsites to target() don't do any null checks:

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/animation...

though I do see some null checks internally on KeyframeEffectReadOnly for
example:

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/animation...

[alancutter (OOO until 2018), 2017/03/21 00:32:13]

Both the target element of an effect and the effect of an animation are nullable
in the Web Animations API. Both must be non-null for compositing (or any visible
effect at all) to occur.
Thanks for pointing out the iffy call sites, that's something animations team
should fix.

+                                  CompositorSubElementId::Primary);
+    if (!compositedElementIds->contains(targetElementId))
+      return false;
+  }
 
   return m_timeline && m_content && m_content->isKeyframeEffectReadOnly() &&
          playing();

[alancutter (OOO until 2018), 2017/03/20 06:21:09]

Put the new check after this check, this way m_content and its type is
guaranteed to be what you're expecting.

[wkorman, 2017/03/20 22:32:33]

Done.

 }
 
-bool Animation::isCandidateForAnimationOnCompositor() const {
-  if (!canStartAnimationOnCompositor())
+bool Animation::isCandidateForAnimationOnCompositor(
+    const Optional<CompositorElementIdSet>& compositedElementIds) const {
+  if (!canStartAnimationOnCompositor(compositedElementIds))
     return false;
 
   return toKeyframeEffectReadOnly(m_content.get())
       ->isCandidateForAnimationOnCompositor(m_playbackRate);
 }
 
-bool Animation::maybeStartAnimationOnCompositor() {
-  if (!canStartAnimationOnCompositor())
+bool Animation::maybeStartAnimationOnCompositor(
+    const Optional<CompositorElementIdSet>& compositedElementIds) {
+  if (!canStartAnimationOnCompositor(compositedElementIds))
     return false;
 
   bool reversed = m_playbackRate < 0;