Set layer scroll data from PaintArtifactCompositor

third_party/WebKit/Source/platform/graphics/compositing/PropertyTreeManager.cpp

Index: third_party/WebKit/Source/platform/graphics/compositing/PropertyTreeManager.cpp
diff --git a/third_party/WebKit/Source/platform/graphics/compositing/PropertyTreeManager.cpp b/third_party/WebKit/Source/platform/graphics/compositing/PropertyTreeManager.cpp
index 088dab8095f3d033b4794e52939a16c00ca8eae9..b7f4b6bc72057a8d02026bff19a1177a27937f82 100644
--- a/third_party/WebKit/Source/platform/graphics/compositing/PropertyTreeManager.cpp
+++ b/third_party/WebKit/Source/platform/graphics/compositing/PropertyTreeManager.cpp
@@ -16,6 +16,7 @@
 #include "platform/graphics/paint/GeometryMapper.h"
 #include "platform/graphics/paint/ScrollPaintPropertyNode.h"
 #include "platform/graphics/paint/TransformPaintPropertyNode.h"
+#include "public/platform/WebLayerScrollClient.h"
 
 namespace blink {
 
@@ -264,12 +265,6 @@ int PropertyTreeManager::ensureCompositorScrollNode(
   int id = scrollTree().Insert(cc::ScrollNode(), parentId);
 
   cc::ScrollNode& compositorNode = *scrollTree().Node(id);
-  // TODO(wkorman): Fix owning layer id to reference a layer id rather than a
-  // scroll node index.
-  compositorNode.owning_layer_id = parentId;
-  m_propertyTrees
-      .layer_id_to_scroll_node_index[compositorNode.owning_layer_id] = id;
-
   compositorNode.scrollable = true;
 
   compositorNode.scroll_clip_layer_bounds.SetSize(scrollNode->clip().width(),
@@ -319,7 +314,38 @@ void PropertyTreeManager::updateScrollAndScrollTranslationNodes(
   compositorTransformNode.local.MakeIdentity();
   compositorTransformNode.scrolls = true;
   transformTree().set_needs_update(true);
-  // TODO(pdr): The scroll tree's scroll offset will need to be set here.
+  // TODO(pdr): Because of a layer dependancy, the scroll tree scroll offset is
+  // set in updateLayerScrollMapping but that should occur here.
+}
+
+void PropertyTreeManager::updateLayerScrollMapping(
+    cc::Layer* layer,
+    const TransformPaintPropertyNode* transform) {
+  auto* enclosingScrollNode = transform->findEnclosingScrollNode();
+  int scrollNodeId = ensureCompositorScrollNode(enclosingScrollNode);
+  layer->SetScrollTreeIndex(scrollNodeId);
+  int layerId = layer->id();
+  m_propertyTrees.layer_id_to_scroll_node_index[layerId] = scrollNodeId;
+
+  if (transform->isScrollTranslation()) {

[wkorman, 2017/02/16 22:36:22]

Could return here if false, to early-out and save indenting the below.

[pdr., 2017/02/16 23:08:27]

Done

+    auto& compositorScrollNode = *scrollTree().Node(scrollNodeId);
+    auto& compositorTransformNode =
+        *transformTree().Node(compositorScrollNode.transform_id);
+
+    // TODO(pdr): Remove the scroll node's owning_layer_id.
+    if (compositorScrollNode.owning_layer_id == cc::Layer::INVALID_ID) {

[wkorman, 2017/02/16 22:36:22]

So if it ever got an owning_layer_id previously we will never update it again.
Is that kosher? I have minimal background here, seemed worth checkpoint.

[pdr., 2017/02/16 23:08:27]

Good question, I should have called this out in more detail.

This is not really kosher but I hope the end justifies the means. This is a hack
to use the first layer a scroll node sees as it's owner in cases where two
layers use the same scroll node. This whole conditional is suspect but I expect
to clean it up in a followup when the scroll node -> layer logic is removed.

[ajuma, 2017/02/17 00:06:18]

Might be worth adding a comment about this.

+      compositorScrollNode.owning_layer_id = layerId;
+      // TODO(pdr): Set this in updateScrollAndScrollTranslationNodes once the
+      // layer id is no longer needed.
+      scrollTree().SetScrollOffset(layerId,
+                                   compositorTransformNode.scroll_offset);
+      if (auto* scrollClient = enclosingScrollNode->scrollClient()) {
+        layer->set_did_scroll_callback(
+            base::Bind(&blink::WebLayerScrollClient::didScroll,
+                       base::Unretained(scrollClient)));
+      }
+    }
+  }
 }
 
 int PropertyTreeManager::switchToEffectNode(