[android_webview] Use a fraction to calculate scroll offset.

android_webview/browser/in_process_view_renderer.cc

 // Copyright 2013 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 "android_webview/browser/in_process_view_renderer.h"
 
 #include <android/bitmap.h>
 
 #include "android_webview/browser/aw_gl_surface.h"
 #include "android_webview/browser/scoped_app_gl_state_restore.h"
@@ skipping 550 matching lines @@
   TRACE_EVENT0("android_webview", "InProcessViewRenderer::CapturePicture");
 
   // Return empty Picture objects for empty SkPictures.
   skia::RefPtr<SkPicture> picture = skia::AdoptRef(new SkPicture);
   if (width <= 0 || height <= 0) {
     return picture;
   }
 
   // Reset scroll back to the origin, will go back to the old
   // value when scroll_reset is out of scope.
-  base::AutoReset<gfx::Vector2dF> scroll_reset(&scroll_offset_css_,
+  base::AutoReset<gfx::Vector2dF> scroll_reset(&scroll_offset_dip_,
                                                gfx::Vector2d());
 
   SkCanvas* rec_canvas = picture->beginRecording(width, height, 0);
   if (compositor_)
     CompositeSW(rec_canvas);
   picture->endRecording();
   return picture;
 }
 
 void InProcessViewRenderer::EnableOnNewPicture(bool enabled) {
@@ skipping 126 matching lines @@
 void InProcessViewRenderer::SetDipScale(float dip_scale) {
   dip_scale_ = dip_scale;
   CHECK(dip_scale_ > 0);
 }
 
 void InProcessViewRenderer::SetPageScaleFactor(float page_scale_factor) {
   page_scale_factor_ = page_scale_factor;
   CHECK(page_scale_factor_ > 0);
 }
 
-void InProcessViewRenderer::ScrollTo(gfx::Vector2d new_value) {
+gfx::Vector2d InProcessViewRenderer::max_scroll_offset() const {
   DCHECK(dip_scale_ > 0);
-  // In general we don't guarantee that the scroll offset transforms are
-  // symmetrical. That is if scrolling from JS to offset1 results in a native
-  // offset2 then scrolling from UI to offset2 results in JS being scrolled to
-  // offset1 again.
-  // The reason we explicitly do rounding here is that it seems to yeld the
-  // most stabile transformation.
-  gfx::Vector2dF new_value_css = gfx::ToRoundedVector2d(
-      gfx::ScaleVector2d(new_value, 1.0f / (dip_scale_ * page_scale_factor_)));
+  return gfx::ToRoundedVector2d(gfx::ScaleVector2d(

[aelias_OOO_until_Jul13, 2013/09/24 22:03:19]

Is there a reason why you're using Rounded and not Ceil or Floor?  In general I
prefer the latter because they behave more predictably.

[mkosiba (inactive), 2013/09/24 23:18:53]

no, there was no particular reason. I assumed the rounding wouldn't matter with
this approach.

Now that you point this out it seems like I should have used Ceil. That way if
we have a max_scroll_offset of 0.1 we can still represent 0 and 0.1 as 0 and 1
physical pixels and maintain the invariant that if the cc:: root layer thinks
it's scrollable so does the Java side (not sure it's strictly neccesary for
anything but feels like the sane thing to do).

+      max_scroll_offset_dip_, dip_scale_ * page_scale_factor_));
+}
 
-  // It's possible that more than one set of unique physical coordinates maps
-  // to the same set of CSS coordinates which means we can't reliably early-out
-  // earlier in the call stack.
-  if (scroll_offset_css_ == new_value_css)
+void InProcessViewRenderer::ScrollTo(gfx::Vector2d scroll_offset) {
+  gfx::Vector2d max_offset = max_scroll_offset();
+  gfx::Vector2dF scroll_offset_dip;
+  if (max_offset.x()) {
+    scroll_offset_dip.set_x((scroll_offset.x() * max_scroll_offset_dip_.x()) /
+                            max_offset.x());
+  }
+  if (max_offset.y()) {
+    scroll_offset_dip.set_y((scroll_offset.y() * max_scroll_offset_dip_.y()) /
+                            max_offset.y());

[mkosiba (inactive), 2013/09/24 18:43:05]

I could alternatively write these as

  gfx::Vector2dF scroll_offset_dip(
      (scroll_offset.x() * max_scroll_offset_dip_.x()) / max_offset.x(),
      (scroll_offset.y() * max_scroll_offset_dip_.y()) / max_offset.y());
  if (!max_offset.y()) scroll_offset_dip.set_y(0);
  if (!max_offset.x()) scroll_offset_dip.set_x(0);

[aelias_OOO_until_Jul13, 2013/09/24 22:03:19]

The current way is fine.  Could you add a comment explaining what you're doing
though?

+  }
+
+  DCHECK(0 <= scroll_offset_dip.x());

[aelias_OOO_until_Jul13, 2013/09/24 22:03:19]

Use DCHECK_LE instead.  That gives more informative output on failure.

[mkosiba (inactive), 2013/09/25 14:23:49]

Done.

+  DCHECK(0 <= scroll_offset_dip.y());
+  DCHECK(scroll_offset_dip.x() <= max_scroll_offset_dip_.x());
+  DCHECK(scroll_offset_dip.y() <= max_scroll_offset_dip_.y());
+
+  if (scroll_offset_dip_ == scroll_offset_dip)
     return;
 
-  scroll_offset_css_ = new_value_css;
+  scroll_offset_dip_ = scroll_offset_dip;
 
   if (compositor_)
     compositor_->DidChangeRootLayerScrollOffset();
 }
 
 void InProcessViewRenderer::DidUpdateContent() {
   if (on_new_picture_enable_)
     client_->OnNewPicture();
 }
 
+void InProcessViewRenderer::SetMaxRootLayerScrollOffset(
+    gfx::Vector2dF new_value_dip) {
+  DCHECK(dip_scale_ > 0);
+
+  max_scroll_offset_dip_ = new_value_dip;
+  DCHECK(max_scroll_offset_dip_.x() >= 0);
+  DCHECK(max_scroll_offset_dip_.y() >= 0);
+
+  client_->SetMaxContainerViewScrollOffset(max_scroll_offset());
+}
+
 void InProcessViewRenderer::SetTotalRootLayerScrollOffset(
-    gfx::Vector2dF new_value_css) {
+    gfx::Vector2dF scroll_offset_dip) {
   // TOOD(mkosiba): Add a DCHECK to say that this does _not_ get called during
   // DrawGl when http://crbug.com/249972 is fixed.
-  if (scroll_offset_css_ == new_value_css)
+  if (scroll_offset_dip_ == scroll_offset_dip)
     return;
 
-  scroll_offset_css_ = new_value_css;
+  scroll_offset_dip_ = scroll_offset_dip;
 
-  DCHECK(dip_scale_ > 0);
-  DCHECK(page_scale_factor_ > 0);
+  gfx::Vector2d max_offset = max_scroll_offset();
+  gfx::Vector2d scroll_offset;
+  if (max_scroll_offset_dip_.x()) {
+    scroll_offset.set_x((scroll_offset_dip.x() * max_offset.x()) /
+                        max_scroll_offset_dip_.x());
+  }
 
-  gfx::Vector2d scroll_offset = gfx::ToRoundedVector2d(
-      gfx::ScaleVector2d(new_value_css, dip_scale_ * page_scale_factor_));
+  if (max_scroll_offset_dip_.y()) {
+    scroll_offset.set_y((scroll_offset_dip.y() * max_offset.y()) /
+                        max_scroll_offset_dip_.y());
+  }
+
+  DCHECK(0 <= scroll_offset.x());
+  DCHECK(0 <= scroll_offset.y());
+  DCHECK(scroll_offset.x() <= max_offset.x());
+  DCHECK(scroll_offset.y() <= max_offset.y());
 
   client_->ScrollContainerViewTo(scroll_offset);
 }
 
 gfx::Vector2dF InProcessViewRenderer::GetTotalRootLayerScrollOffset() {
-  return scroll_offset_css_;
+  return scroll_offset_dip_;
 }
 
 void InProcessViewRenderer::DidOverscroll(
     gfx::Vector2dF accumulated_overscroll,
     gfx::Vector2dF latest_overscroll_delta,
     gfx::Vector2dF current_fling_velocity) {
   DCHECK(current_fling_velocity.IsZero());
   const float physical_pixel_scale = dip_scale_ * page_scale_factor_;
   if (accumulated_overscroll == latest_overscroll_delta)
     overscroll_rounding_error_ = gfx::Vector2dF();
@@ skipping 85 matching lines @@
   base::StringAppendF(&str, "attached_to_window: %d ", attached_to_window_);
   base::StringAppendF(&str, "hardware_initialized: %d ", hardware_initialized_);
   base::StringAppendF(&str, "hardware_failed: %d ", hardware_failed_);
   base::StringAppendF(&str,
                       "global visible rect: %s ",
                       cached_global_visible_rect_.ToString().c_str());
   base::StringAppendF(&str,
                       "scroll_at_start_of_frame: %s ",
                       scroll_at_start_of_frame_.ToString().c_str());
   base::StringAppendF(
-      &str, "scroll_offset_css: %s ", scroll_offset_css_.ToString().c_str());
+      &str, "scroll_offset_dip: %s ", scroll_offset_dip_.ToString().c_str());
   base::StringAppendF(&str,
                       "overscroll_rounding_error_: %s ",
                       overscroll_rounding_error_.ToString().c_str());
   if (draw_info) {
     base::StringAppendF(&str,
                         "clip left top right bottom: [%d %d %d %d] ",
                         draw_info->clip_left,
                         draw_info->clip_top,
                         draw_info->clip_right,
                         draw_info->clip_bottom);
     base::StringAppendF(&str,
                         "surface width height: [%d %d] ",
                         draw_info->width,
                         draw_info->height);
     base::StringAppendF(&str, "is_layer: %d ", draw_info->is_layer);
   }
   return str;
 }
 
 }  // namespace android_webview