Pinch/Zoom Infrastructure & Plumbing CL

cc/layers/layer.cc

Index: cc/layers/layer.cc
diff --git a/cc/layers/layer.cc b/cc/layers/layer.cc
index 7110dc70812c7987694b81fb8549345cc0c88d2d..5b76198133777e342cf4748361b1348decfa7313 100644
--- a/cc/layers/layer.cc
+++ b/cc/layers/layer.cc
@@ -16,6 +16,7 @@
 #include "cc/animation/layer_animation_controller.h"
 #include "cc/layers/layer_client.h"
 #include "cc/layers/layer_impl.h"
+#include "cc/layers/scrollbar_layer_interface.h"
 #include "cc/output/copy_output_request.h"
 #include "cc/output/copy_output_result.h"
 #include "cc/trees/layer_tree_host.h"
@@ -39,7 +40,7 @@ Layer::Layer()
       ignore_set_needs_commit_(false),
       parent_(NULL),
       layer_tree_host_(NULL),
-      scrollable_(false),
+      scroll_clip_layer_(NULL),
       should_scroll_on_main_thread_(false),
       have_wheel_event_handlers_(false),
       user_scrollable_horizontal_(true),
@@ -679,9 +680,16 @@ void Layer::RemoveClipChild(Layer* child) {
 
 void Layer::SetScrollOffset(gfx::Vector2d scroll_offset) {
   DCHECK(IsPropertyChangeAllowed());
+
+  if (layer_tree_host()) {
+    scroll_offset = layer_tree_host()->DistributeScrollOffsetToViewports(

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

Instead of doing this, I think we should make Blink be fully aware of the inner
and outer viewports and call SetScrollOffset on both layers.  I don't think it's
generally legitimate to split a combined scroll offset into some arbitrary mix.

One place this distribution might be appropriate is when restoring from a legacy
history item, but in that case that distribution logic should be in Blink, not
CC.

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

Hmm, but the design doc specifies (and I have always been told) that Blink *must
not* know about the two viewport layers (at least, not above the WebView level).

I agree that the splitting algorithm should not be arbitrary, but as long as it
is done in a way that doesn't introduce jitter (inner and/or outer viewport
suddenly changes in a non-smooth way), it should be fine. Given the bubble-up
nature of the scrolls, I think it's possible to do this.

If we suddenly change to letting Blink know about both viewports, that is a
*big* change in direction. This strategy has been in this patch for well over a
month now (probably much longer), so I would have expected this feedback a lot
sooner.

[enne (OOO), 2014/01/16 19:35:53]

I disagree.  I don't think Javascript is ever going to know about the two
viewports, so at some point these are going to need to be combined or
distributed depending on which direction the information is flowing.  Then the
question then is where to do this, and I think this patch has a reasonable
direction.

If you don't think it's legitimate, how do you handle communicating with
Javascript?

[aelias_OOO_until_Jul13, 2014/01/16 19:49:26]

For the purpose of this comment I am including WebViewImpl into the concept of
"Blink".  I don't think that the split should be plumbed down all the way to the
depths of Blink, but WebViewImpl and possibly a few top-level classes like
FrameView need to be aware of it.  Blink will use the outer offset for some
things (like fixed-pos) and the combined offset for other things (like Android
selection handles) so I don't see how it can work for it to only hear about the
combined one.

Sorry if it didn't end up sufficiently emphasized in the design doc, but I
recall emphasizing this (the upper layers wrapping Blink being aware of both
offsets) as a key condition for this split to be feasible.  For what it's worth,
I think the design change is somewhat superficial and your existing work is
mostly compatible with it.

+        scroll_offset, this);
+  }
+
   if (scroll_offset_ == scroll_offset)
     return;
   scroll_offset_ = scroll_offset;
+
   SetNeedsCommit();
 }
 
@@ -700,19 +708,43 @@ void Layer::SetScrollOffsetFromImplSide(gfx::Vector2d scroll_offset) {
   // "this" may have been destroyed during the process.
 }
 
-void Layer::SetMaxScrollOffset(gfx::Vector2d max_scroll_offset) {
-  DCHECK(IsPropertyChangeAllowed());
-  if (max_scroll_offset_ == max_scroll_offset)
-    return;
-  max_scroll_offset_ = max_scroll_offset;
-  SetNeedsCommit();
+// TODO(wjmaclean) We should template this and put it into LayerTreeHostCommon

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

I don't believe main-thread Layers need to know about max scroll offset at all,
so maybe we can just delete this instead?

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

Main thread layers need to know in order to deal with the combining/splitting
issue.

+// so that both Layer and LayerImpl are using the same code. In order
+// to template it we should avoid calling layer_tree_host() by giving
+// Layer/LayerImpl local accessors for page_scale_layer() and
+// page_scale_factor().
+gfx::Vector2d Layer::MaxScrollOffset() const {
+  if (!scroll_clip_layer_)
+    return gfx::Vector2d();
+
+  gfx::Size scaled_scroll_bounds(bounds());
+  Layer const* current_layer = this;
+  Layer const* page_scale_layer = layer_tree_host()->page_scale_layer();
+  float scale_factor = 1.f;
+  do {
+    if (current_layer == page_scale_layer) {
+      scale_factor = layer_tree_host()->page_scale_factor();
+      scaled_scroll_bounds.SetSize(
+          scale_factor * scaled_scroll_bounds.width(),
+          scale_factor * scaled_scroll_bounds.height());
+    }
+    current_layer = current_layer->parent();
+  } while (current_layer && current_layer != scroll_clip_layer_);
+  DCHECK(current_layer == scroll_clip_layer_);
+
+  gfx::Vector2dF max_offset(
+      scaled_scroll_bounds.width() - scroll_clip_layer_->bounds().width(),
+      scaled_scroll_bounds.height() - scroll_clip_layer_->bounds().height());
+  // We need the final scroll offset to be in CSS coords.
+  max_offset.Scale(1 / scale_factor);
+  return gfx::Vector2d(max_offset.x(), max_offset.y());
 }
 
-void Layer::SetScrollable(bool scrollable) {
+void Layer::SetScrollClipLayer(Layer* clip_layer) {
   DCHECK(IsPropertyChangeAllowed());
-  if (scrollable_ == scrollable)
+  if (scroll_clip_layer_ == clip_layer)
     return;
-  scrollable_ = scrollable;
+  scroll_clip_layer_ = clip_layer;
   SetNeedsCommit();
 }
 
@@ -915,10 +947,10 @@ void Layer::PushPropertiesTo(LayerImpl* layer) {
     layer->SetTransform(transform_);
   DCHECK(!(TransformIsAnimating() && layer->TransformIsAnimatingOnImplOnly()));
 
-  layer->SetScrollable(scrollable_);
+  layer->SetScrollClipLayer(scroll_clip_layer_ ? scroll_clip_layer_->id()
+                                        : Layer::INVALID_ID);
   layer->set_user_scrollable_horizontal(user_scrollable_horizontal_);
   layer->set_user_scrollable_vertical(user_scrollable_vertical_);
-  layer->SetMaxScrollOffset(max_scroll_offset_);
 
   LayerImpl* scroll_parent = NULL;
   if (scroll_parent_)