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

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 e61d98d7d9afa174a98d946ae9132cedf2e53e79..530d136ab38b54e2e1309b9982452db0bf49efe6 100644
--- a/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
+++ b/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
@@ -74,10 +74,11 @@ static bool UpdateContentClip(
 
 static CompositorElementId CreateDomNodeBasedCompositorElementId(

[wkorman, 2017/05/18 00:45:58]

Suggest rename to CreatePaintLayerBasedCompositorElementId() or something, to
match logic change.

[chrishtr, 2017/05/18 01:25:47]

Done.

     const LayoutObject& object) {
+  DCHECK(object.IsBoxModelObject() && object.HasLayer());
   // TODO(wkorman): Centralize this implementation with similar across

[wkorman, 2017/05/18 00:45:58]

Maybe we can remove this now. It's not looking to me like we'd want to share
more than we do currently, but open to thoughts.

I mean, maybe there are a few other places where we use
ToLayoutBoxModelObject(object).Layer()->UniqueId(), and we could move this
method somewhere in CompositorElementId.h and have all of those use it, but not
sure it's worth it.

When I wrote the TODO I think we were using DOMNodeIds everywhere when creating
a CompositorElementId.

[chrishtr, 2017/05/18 01:25:47]

Done.

   // animation, scrolling and compositing logic.
-  return CompositorElementIdFromDOMNodeId(
-      DOMNodeIds::IdForNode(object.GetNode()),
+  return CompositorElementIdFromPaintLayerId(
+      ToLayoutBoxModelObject(object).Layer()->UniqueId(),
       CompositorElementIdNamespace::kPrimary);
 }
 
@@ -1195,7 +1196,7 @@ void PaintPropertyTreeBuilder::UpdatePaintProperties(
   if (needs_paint_properties && !had_paint_properties) {
     ObjectPaintProperties& paint_properties =
         object.GetMutableForPainting().EnsurePaintProperties();
-    if (RuntimeEnabledFeatures::slimmingPaintV2Enabled()) {
+    if (RuntimeEnabledFeatures::slimmingPaintV2Enabled() && object.HasLayer()) {

[wkorman, 2017/05/18 00:45:58]

This means we will stop setting compositor element ids on transform/effect nodes
in SPv2 when the involved layout object has no PaintLayer, right?

I think it also means that the majority of element ids we see in SPv2 will now
be paint layer ids rather than dom node ids. I only mention this because it
means we'll stop pounding the WeakIdentifierMap for DOM node ids.

Both seem fine, just noting while reading. Suggest noting both in CL
description. It currently only mentions SPv1 logic changes. Changes here still
have no ramification for SPv1 despite spinvalidation, right?

Do currently-passing animation layout tests in SPv2 still pass with the change?

[chrishtr, 2017/05/18 01:25:47]

No, because transforms and effects all induce PaintLayers. See:

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/La...
Yes. This is the reason I made the WeakIdentifierMap at the same time as
setting it unconditionally in SPv1 mode - to avoid a perf regression.
Haven't checked. Let's see what CQ dry run says.

       paint_properties.SetCompositorElementId(
           CreateDomNodeBasedCompositorElementId(object));
     }