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 d36b46bdf321ddeb49160399dbc4ad3b18f0d130..68bce2b36f7119d518a863ff3a6813da6a25cdbf 100644
--- a/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
+++ b/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
@@ -1522,34 +1522,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 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);
+        mainMutableProperties = (CompositorMutableProperty::kOpacity | CompositorMutableProperty::kTransform) & compositorMutableProperties;

[ajuma, 2016/06/01 23:53:42]

Nit: maybe call this something like primaryMutableProperties or
cssMutableProperties, to avoid making this sound like "properties mutable on the
main thread".

[Ian Vollick, 2016/06/02 19:03:11]

Done.

+        scrollMutableProperties = (CompositorMutableProperty::kScrollLeft | CompositorMutableProperty::kScrollTop) & compositorMutableProperties;
     }
 
-    m_graphicsLayer->setElementId(elementId);
+    if (m_owningLayer.layoutObject()->style()->shouldCompositeForCurrentAnimations() && owningNode)
+        elementId = DOMNodeIds::idForNode(owningNode);
+
+    m_graphicsLayer->setElementId(CompositorElementId(elementId));
     m_graphicsLayer->setCompositorMutableProperties(mainMutableProperties);
 
-    if (m_scrollingContentsLayer.get()) {
-        m_scrollingContentsLayer->setElementId(elementId);
+    // We always set the elementId for m_scrollingContentsLayer since it can be
+    // animated for smooth scrolling.
+    if (m_scrollingContentsLayer.get())
         m_scrollingContentsLayer->setCompositorMutableProperties(scrollMutableProperties);
-    }
 }
 
 bool CompositedLayerMapping::updateForegroundLayer(bool needsForegroundLayer)
@@ -1636,6 +1634,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(CompositorElementId(DOMNodeIds::idForNode(owningNode) | kScrollSubElement));

[Ian Vollick, 2016/06/01 18:08:26]

NB: this is some of the glue code that knows about sub elements.

[ajuma, 2016/06/01 23:53:42]

I'm probably missing something obvious, but shouldn't kScrollSubElement be
shifted before OR-ing with the DOMNodeId?

[Ian Vollick, 2016/06/02 19:03:11]

It should indeed be shifted. I'd started implementing option 1 (storing multiple
values in ElementId) for a bit, but backtracked when I saw the shape of the code
in cc. When I did that, I failed to put the shifting back and I just happened to
get lucky with no collisions. I've fixed this (see CompositorElementId), and
added a test in CompositorMutableStateTest that checks for this sort of
collision in case I accidentally get rid of the shifting again.

+                m_scrollingContentsLayer->setCompositorMutableProperties(CompositorMutableProperty::kScrollLeft | CompositorMutableProperty::kScrollTop);
+            }
+
             m_scrollingLayer->addChild(m_scrollingContentsLayer.get());
 
             layerChanged = true;