Pinch/Zoom Infrastructure & Plumbing CL

cc/trees/layer_tree_host.cc

Index: cc/trees/layer_tree_host.cc
diff --git a/cc/trees/layer_tree_host.cc b/cc/trees/layer_tree_host.cc
index 55524af0065f84f0717c1094292a6a4f3a6c8a12..4498bb6124b02fb9556edce07bfd682befce71b0 100644
--- a/cc/trees/layer_tree_host.cc
+++ b/cc/trees/layer_tree_host.cc
@@ -332,12 +332,7 @@ void LayerTreeHost::FinishCommitOnImplThread(LayerTreeHostImpl* host_impl) {
   sync_tree->set_background_color(background_color_);
   sync_tree->set_has_transparent_background(has_transparent_background_);
 
-  sync_tree->FindRootScrollLayer();

[aelias_OOO_until_Jul13, 2014/01/16 03:44:04]

I don't follow why this can be removed (particularly for the case where your new
flag isn't specified), can you put it back or explain why it's no longer needed?

[wjmaclean, 2014/01/16 15:07:32]

The functions that register the inner/outerviewport layers explicitly tell the
trees which layers are which, so there's no longer any need for a "discovery"
mechanism.

-
-  // TODO(wjmaclean) For now, not all LTH clients will register viewports, so
-  // only set them when available..
-  if (page_scale_layer_) {
-    DCHECK(inner_viewport_scroll_layer_);
+  if (page_scale_layer_ && inner_viewport_scroll_layer_) {

[aelias_OOO_until_Jul13, 2014/01/16 03:44:04]

I suggest doing DCHECK(inner_viewport_scroll_layer_) instead if it can never
happen to have one without the other.

[wjmaclean, 2014/01/16 15:07:32]

There are tests that do not explicitly specify these layers.

     sync_tree->SetViewportLayersFromIds(
         page_scale_layer_->id(),
         inner_viewport_scroll_layer_->id(),
@@ -1039,8 +1034,8 @@ void LayerTreeHost::ApplyScrollAndScale(const ScrollAndScaleSet& info) {
   if (!root_layer_.get())
     return;
 
-  gfx::Vector2d root_scroll_delta;
-  Layer* root_scroll_layer = FindFirstScrollableLayer(root_layer_.get());
+  gfx::Vector2d inner_viewport_scroll_delta;
+  gfx::Vector2d outer_viewport_scroll_delta;
 
   for (size_t i = 0; i < info.scrolls.size(); ++i) {
     Layer* layer =
@@ -1048,29 +1043,86 @@ void LayerTreeHost::ApplyScrollAndScale(const ScrollAndScaleSet& info) {
                                                 info.scrolls[i].layer_id);
     if (!layer)
       continue;
-    if (layer == root_scroll_layer) {
-      root_scroll_delta += info.scrolls[i].scroll_delta;
+    if (layer == outer_viewport_scroll_layer_.get()) {
+      outer_viewport_scroll_delta += info.scrolls[i].scroll_delta;
+    } else if (layer == inner_viewport_scroll_layer_.get()) {
+      inner_viewport_scroll_delta += info.scrolls[i].scroll_delta;
     } else {
       layer->SetScrollOffsetFromImplSide(layer->scroll_offset() +
                                          info.scrolls[i].scroll_delta);
     }
   }
 
-  if (!root_scroll_delta.IsZero() || info.page_scale_delta != 1.f) {
+  if (!inner_viewport_scroll_delta.IsZero() ||
+      !outer_viewport_scroll_delta.IsZero() ||
+      info.page_scale_delta != 1.f) {
     // SetScrollOffsetFromImplSide above could have destroyed the tree,
     // so re-get this layer before doing anything to it.
-    root_scroll_layer = FindFirstScrollableLayer(root_layer_.get());
 
     // Preemptively apply the scroll offset and scale delta here before sending
     // it to the client.  If the client comes back and sets it to the same
     // value, then the layer can early out without needing a full commit.
-    if (root_scroll_layer) {
-      root_scroll_layer->SetScrollOffsetFromImplSide(
-          root_scroll_layer->scroll_offset() + root_scroll_delta);
+    DCHECK(inner_viewport_scroll_layer_);  // We should always have this.
+
+    inner_viewport_scroll_layer_->SetScrollOffsetFromImplSide(
+        inner_viewport_scroll_layer_->scroll_offset() +
+        inner_viewport_scroll_delta);
+    if (outer_viewport_scroll_layer_) {
+      outer_viewport_scroll_layer_->SetScrollOffsetFromImplSide(
+          outer_viewport_scroll_layer_->scroll_offset() +
+          outer_viewport_scroll_delta);
     }
     ApplyPageScaleDeltaFromImplSide(info.page_scale_delta);
-    client_->ApplyScrollAndScale(root_scroll_delta, info.page_scale_delta);
+
+    client_->ApplyScrollAndScale(
+        inner_viewport_scroll_delta + outer_viewport_scroll_delta,

[aelias_OOO_until_Jul13, 2014/01/16 03:44:04]

Blink will ultimately have to be aware of the distinction between inner and
outer viewport scrolls for everything to work properly.  This API should be
updated to provide both values separately.

[wjmaclean, 2014/01/16 15:07:32]

Again, it was always my understanding that people were *against* Blink knowing
about the separation of the layers. Either is a valid design decision, but given
that we went the other way, this becomes a big change.

+        info.page_scale_delta);
+  }
+}
+
+gfx::Vector2d LayerTreeHost::DistributeScrollOffsetToViewports(

[aelias_OOO_until_Jul13, 2014/01/16 03:44:04]

As mentioned above, I don't think this function should exist here.

[wjmaclean, 2014/01/16 15:07:32]

Understood. Whether it remains or not will need to depend on the resolution of
the Blink-awareness question.

+    const gfx::Vector2d offset,
+    Layer* layer) {
+  DCHECK(layer);
+  if (layer != outer_viewport_scroll_layer_.get())
+    return offset;
+
+  gfx::Vector2d inner_viewport_offset =
+      inner_viewport_scroll_layer_->scroll_offset();
+  gfx::Vector2d outer_viewport_offset =
+      outer_viewport_scroll_layer_->scroll_offset();
+
+  if (offset == inner_viewport_offset + outer_viewport_offset) {
+    // In this case, nothing should change, so we just return to the outer
+    // viewport the offset is already has.
+    return outer_viewport_offset;
   }
+
+  // In the spirit of document-scrolls-first, we always want any change to
+  // go to the outer viewport first.
+  gfx::Vector2d max_outer_viewport_scroll_offset =
+      outer_viewport_scroll_layer_->MaxScrollOffset();
+#if ENABLE_DCHECK
+// The DCHECK below is triggering during zoom-out.
+/*
+  gfx::Vector2d maxInnerViewportScrollOffset =
+      inner_viewport_scroll_layer_->MaxScrollOffset();
+
+  gfx::Vector2d totalMaxScrollOffset =
+      max_outer_viewport_scroll_offset + maxInnerViewportScrollOffset;
+  DCHECK(totalMaxScrollOffset.x() >= offset.x() &&
+         totalMaxScrollOffset.y() >= offset.y());
+*/
+#endif
+
+  outer_viewport_offset = offset - inner_viewport_offset;
+  outer_viewport_offset.SetToMin(max_outer_viewport_scroll_offset);
+  outer_viewport_offset.SetToMax(gfx::Vector2d());
+
+  inner_viewport_offset = offset - outer_viewport_offset;
+  inner_viewport_scroll_layer_->SetScrollOffset(inner_viewport_offset);
+
+  return outer_viewport_offset;
 }
 
 void LayerTreeHost::StartRateLimiter() {