Use total page scale in the LayerScrollOffsetDelegate.

In order for the LayerScrollOffsetDelegate to see the most up to
date pageScale we need to use the total page scale rather than
just the value that was last synced with the main thread.

BUG=None
TEST=cc_unittests
R=aelias@chromium.org, joth@chromium.org, yfriedman@chromium.org

Review URL: https://codereview.chromium.org/26274003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@227763 0039d316-1c4b-4281-b951-d872f2087c98

13 files changed, 73 insertions, 178 deletions

diff --git a/android_webview/java/src/org/chromium/android_webview/AwContents.java b/android_webview/java/src/org/chromium/android_webview/AwContents.java
index acf18ad..aceb024 100644
--- a/android_webview/java/src/org/chromium/android_webview/AwContents.java
+++ b/android_webview/java/src/org/chromium/android_webview/AwContents.java
@@ -77,6 +77,10 @@ public class AwContents {
 
     private static final String WEB_ARCHIVE_EXTENSION = ".mht";
 
+    // Used to avoid enabling zooming in / out if resulting zooming will
+    // produce little visible difference.
+    private static final float ZOOM_CONTROLS_EPSILON = 0.007f;
+
     /**
      * WebKit hit test related data strcutre. These are used to implement
      * getHitTestResult, requestFocusNodeHref, requestImageRef methods in WebView.
@@ -532,7 +536,6 @@ public class AwContents {
         mDefaultVideoPosterRequestHandler = new DefaultVideoPosterRequestHandler(mContentsClient);
         mSettings.setDefaultVideoPosterURL(
                 mDefaultVideoPosterRequestHandler.getDefaultVideoPosterURL());
-        mContentsClient.setDIPScale(mDIPScale);
         mScrollOffsetManager = new AwScrollOffsetManager(new AwScrollOffsetManagerDelegate(),
                 new OverScroller(mContainerView.getContext()));
 
@@ -1385,29 +1388,47 @@ public class AwContents {
     /**
      * @see android.webkit.WebView#canZoomIn()
      */
+    // This method uses the term 'zoom' for legacy reasons, but relates
+    // to what chrome calls the 'page scale factor'.
     public boolean canZoomIn() {
-        return mContentViewCore.canZoomIn();
+        final float zoomInExtent = mContentViewCore.getRenderCoordinates().getMaxPageScaleFactor()
+                - mPageScaleFactor;
+        return zoomInExtent > ZOOM_CONTROLS_EPSILON;
     }
 
     /**
      * @see android.webkit.WebView#canZoomOut()
      */
+    // This method uses the term 'zoom' for legacy reasons, but relates
+    // to what chrome calls the 'page scale factor'.
     public boolean canZoomOut() {
-        return mContentViewCore.canZoomOut();
+        final float zoomOutExtent = mPageScaleFactor
+                - mContentViewCore.getRenderCoordinates().getMinPageScaleFactor();
+        return zoomOutExtent > ZOOM_CONTROLS_EPSILON;
     }
 
     /**
      * @see android.webkit.WebView#zoomIn()
      */
+    // This method uses the term 'zoom' for legacy reasons, but relates
+    // to what chrome calls the 'page scale factor'.
     public boolean zoomIn() {
-        return mContentViewCore.zoomIn();
+        if (!canZoomIn()) {
+            return false;
+        }
+        return mContentViewCore.pinchByDelta(1.25f);
     }
 
     /**
      * @see android.webkit.WebView#zoomOut()
      */
+    // This method uses the term 'zoom' for legacy reasons, but relates
+    // to what chrome calls the 'page scale factor'.
     public boolean zoomOut() {
-        return mContentViewCore.zoomOut();
+        if (!canZoomOut()) {
+            return false;
+        }
+        return mContentViewCore.pinchByDelta(0.8f);
     }
 
     /**
@@ -1867,7 +1888,12 @@ public class AwContents {
 
     @CalledByNative
     private void setPageScaleFactor(float pageScaleFactor) {
+        if (mPageScaleFactor == pageScaleFactor)
+            return;
+        float oldPageScaleFactor = mPageScaleFactor;
         mPageScaleFactor = pageScaleFactor;
+        mContentsClient.getCallbackHelper().postOnScaleChangedScaled(
+                (float)(oldPageScaleFactor * mDIPScale), (float)(mPageScaleFactor * mDIPScale));
     }
 
     @CalledByNative
diff --git a/android_webview/java/src/org/chromium/android_webview/AwContentsClient.java b/android_webview/java/src/org/chromium/android_webview/AwContentsClient.java
index 3f2245c..1f7bb24 100644
--- a/android_webview/java/src/org/chromium/android_webview/AwContentsClient.java
+++ b/android_webview/java/src/org/chromium/android_webview/AwContentsClient.java
@@ -50,8 +50,6 @@ public abstract class AwContentsClient {
 
     private AwContentViewClient mContentViewClient = new AwContentViewClient();
 
-    private double mDIPScale;
-
     // Last background color reported from the renderer. Holds the sentinal value INVALID_COLOR
     // if not valid.
     private int mCachedRendererBackgroundColor = INVALID_COLOR;
@@ -104,12 +102,6 @@ public abstract class AwContentsClient {
         }
 
         @Override
-        public void onScaleChanged(float oldScale, float newScale) {
-            AwContentsClient.this.onScaleChangedScaled((float)(oldScale * mDIPScale),
-                    (float)(newScale * mDIPScale));
-        }
-
-        @Override
         public void onStartContentIntent(Context context, String contentUrl) {
             //  Callback when detecting a click on a content link.
             AwContentsClient.this.shouldOverrideUrlLoading(contentUrl);
@@ -174,10 +166,6 @@ public abstract class AwContentsClient {
         }
     }
 
-    final void setDIPScale(double dipScale) {
-        mDIPScale = dipScale;
-    }
-
     final AwContentsClientCallbackHelper getCallbackHelper() {
         return mCallbackHelper;
     }
diff --git a/android_webview/java/src/org/chromium/android_webview/AwContentsClientCallbackHelper.java b/android_webview/java/src/org/chromium/android_webview/AwContentsClientCallbackHelper.java
index c792a18..8b9fcb6 100644
--- a/android_webview/java/src/org/chromium/android_webview/AwContentsClientCallbackHelper.java
+++ b/android_webview/java/src/org/chromium/android_webview/AwContentsClientCallbackHelper.java
@@ -75,6 +75,7 @@ class AwContentsClientCallbackHelper {
     private final static int MSG_ON_RECEIVED_LOGIN_REQUEST = 4;
     private final static int MSG_ON_RECEIVED_ERROR = 5;
     private final static int MSG_ON_NEW_PICTURE = 6;
+    private final static int MSG_ON_SCALE_CHANGED_SCALED = 7;
 
     // Minimum period allowed between consecutive onNewPicture calls, to rate-limit the callbacks.
     private static final long ON_NEW_PICTURE_MIN_PERIOD_MILLIS = 500;
@@ -128,6 +129,12 @@ class AwContentsClientCallbackHelper {
                     mHasPendingOnNewPicture = false;
                     break;
                 }
+                case MSG_ON_SCALE_CHANGED_SCALED: {
+                    float oldScale = Float.intBitsToFloat(msg.arg1);
+                    float newScale = Float.intBitsToFloat(msg.arg2);
+                    mContentsClient.onScaleChangedScaled(oldScale, newScale);
+                    break;
+                }
                 default:
                     throw new IllegalStateException(
                             "AwContentsClientCallbackHelper: unhandled message " + msg.what);
@@ -172,4 +179,13 @@ class AwContentsClientCallbackHelper {
         mHandler.sendMessageAtTime(mHandler.obtainMessage(MSG_ON_NEW_PICTURE, pictureProvider),
                 pictureTime);
     }
+
+    public void postOnScaleChangedScaled(float oldScale, float newScale) {
+        // The float->int->float conversion here is to avoid unnecessary allocations. The
+        // documentation states that intBitsToFloat(floatToIntBits(a)) == a for all values of a
+        // (except for NaNs which are collapsed to a single canonical NaN, but we don't care for
+        // that case).
+        mHandler.sendMessage(mHandler.obtainMessage(MSG_ON_SCALE_CHANGED_SCALED,
+                    Float.floatToIntBits(oldScale), Float.floatToIntBits(newScale)));
+    }
 }
diff --git a/cc/input/layer_scroll_offset_delegate.h b/cc/input/layer_scroll_offset_delegate.h
index 805bbfb..54a11c9 100644
--- a/cc/input/layer_scroll_offset_delegate.h
+++ b/cc/input/layer_scroll_offset_delegate.h
@@ -40,7 +40,7 @@ class LayerScrollOffsetDelegate {
 
   // This is called by the compositor to notify the delegate what is the current
   // page scale factor is.
-  virtual void SetPageScaleFactor(float page_scale_factor) = 0;
+  virtual void SetTotalPageScaleFactor(float page_scale_factor) = 0;
 
   // This is called by the compositor to notify the delegate what is the layer's
   // scrollable size is.
diff --git a/cc/layers/layer_impl_unittest.cc b/cc/layers/layer_impl_unittest.cc
index 3a0b1ce..83cd346 100644
--- a/cc/layers/layer_impl_unittest.cc
+++ b/cc/layers/layer_impl_unittest.cc
@@ -382,7 +382,7 @@ class ScrollDelegateIgnore : public LayerScrollOffsetDelegate {
     fixed_offset_ = fixed_offset;
   }
 
-  virtual void SetPageScaleFactor(float page_scale_factor) OVERRIDE {}
+  virtual void SetTotalPageScaleFactor(float page_scale_factor) OVERRIDE {}
   virtual void SetScrollableSize(gfx::SizeF scrollable_size) OVERRIDE {}
 
  private:
@@ -434,7 +434,7 @@ class ScrollDelegateAccept : public LayerScrollOffsetDelegate {
     return current_offset_;
   }
   virtual bool IsExternalFlingActive() const OVERRIDE { return false; }
-  virtual void SetPageScaleFactor(float page_scale_factor) OVERRIDE {}
+  virtual void SetTotalPageScaleFactor(float page_scale_factor) OVERRIDE {}
   virtual void SetScrollableSize(gfx::SizeF scrollable_size) OVERRIDE {}
 
  private:
diff --git a/cc/trees/layer_tree_host_impl_unittest.cc b/cc/trees/layer_tree_host_impl_unittest.cc
index 23a0378..720ae91 100644
--- a/cc/trees/layer_tree_host_impl_unittest.cc
+++ b/cc/trees/layer_tree_host_impl_unittest.cc
@@ -2451,7 +2451,7 @@ class TestScrollOffsetDelegate : public LayerScrollOffsetDelegate {
 
   virtual bool IsExternalFlingActive() const OVERRIDE { return false; }
 
-  virtual void SetPageScaleFactor(float page_scale_factor) OVERRIDE {
+  virtual void SetTotalPageScaleFactor(float page_scale_factor) OVERRIDE {
     page_scale_factor_ = page_scale_factor;
   }
 
@@ -2509,6 +2509,9 @@ TEST_F(LayerTreeHostImplTest, RootLayerScrollOffsetDelegation) {
   // Updating page scale immediately updates the delegate.
   host_impl_->active_tree()->SetPageScaleFactorAndLimits(2.f, 0.5f, 4.f);
   EXPECT_EQ(2.f, scroll_delegate.page_scale_factor());
+  host_impl_->active_tree()->SetPageScaleDelta(1.5f);
+  EXPECT_EQ(3.f, scroll_delegate.page_scale_factor());
+  host_impl_->active_tree()->SetPageScaleDelta(1.f);
   host_impl_->active_tree()->SetPageScaleFactorAndLimits(1.f, 0.5f, 4.f);
   EXPECT_EQ(1.f, scroll_delegate.page_scale_factor());
 
diff --git a/cc/trees/layer_tree_impl.cc b/cc/trees/layer_tree_impl.cc
index 1b527cc..aa587b4 100644
--- a/cc/trees/layer_tree_impl.cc
+++ b/cc/trees/layer_tree_impl.cc
@@ -190,8 +190,10 @@ void LayerTreeImpl::SetPageScaleFactorAndLimits(float page_scale_factor,
   max_page_scale_factor_ = max_page_scale_factor;
   page_scale_factor_ = page_scale_factor;
 
-  if (root_layer_scroll_offset_delegate_)
-    root_layer_scroll_offset_delegate_->SetPageScaleFactor(page_scale_factor_);
+  if (root_layer_scroll_offset_delegate_) {
+    root_layer_scroll_offset_delegate_->SetTotalPageScaleFactor(
+        total_page_scale_factor());
+  }
 }
 
 void LayerTreeImpl::SetPageScaleDelta(float delta) {
@@ -218,6 +220,11 @@ void LayerTreeImpl::SetPageScaleDelta(float delta) {
 
   UpdateMaxScrollOffset();
   set_needs_update_draw_properties();
+
+  if (root_layer_scroll_offset_delegate_) {
+    root_layer_scroll_offset_delegate_->SetTotalPageScaleFactor(
+        total_page_scale_factor());
+  }
 }
 
 gfx::SizeF LayerTreeImpl::ScrollableViewportSize() const {
@@ -618,7 +625,8 @@ void LayerTreeImpl::SetRootLayerScrollOffsetDelegate(
 
   if (root_layer_scroll_offset_delegate_) {
     root_layer_scroll_offset_delegate_->SetScrollableSize(ScrollableSize());
-    root_layer_scroll_offset_delegate_->SetPageScaleFactor(page_scale_factor_);
+    root_layer_scroll_offset_delegate_->SetTotalPageScaleFactor(
+        total_page_scale_factor());
   }
 }
 
diff --git a/content/browser/android/in_process/synchronous_compositor_impl.cc b/content/browser/android/in_process/synchronous_compositor_impl.cc
index 35a01cf..91cbdac 100644
--- a/content/browser/android/in_process/synchronous_compositor_impl.cc
+++ b/content/browser/android/in_process/synchronous_compositor_impl.cc
@@ -443,7 +443,8 @@ bool SynchronousCompositorImpl::IsExternalFlingActive() const {
   return false;
 }
 
-void SynchronousCompositorImpl::SetPageScaleFactor(float page_scale_factor) {
+void SynchronousCompositorImpl::SetTotalPageScaleFactor(
+    float page_scale_factor) {
   DCHECK(CalledOnValidThread());
   if (compositor_client_)
     compositor_client_->SetRootLayerPageScaleFactor(page_scale_factor);
diff --git a/content/browser/android/in_process/synchronous_compositor_impl.h b/content/browser/android/in_process/synchronous_compositor_impl.h
index 32b6ba9..4d61c37 100644
--- a/content/browser/android/in_process/synchronous_compositor_impl.h
+++ b/content/browser/android/in_process/synchronous_compositor_impl.h
@@ -77,7 +77,7 @@ class SynchronousCompositorImpl
   virtual void SetTotalScrollOffset(gfx::Vector2dF new_value) OVERRIDE;
   virtual gfx::Vector2dF GetTotalScrollOffset() OVERRIDE;
   virtual bool IsExternalFlingActive() const OVERRIDE;
-  virtual void SetPageScaleFactor(float page_scale_factor) OVERRIDE;
+  virtual void SetTotalPageScaleFactor(float page_scale_factor) OVERRIDE;
   virtual void SetScrollableSize(gfx::SizeF scrollable_size) OVERRIDE;
 
   void SetInputHandler(cc::InputHandler* input_handler);
diff --git a/content/public/android/java/src/org/chromium/content/browser/ContentView.java b/content/public/android/java/src/org/chromium/content/browser/ContentView.java
index 5ec50c4..60c314c 100644
--- a/content/public/android/java/src/org/chromium/content/browser/ContentView.java
+++ b/content/public/android/java/src/org/chromium/content/browser/ContentView.java
@@ -650,64 +650,7 @@ public class ContentView extends FrameLayout
     }
 
     /**
-     * Checks whether the WebView can be zoomed in.
-     *
-     * @return True if the WebView can be zoomed in.
-     */
-    // This method uses the term 'zoom' for legacy reasons, but relates
-    // to what chrome calls the 'page scale factor'.
-    public boolean canZoomIn() {
-        return mContentViewCore.canZoomIn();
-    }
-
-    /**
-     * Checks whether the WebView can be zoomed out.
-     *
-     * @return True if the WebView can be zoomed out.
-     */
-    // This method uses the term 'zoom' for legacy reasons, but relates
-    // to what chrome calls the 'page scale factor'.
-    public boolean canZoomOut() {
-        return mContentViewCore.canZoomOut();
-    }
-
-    /**
-     * Zooms in the WebView by 25% (or less if that would result in zooming in
-     * more than possible).
-     *
-     * @return True if there was a zoom change, false otherwise.
-     */
-    // This method uses the term 'zoom' for legacy reasons, but relates
-    // to what chrome calls the 'page scale factor'.
-    public boolean zoomIn() {
-        return mContentViewCore.zoomIn();
-    }
-
-    /**
-     * Zooms out the WebView by 20% (or less if that would result in zooming out
-     * more than possible).
-     *
-     * @return True if there was a zoom change, false otherwise.
-     */
-    // This method uses the term 'zoom' for legacy reasons, but relates
-    // to what chrome calls the 'page scale factor'.
-    public boolean zoomOut() {
-        return mContentViewCore.zoomOut();
-    }
-
-    /**
-     * Resets the zoom factor of the WebView.
-     *
-     * @return True if there was a zoom change, false otherwise.
-     */
-    // This method uses the term 'zoom' for legacy reasons, but relates
-    // to what chrome calls the 'page scale factor'.
-    public boolean zoomReset() {
-        return mContentViewCore.zoomReset();
-    }
-
-    /**
-     * Return the current scale of the WebView
+     * Return the current scale.
      * @return The current scale.
      */
     public float getScale() {
diff --git a/content/public/android/java/src/org/chromium/content/browser/ContentViewClient.java b/content/public/android/java/src/org/chromium/content/browser/ContentViewClient.java
index 755feb7..06e051b 100644
--- a/content/public/android/java/src/org/chromium/content/browser/ContentViewClient.java
+++ b/content/public/android/java/src/org/chromium/content/browser/ContentViewClient.java
@@ -43,13 +43,6 @@ public class ContentViewClient {
     }
 
     /**
-      * Lets client listen on the scaling changes on delayed, throttled
-      * and best-effort basis. Used for WebView.onScaleChanged.
-      */
-    public void onScaleChanged(float oldScale, float newScale) {
-    }
-
-    /**
      * Notifies the client that the position of the top controls has changed.
      * @param topControlsOffsetYPix The Y offset of the top controls in physical pixels.
      * @param contentOffsetYPix The Y offset of the content in physical pixels.
diff --git a/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java b/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
index c4b496a..2f2c27d 100644
--- a/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
+++ b/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
@@ -100,10 +100,6 @@ import java.util.Map;
 
     private static final String TAG = "ContentViewCore";
 
-    // Used to avoid enabling zooming in / out if resulting zooming will
-    // produce little visible difference.
-    private static final float ZOOM_CONTROLS_EPSILON = 0.007f;
-
     // Used to represent gestures for long press and long tap.
     private static final int IS_LONG_PRESS = 1;
     private static final int IS_LONG_TAP = 2;
@@ -2359,14 +2355,6 @@ import java.util.Map;
 
         if (needHidePopupZoomer) mPopupZoomer.hide(true);
 
-        if (pageScaleChanged) {
-            // This function should be called back from native as soon
-            // as the scroll is applied to the backbuffer.  We should only
-            // update mNativeScrollX/Y here for consistency.
-            getContentViewClient().onScaleChanged(
-                    mRenderCoordinates.getPageScaleFactor(), pageScaleFactor);
-        }
-
         mRenderCoordinates.updateFrameInfo(
                 scrollOffsetX, scrollOffsetY,
                 contentWidth, contentHeight,
@@ -2587,78 +2575,13 @@ import java.util.Map;
     }
 
     /**
-     * Checks whether the ContentViewCore can be zoomed in.
-     *
-     * @return True if the ContentViewCore can be zoomed in.
-     */
-    // This method uses the term 'zoom' for legacy reasons, but relates
-    // to what chrome calls the 'page scale factor'.
-    public boolean canZoomIn() {
-        final float zoomInExtent = mRenderCoordinates.getMaxPageScaleFactor()
-                - mRenderCoordinates.getPageScaleFactor();
-        return zoomInExtent > ZOOM_CONTROLS_EPSILON;
-    }
-
-    /**
-     * Checks whether the ContentViewCore can be zoomed out.
-     *
-     * @return True if the ContentViewCore can be zoomed out.
-     */
-    // This method uses the term 'zoom' for legacy reasons, but relates
-    // to what chrome calls the 'page scale factor'.
-    public boolean canZoomOut() {
-        final float zoomOutExtent = mRenderCoordinates.getPageScaleFactor()
-                - mRenderCoordinates.getMinPageScaleFactor();
-        return zoomOutExtent > ZOOM_CONTROLS_EPSILON;
-    }
-
-    /**
-     * Zooms in the ContentViewCore by 25% (or less if that would result in
-     * zooming in more than possible).
-     *
-     * @return True if there was a zoom change, false otherwise.
-     */
-    // This method uses the term 'zoom' for legacy reasons, but relates
-    // to what chrome calls the 'page scale factor'.
-    public boolean zoomIn() {
-        if (!canZoomIn()) {
-            return false;
-        }
-        return zoomByDelta(1.25f);
-    }
-
-    /**
-     * Zooms out the ContentViewCore by 20% (or less if that would result in
-     * zooming out more than possible).
+     * Simulate a pinch zoom gesture.
      *
-     * @return True if there was a zoom change, false otherwise.
-     */
-    // This method uses the term 'zoom' for legacy reasons, but relates
-    // to what chrome calls the 'page scale factor'.
-    public boolean zoomOut() {
-        if (!canZoomOut()) {
-            return false;
-        }
-        return zoomByDelta(0.8f);
-    }
-
-    /**
-     * Resets the zoom factor of the ContentViewCore.
+     * @param delta the factor by which the current page scale should be multiplied by.
      *
-     * @return True if there was a zoom change, false otherwise.
+     * @return whether the gesture was sent.
      */
-    // This method uses the term 'zoom' for legacy reasons, but relates
-    // to what chrome calls the 'page scale factor'.
-    public boolean zoomReset() {
-        // The page scale factor is initialized to mNativeMinimumScale when
-        // the page finishes loading. Thus sets it back to mNativeMinimumScale.
-        if (!canZoomOut()) return false;
-        return zoomByDelta(
-                mRenderCoordinates.getMinPageScaleFactor()
-                        / mRenderCoordinates.getPageScaleFactor());
-    }
-
-    private boolean zoomByDelta(float delta) {
+    public boolean pinchByDelta(float delta) {
         if (mNativeContentViewCore == 0) {
             return false;
         }
diff --git a/content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TestContentViewClientWrapper.java b/content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TestContentViewClientWrapper.java
index 5c94650..2cc5e07 100644
--- a/content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TestContentViewClientWrapper.java
+++ b/content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TestContentViewClientWrapper.java
@@ -35,12 +35,6 @@ public class TestContentViewClientWrapper extends TestContentViewClient {
     }
 
     @Override
-    public void onScaleChanged(float oldScale, float newScale) {
-        super.onScaleChanged(oldScale, newScale);
-        mWrappedClient.onScaleChanged(oldScale, newScale);
-    }
-
-    @Override
     public void onRendererCrash(boolean processWasOomProtected) {
         super.onRendererCrash(processWasOomProtected);
         mWrappedClient.onRendererCrash(processWasOomProtected);