Store compositor element id in paint properties for animated objects.

third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp

Index: third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
diff --git a/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp b/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
index b01a6cdf72ebf83e8f3cd03c97ff373f5a15b88f..2de00f63a0dfb08e25f844fea63477e766a2aa13 100644
--- a/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
+++ b/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
@@ -4,6 +4,7 @@
 
 #include "core/paint/PaintPropertyTreeBuilder.h"
 
+#include "core/dom/DOMNodeIds.h"
 #include "core/frame/FrameView.h"
 #include "core/frame/LocalFrame.h"
 #include "core/frame/Settings.h"
@@ -375,12 +376,17 @@ void PaintPropertyTreeBuilder::updateTransform(
         if (style.preserves3D() && !renderingContextId)
           renderingContextId = PtrHash<const LayoutObject>::hash(&object);
 
+        CompositorElementId compositorElementId;
+        if (style.hasCurrentTransformAnimation()) {
+          updateCompositorElementId(object, compositorElementId);
+        }
+
         auto& properties =
             object.getMutableForPainting().ensurePaintProperties();
         context.forceSubtreeUpdate |= properties.updateTransform(
             context.current.transform, matrix, transformOrigin(box),
             context.current.shouldFlattenInheritedTransform, renderingContextId,
-            compositingReasons);
+            compositingReasons, compositorElementId);
         hasTransform = true;
       }
     }
@@ -484,16 +490,23 @@ void PaintPropertyTreeBuilder::updateEffect(
     }
 
     CompositingReasons compositingReasons =
-        CompositingReasonFinder::requiresCompositingForEffectAnimation(
-            object.styleRef());
+        CompositingReasonFinder::requiresCompositingForEffectAnimation(style);
     if (compositingReasons != CompositingReasonNone)
       effectNodeNeeded = true;
 
+    CompositorElementId compositorElementId;

[pdr., 2017/01/03 21:29:04]

Could you do something like:
bool needsCompositorElementId = style.has...
CompositorElementId compositorElementId = needsCompositorElementId ?
createCompositorElementId(...) : 0;

[wkorman, 2017/01/03 22:04:40]

Yeah, sort of -- CompositorElementId is a struct with two ints, so it's not just
a '0' and it seems heavyweight to jockey a pointer around. I used updateXxx()
with ref param attempting to avoid a retval copy, and followed existing method
naming pattern, but I may have misinterpreted what that pattern was going for.

Really I just wanted a method to share the redundant logic around using DOM node
id as the compositor element id. So I renamed the method to
createDomNodeBasedCompositorElementId() and had it return the generated object
rather than mutating input param. It means we'll be copying a
CompositorElementId for the retval but probably that is cheap.

+    if (style.hasCurrentOpacityAnimation() ||
+        style.hasCurrentFilterAnimation() ||
+        style.hasCurrentBackdropFilterAnimation()) {
+      updateCompositorElementId(object, compositorElementId);
+    }
+
     if (effectNodeNeeded) {
       auto& properties = object.getMutableForPainting().ensurePaintProperties();
       context.forceSubtreeUpdate |= properties.updateEffect(
           context.currentEffect, context.current.transform, outputClip,
-          std::move(filter), opacity, blendMode, compositingReasons);
+          std::move(filter), opacity, blendMode, compositingReasons,
+          compositorElementId);
     } else {
       if (auto* properties = object.getMutableForPainting().paintProperties())
         context.forceSubtreeUpdate |= properties->clearEffect();
@@ -835,6 +848,13 @@ void PaintPropertyTreeBuilder::updateOutOfFlowContext(
   }
 }
 
+void PaintPropertyTreeBuilder::updateCompositorElementId(

[pdr., 2017/01/03 21:29:04]

The other update* functions follow a style where specific kinds of updates are
done. WDYT of just calling createCompositorElementId instead of adding this
function?

[wkorman, 2017/01/03 22:04:40]

See above comment.

[wkorman, 2017/01/03 22:15:24]

Moved to anonymous namespace and removed method from class definition.

+    const LayoutObject& object,
+    CompositorElementId& compositorElementId) {
+  compositorElementId = createCompositorElementId(
+      DOMNodeIds::idForNode(object.node()), CompositorSubElementId::Primary);
+}
+
 // Override ContainingBlockContext based on the properties of a containing block
 // that was previously walked in a subtree other than the current subtree being
 // walked. Used for out-of-flow positioned descendants of multi-column spanner