[android_webview] Use a fraction to calculate scroll offset.

android_webview/browser/in_process_view_renderer.cc

Index: android_webview/browser/in_process_view_renderer.cc
diff --git a/android_webview/browser/in_process_view_renderer.cc b/android_webview/browser/in_process_view_renderer.cc
index c343fcf0def02eadbbf2662142d811ed0326908e..d659f46e2b52156c5f333537d7d6c1e2ee79879e 100644
--- a/android_webview/browser/in_process_view_renderer.cc
+++ b/android_webview/browser/in_process_view_renderer.cc
@@ -568,7 +568,7 @@ skia::RefPtr<SkPicture> InProcessViewRenderer::CapturePicture(int width,
 
   // 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);
@@ -715,24 +715,33 @@ void InProcessViewRenderer::SetPageScaleFactor(float 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_)));
-
-  // 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)
+  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_));
+}
+
+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();
@@ -743,26 +752,48 @@ void InProcessViewRenderer::DidUpdateContent() {
     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());
+  }
+
+  if (max_scroll_offset_dip_.y()) {
+    scroll_offset.set_y((scroll_offset_dip.y() * max_offset.y()) /
+                        max_scroll_offset_dip_.y());
+  }
 
-  gfx::Vector2d scroll_offset = gfx::ToRoundedVector2d(
-      gfx::ScaleVector2d(new_value_css, dip_scale_ * page_scale_factor_));
+  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(
@@ -868,7 +899,7 @@ std::string InProcessViewRenderer::ToString(AwDrawGLInfo* draw_info) const {
                       "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());