Use element id's for animations

third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp

Index: third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
diff --git a/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp b/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
index f6ba036b0fba588ba276e462646a35220160c694..35dccce0ab27dbc298936b63435d1d183be15650 100644
--- a/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
+++ b/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
@@ -1523,34 +1523,32 @@ void CompositedLayerMapping::updateShouldFlattenTransform()
 
 void CompositedLayerMapping::updateElementIdAndCompositorMutableProperties()
 {
-    if (!RuntimeEnabledFeatures::compositorWorkerEnabled())
-        return;
-
-    TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("compositor-worker"), "CompositedLayerMapping::updateElementId()");
-
     uint64_t elementId = 0;
-    uint32_t mainMutableProperties = CompositorMutableProperty::kNone;
+    uint32_t primaryMutableProperties = CompositorMutableProperty::kNone;
     uint32_t scrollMutableProperties = CompositorMutableProperty::kNone;
 
-    if (m_owningLayer.layoutObject()->style()->hasCompositorProxy()) {
-        if (Node* owningNode = m_owningLayer.layoutObject()->generatingNode()) {
-            if (owningNode->isElementNode()) {
-                Element* owningElement = toElement(owningNode);
-                uint32_t compositorMutableProperties = owningElement->compositorMutableProperties();
-                elementId = DOMNodeIds::idForNode(owningNode);
-                mainMutableProperties = (CompositorMutableProperty::kOpacity | CompositorMutableProperty::kTransform) & compositorMutableProperties;
-                scrollMutableProperties = (CompositorMutableProperty::kScrollLeft | CompositorMutableProperty::kScrollTop) & compositorMutableProperties;
-            }
-        }
+    Node* owningNode = m_owningLayer.layoutObject()->generatingNode();
+    Element* owningElement = nullptr;
+    if (owningNode && owningNode->isElementNode())
+        owningElement = toElement(owningNode);
+
+    if (RuntimeEnabledFeatures::compositorWorkerEnabled() && owningElement && m_owningLayer.layoutObject()->style()->hasCompositorProxy()) {
+        uint32_t compositorMutableProperties = owningElement->compositorMutableProperties();
+        elementId = DOMNodeIds::idForNode(owningNode);
+        primaryMutableProperties = (CompositorMutableProperty::kOpacity | CompositorMutableProperty::kTransform) & compositorMutableProperties;
+        scrollMutableProperties = (CompositorMutableProperty::kScrollLeft | CompositorMutableProperty::kScrollTop) & compositorMutableProperties;
     }
 
-    m_graphicsLayer->setElementId(elementId);
-    m_graphicsLayer->setCompositorMutableProperties(mainMutableProperties);
+    if (m_owningLayer.layoutObject()->style()->shouldCompositeForCurrentAnimations() && owningNode)
+        elementId = DOMNodeIds::idForNode(owningNode);
 
-    if (m_scrollingContentsLayer.get()) {
-        m_scrollingContentsLayer->setElementId(elementId);
+    m_graphicsLayer->setElementId(createCompositorElementId(elementId, CompositorSubElementId::Primary));

[ajuma, 2016/06/07 13:44:01]

This is implicitly relying on Primary having value 0, so layers that aren't
animated and don't have a compositor proxy get an element id of (0, 0) (and
don't end up getting added to the element-id-to-layer map in cc). I think it'd
be safer to check if elementId is still 0 here, and if so set element id to a
default-constructed CompositorElementId().

[Ian Vollick, 2016/06/07 14:30:08]

This is perhaps too subtle, but operator bool for ElementId's only depends on
the primaryId. As long as elementId is zero, I don't think we'll be adding it to
the element id to layer map. I think what you suggest is a great idea, though,
and I've updated the code, just to be safe.

+    m_graphicsLayer->setCompositorMutableProperties(primaryMutableProperties);
+
+    // We always set the elementId for m_scrollingContentsLayer since it can be
+    // animated for smooth scrolling.
+    if (m_scrollingContentsLayer.get())
         m_scrollingContentsLayer->setCompositorMutableProperties(scrollMutableProperties);

[ajuma, 2016/06/07 13:44:01]

How does this relate to the logic added to ::updateScrollingLayers below? It
looks like scrollMutableProperties has value kNone here (unless the layer has a
compositor proxy). Should this really be kScrollLeft | kScrollTop? (Or can we
rely on the logic below to set up scroll layers correctly, and in that case do
we need the logic here at all?)

[Ian Vollick, 2016/06/07 14:30:08]

Good question.

 * element id: if you have a compositor proxy, an animation, or you're a
scroller (and might impl animate).
 * mutable properties: if you have a compositor proxy.

So I've set up the element id for the scroll layers when they're constructed,
but I conditionally set up the element id for the primary layer as well as the
mutable properties for all layers based on the rules above.

I've added more detailed comments about this.

-    }
 }
 
 bool CompositedLayerMapping::updateForegroundLayer(bool needsForegroundLayer)
@@ -1637,6 +1635,12 @@ bool CompositedLayerMapping::updateScrollingLayers(bool needsScrollingLayers)
 
             // Inner layer which renders the content that scrolls.
             m_scrollingContentsLayer = createGraphicsLayer(CompositingReasonLayerForScrollingContents);
+
+            if (Node* owningNode = m_owningLayer.layoutObject()->generatingNode()) {
+                m_scrollingContentsLayer->setElementId(createCompositorElementId(DOMNodeIds::idForNode(owningNode), CompositorSubElementId::Scroll));
+                m_scrollingContentsLayer->setCompositorMutableProperties(CompositorMutableProperty::kScrollLeft | CompositorMutableProperty::kScrollTop);
+            }
+
             m_scrollingLayer->addChild(m_scrollingContentsLayer.get());
 
             layerChanged = true;