[SPv1] Always set a CompositorElementId on main graphics layers; use PaintLayer id.

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 50a63a3e6fb39273db67ebc6f3932bc0193f7759..5ee36f4a9e1db48f3db9db87916ae3433da2fe8c 100644
--- a/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
+++ b/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
@@ -2199,6 +2199,36 @@ void CompositedLayerMapping::UpdateShouldFlattenTransform() {
   }
 }
 
+struct AnimatingData {
+  Persistent<Node> owning_node = nullptr;

[wkorman, 2017/05/18 21:31:57]

add STACK_ALLOCATED()? seems to be MO.

[chrishtr, 2017/05/18 22:15:31]

Done.

+  Persistent<Element> animating_element = nullptr;
+  const ComputedStyle* animating_style = nullptr;
+};
+
+void GetAnimatingData(PaintLayer& paint_layer, AnimatingData& data) {
+  if (!RuntimeEnabledFeatures::compositorWorkerEnabled())
+    return;
+
+  data.owning_node = paint_layer.GetLayoutObject().GetNode();
+
+  if (!data.owning_node)
+    return;
+
+  Document& document = data.owning_node->GetDocument();
+  Element* scrolling_element = document.ScrollingElementNoLayout();
+  if (data.owning_node->IsElementNode() &&
+      (!RuntimeEnabledFeatures::rootLayerScrollingEnabled() ||
+       data.owning_node != scrolling_element)) {
+    data.animating_element = ToElement(data.owning_node);
+    data.animating_style = paint_layer.GetLayoutObject().Style();
+  } else if (data.owning_node->IsDocumentNode() &&
+             RuntimeEnabledFeatures::rootLayerScrollingEnabled()) {
+    data.owning_node = data.animating_element = scrolling_element;
+    if (scrolling_element && scrolling_element->GetLayoutObject())
+      data.animating_style = scrolling_element->GetLayoutObject()->Style();
+  }
+}
+
 // Some background on when you receive an element id or mutable properties.
 //
 // element id:
@@ -2213,52 +2243,34 @@ void CompositedLayerMapping::UpdateShouldFlattenTransform() {
 // well as the mutable properties for all layers may change according to the
 // rules above so we update those values here.
 void CompositedLayerMapping::UpdateElementIdAndCompositorMutableProperties() {
-  int element_id = 0;
   uint32_t primary_mutable_properties = CompositorMutableProperty::kNone;
   uint32_t scroll_mutable_properties = CompositorMutableProperty::kNone;
 
-  Node* owning_node = owning_layer_.GetLayoutObject().GetNode();
-  Element* animating_element = nullptr;
-  const ComputedStyle* animating_style = nullptr;
-  if (owning_node) {
-    Document& document = owning_node->GetDocument();
-    Element* scrolling_element = document.ScrollingElementNoLayout();
-    if (owning_node->IsElementNode() &&
-        (!RuntimeEnabledFeatures::rootLayerScrollingEnabled() ||
-         owning_node != scrolling_element)) {
-      animating_element = ToElement(owning_node);
-      animating_style = owning_layer_.GetLayoutObject().Style();
-    } else if (owning_node->IsDocumentNode() &&
-               RuntimeEnabledFeatures::rootLayerScrollingEnabled()) {
-      owning_node = animating_element = scrolling_element;
-      if (scrolling_element && scrolling_element->GetLayoutObject())
-        animating_style = scrolling_element->GetLayoutObject()->Style();
-    }
-  }
+  AnimatingData data;
+  GetAnimatingData(owning_layer_, data);
+
+  CompositorElementId element_id;
+  if (data.animating_style && data.animating_style->HasCompositorProxy()) {
+    // Compositor proxy element ids cannot be based on PaintLayers, since
+    // those are not kept alive by script across frames.
+    element_id = CompositorElementIdFromDOMNodeId(
+        DOMNodeIds::IdForNode(data.owning_node),
+        CompositorElementIdNamespace::kPrimaryCompositorProxy);
 
-  if (RuntimeEnabledFeatures::compositorWorkerEnabled() && animating_style &&
-      animating_style->HasCompositorProxy()) {
     uint32_t compositor_mutable_properties =
-        animating_element->CompositorMutableProperties();
-    element_id = DOMNodeIds::IdForNode(owning_node);
+        data.animating_element->CompositorMutableProperties();
     primary_mutable_properties = (CompositorMutableProperty::kOpacity |
                                   CompositorMutableProperty::kTransform) &
                                  compositor_mutable_properties;
     scroll_mutable_properties = (CompositorMutableProperty::kScrollLeft |
                                  CompositorMutableProperty::kScrollTop) &
                                 compositor_mutable_properties;
+  } else {
+    element_id = CompositorElementIdFromPaintLayerId(

[wkorman, 2017/05/18 21:31:57]

Perhaps worth adding a comment here re: why we prefer using paint layer ids.
Could mention (1) fragmentation, (2) desire to avoid dom id map overhead.

Or could add the comment to where we define CompositorElementIdFromPaintLayerId
since we have another callsite below. You have the inverse on the dom node id
method definition (noting it should be used only in exceptional situations).

[chrishtr, 2017/05/18 22:15:31]

Done.

+        owning_layer_.UniqueId(), CompositorElementIdNamespace::kPrimary);
   }
 
-  if (animating_style && animating_style->ShouldCompositeForCurrentAnimations())
-    element_id = DOMNodeIds::IdForNode(owning_node);
-
-  CompositorElementId compositor_element_id;
-  if (element_id) {
-    compositor_element_id = CompositorElementIdFromDOMNodeId(
-        element_id, CompositorElementIdNamespace::kPrimary);
-  }
-
-  graphics_layer_->SetElementId(compositor_element_id);
+  graphics_layer_->SetElementId(element_id);
   graphics_layer_->SetCompositorMutableProperties(primary_mutable_properties);
 
   // We always set the elementId for m_scrollingContentsLayer since it can be
@@ -2380,13 +2392,21 @@ bool CompositedLayerMapping::UpdateScrollingLayers(
       scrolling_contents_layer_ =
           CreateGraphicsLayer(kCompositingReasonLayerForScrollingContents);
 
-      if (Node* owning_node = owning_layer_.GetLayoutObject().GetNode()) {
-        scrolling_contents_layer_->SetElementId(
-            CompositorElementIdFromDOMNodeId(
-                DOMNodeIds::IdForNode(owning_node),
-                CompositorElementIdNamespace::kScroll));
+      AnimatingData data;
+      GetAnimatingData(owning_layer_, data);
+
+      CompositorElementId element_id;
+      if (data.animating_style && data.animating_style->HasCompositorProxy()) {
+        element_id = CompositorElementIdFromDOMNodeId(
+            DOMNodeIds::IdForNode(data.owning_node),
+            CompositorElementIdNamespace::kScrollCompositorProxy);
+      } else {
+        element_id = CompositorElementIdFromPaintLayerId(
+            owning_layer_.UniqueId(), CompositorElementIdNamespace::kScroll);
       }
 
+      scrolling_contents_layer_->SetElementId(element_id);
+
       scrolling_layer_->AddChild(scrolling_contents_layer_.get());
 
       layer_changed = true;