Add compositor element id for scroll nodes in SPv2.

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

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #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"
@@ skipping 68 matching lines @@
   DCHECK(!RuntimeEnabledFeatures::rootLayerScrollingEnabled());
   if (auto* existingScrollTranslation = frameView.scrollTranslation()) {
     existingScrollTranslation->update(std::move(parent), matrix, origin);
     return false;
   }
   frameView.setScrollTranslation(
       TransformPaintPropertyNode::create(std::move(parent), matrix, origin));
   return true;
 }
 
+static CompositorElementId createDomNodeBasedCompositorElementId(

[wkorman, 2017/01/19 23:43:10]

Moved here without changes so that it's in scope for new call from static method
below.

+    const LayoutObject& object) {
+  return createCompositorElementId(DOMNodeIds::idForNode(object.node()),
+                                   CompositorSubElementId::Primary);
+}
+
 // True if a new property was created or a main thread scrolling reason changed
 // (which can affect descendants), false if an existing one was updated.
 static bool updateScroll(
     FrameView& frameView,
     PassRefPtr<const ScrollPaintPropertyNode> parent,
     PassRefPtr<const TransformPaintPropertyNode> scrollOffset,
     const IntSize& clip,
     const IntSize& bounds,
     bool userScrollableHorizontal,
     bool userScrollableVertical,
     MainThreadScrollingReasons mainThreadScrollingReasons) {
   DCHECK(!RuntimeEnabledFeatures::rootLayerScrollingEnabled());
+
+  // TODO(wkorman): Should we generate compositor element id conditionally on

[wkorman, 2017/01/19 23:43:10]

Seeking feedback here. The general things I'm uncertain of though of course I
did research briefly:

- am I missing any ScrollPaintPropertyNode create/update calls?
- am I building the compositor element id from the correct layout object?
- are we assured that these are only created for composited scrolling cases? If
not, what should we be checking?

If we get either of these wrong we can correct in subsequent change but worth
considering now.

[pdr., 2017/01/20 06:26:32]

Nope, you got the two.
This is the non-RLS codepath so it's for FrameView scrolling. I'm really not
sure what Element a FrameView would have.
These are currently created for composited and non-composited scrollers, and
will be in the future too. With SPV2's design, we won't know until
PaintArtifactCompositor which scrollers actually end up being composited. Is
there any harm in setting this id if it isn't used?
+1

[ajuma, 2017/01/20 14:41:57]

The only downside I can think of is that each DOMNodeId we generate gets
inserted into a hash map (see
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/dom/DOMNo...),
so that hash map gets a bit bigger. So just something to keep in mind as a
possible performance issue we might want to look into later.

+  // something that connotes use of composited scrolling? Or pass it in as a
+  // param similar to scrolling reasons? See also similar comment in
+  // PaintPropertyTreeBuilder::updateScrollAndScrollTranslation.
+  CompositorElementId compositorElementId =
+      createDomNodeBasedCompositorElementId(*frameView.layoutView());
   if (auto* existingScroll = frameView.scroll()) {
     auto existingReasons = existingScroll->mainThreadScrollingReasons();
     existingScroll->update(std::move(parent), std::move(scrollOffset), clip,
                            bounds, userScrollableHorizontal,
-                           userScrollableVertical, mainThreadScrollingReasons);
+                           userScrollableVertical, mainThreadScrollingReasons,
+                           compositorElementId);
     return existingReasons != mainThreadScrollingReasons;
   }
   frameView.setScroll(ScrollPaintPropertyNode::create(
       std::move(parent), std::move(scrollOffset), clip, bounds,
       userScrollableHorizontal, userScrollableVertical,
-      mainThreadScrollingReasons));
+      mainThreadScrollingReasons, compositorElementId));
   return true;
 }
 
 static MainThreadScrollingReasons mainThreadScrollingReasons(
     const FrameView& frameView,
     MainThreadScrollingReasons ancestorReasons) {
   auto reasons = ancestorReasons;
   if (!frameView.frame().settings()->getThreadedScrollingEnabled())
     reasons |= MainThreadScrollingReason::kThreadedScrollingDisabled;
   if (frameView.hasBackgroundAttachmentFixedObjects())
@@ skipping 220 matching lines @@
   // Transform origin has no effect without a transform or motion path.
   if (!style.hasTransform())
     return FloatPoint3D();
   FloatSize borderBoxSize(box.size());
   return FloatPoint3D(
       floatValueForLength(style.transformOriginX(), borderBoxSize.width()),
       floatValueForLength(style.transformOriginY(), borderBoxSize.height()),
       style.transformOriginZ());
 }
 
-static CompositorElementId createDomNodeBasedCompositorElementId(
-    const LayoutObject& object) {
-  return createCompositorElementId(DOMNodeIds::idForNode(object.node()),
-                                   CompositorSubElementId::Primary);
-}
-
 void PaintPropertyTreeBuilder::updateTransform(
     const LayoutObject& object,
     PaintPropertyTreeBuilderContext& context) {
   if (object.isSVGChild()) {
     updateTransformForNonRootSVG(object, context);
     return;
   }
 
   if (object.needsPaintPropertyUpdate() || context.forceSubtreeUpdate) {
     const ComputedStyle& style = object.styleRef();
@@ skipping 419 matching lines @@
         bool userScrollableVertical =
             scrollableArea->userInputScrollable(VerticalScrollbar);
 
         // Main thread scrolling reasons depend on their ancestor's reasons
         // so ensure the entire subtree is updated when reasons change.
         if (auto* existingScrollNode = properties.scroll()) {
           if (existingScrollNode->mainThreadScrollingReasons() != reasons)
             context.forceSubtreeUpdate = true;
         }
 
+        // TODO(wkorman): Should we do this conditionally on something that

[wkorman, 2017/01/19 23:43:10]

And seeking feedback here.

[ajuma, 2017/01/20 14:41:57]

See comment above, but I think this is fine for now.

+        // connotes use of composited scrolling?
+        // ScrollableArea::usesCompositedScrolling is not currently SPv2
+        // compatible due to dependence on CompositedLayerMapping (see
+        // PaintLayerScrollableArea override).
+        CompositorElementId compositorElementId =
+            createDomNodeBasedCompositorElementId(object);
+
         context.forceSubtreeUpdate |= properties.updateScroll(
             context.current.scroll, properties.scrollTranslation(), scrollClip,
             scrollBounds, userScrollableHorizontal, userScrollableVertical,
-            reasons);
+            reasons, compositorElementId);
       }
     }
 
     if (!needsScrollProperties) {
       // Ensure pre-existing properties are cleared.
       if (auto* properties = object.getMutableForPainting().paintProperties()) {
         context.forceSubtreeUpdate |= properties->clearScrollTranslation();
         context.forceSubtreeUpdate |= properties->clearScroll();
       }
     }
@@ skipping 178 matching lines @@
   updateOverflowClip(object, context);
   updatePerspective(object, context);
   updateSvgLocalToBorderBoxTransform(object, context);
   updateScrollAndScrollTranslation(object, context);
   updateOutOfFlowContext(object, context);
 
   context.forceSubtreeUpdate |= object.subtreeNeedsPaintPropertyUpdate();
 }
 
 }  // namespace blink