Bug 898877 - Prevent pages from getting stuck without the dynamic toolbar. r=Cwiiis
authorKartikaya Gupta <kgupta@mozilla.com>
Fri, 16 Aug 2013 08:42:23 -0400
changeset 142999 ae2f8afea26071d56c4311722e6ef2135005350f
parent 142998 18ea4b3fb24fa40735968f98b5a208a3e9ddeda1
child 143000 f9d39959fc55d35e54047eedf86eb74d2fea0c46
push idunknown
push userunknown
push dateunknown
reviewersCwiiis
bugs898877
milestone26.0a1
Bug 898877 - Prevent pages from getting stuck without the dynamic toolbar. r=Cwiiis The problematic scenario is when the page is exactly the height of the screen (with dynamic toolbar not visible). In this case, the scrollable() function in Axis.java returns false on the vertical axis, and so the JavaPanZoomController never does any scrolling. This in turns means that the scrollBy code in LayerMarginsAnimator never gets to run, so you can never drag the toolbar back into being visible. The patch ensures that scrollable() returns true when some or all of the margins are not visible, ensuring that in these scenarios the user can still scroll the toolbar back onto the screen. This patch also adds some comments/asserts to verify the new code is threadsafe.
mobile/android/base/gfx/Axis.java
mobile/android/base/gfx/GeckoLayerClient.java
mobile/android/base/gfx/JavaPanZoomController.java
mobile/android/base/gfx/LayerMarginsAnimator.java
mobile/android/base/gfx/PanZoomTarget.java
--- a/mobile/android/base/gfx/Axis.java
+++ b/mobile/android/base/gfx/Axis.java
@@ -140,16 +140,17 @@ abstract class Axis {
     private float mDisplacement;
 
     private FlingStates mFlingState = FlingStates.STOPPED; /* The fling state we're in on this axis. */
 
     protected abstract float getOrigin();
     protected abstract float getViewportLength();
     protected abstract float getPageStart();
     protected abstract float getPageLength();
+    protected abstract boolean marginsHidden();
 
     Axis(SubdocumentScrollHelper subscroller) {
         mSubscroller = subscroller;
         mOverscrollMode = View.OVER_SCROLL_IF_CONTENT_SCROLLS;
         mRecentVelocities = new float[FLING_VELOCITY_POINTS];
     }
 
     public void setOverScrollMode(int overscrollMode) {
@@ -248,16 +249,23 @@ abstract class Axis {
             return !mScrollingDisabled;
         }
 
         // if we are axis locked, return false
         if (mScrollingDisabled) {
             return false;
         }
 
+        // if there are margins on this axis but they are currently hidden,
+        // we must be able to scroll in order to make them visible, so allow
+        // scrolling in that case
+        if (marginsHidden()) {
+            return true;
+        }
+
         // there is scrollable space, and we're not disabled, or the document fits the viewport
         // but we always allow overscroll anyway
         return getViewportLength() <= getPageLength() - MIN_SCROLLABLE_DISTANCE ||
                getOverScrollMode() == View.OVER_SCROLL_ALWAYS;
     }
 
     /*
      * Returns the resistance, as a multiplier, that should be taken into account when
--- a/mobile/android/base/gfx/GeckoLayerClient.java
+++ b/mobile/android/base/gfx/GeckoLayerClient.java
@@ -773,16 +773,22 @@ public class GeckoLayerClient implements
     /** Implementation of PanZoomTarget */
     @Override
     public boolean isFullScreen() {
         return mView.isFullScreen();
     }
 
     /** Implementation of PanZoomTarget */
     @Override
+    public RectF getMaxMargins() {
+        return mMarginsAnimator.getMaxMargins();
+    }
+
+    /** Implementation of PanZoomTarget */
+    @Override
     public void setAnimationTarget(ImmutableViewportMetrics metrics) {
         if (mGeckoIsReady) {
             // We know what the final viewport of the animation is going to be, so
             // immediately request a draw of that area by setting the display port
             // accordingly. This way we should have the content pre-rendered by the
             // time the animation is done.
             DisplayPortMetrics displayPort = DisplayPortCalculator.calculate(metrics, null);
             adjustViewport(displayPort);
--- a/mobile/android/base/gfx/JavaPanZoomController.java
+++ b/mobile/android/base/gfx/JavaPanZoomController.java
@@ -1051,28 +1051,40 @@ class JavaPanZoomController
         @Override
         public float getOrigin() { return getMetrics().viewportRectLeft; }
         @Override
         protected float getViewportLength() { return getMetrics().getWidth(); }
         @Override
         protected float getPageStart() { return getMetrics().pageRectLeft; }
         @Override
         protected float getPageLength() { return getMetrics().getPageWidthWithMargins(); }
+        @Override
+        protected boolean marginsHidden() {
+            ImmutableViewportMetrics metrics = getMetrics();
+            RectF maxMargins = mTarget.getMaxMargins();
+            return (metrics.marginLeft < maxMargins.left || metrics.marginRight < maxMargins.right);
+        }
     }
 
     private class AxisY extends Axis {
         AxisY(SubdocumentScrollHelper subscroller) { super(subscroller); }
         @Override
         public float getOrigin() { return getMetrics().viewportRectTop; }
         @Override
         protected float getViewportLength() { return getMetrics().getHeight(); }
         @Override
         protected float getPageStart() { return getMetrics().pageRectTop; }
         @Override
         protected float getPageLength() { return getMetrics().getPageHeightWithMargins(); }
+        @Override
+        protected boolean marginsHidden() {
+            ImmutableViewportMetrics metrics = getMetrics();
+            RectF maxMargins = mTarget.getMaxMargins();
+            return (metrics.marginTop < maxMargins.top || metrics.marginBottom < maxMargins.bottom);
+        }
     }
 
     /*
      * Zooming
      */
     @Override
     public boolean onScaleBegin(SimpleScaleGestureDetector detector) {
         if (mState == PanZoomState.ANIMATED_ZOOM)
--- a/mobile/android/base/gfx/LayerMarginsAnimator.java
+++ b/mobile/android/base/gfx/LayerMarginsAnimator.java
@@ -5,16 +5,17 @@
 
 package org.mozilla.gecko.gfx;
 
 import org.mozilla.gecko.GeckoAppShell;
 import org.mozilla.gecko.GeckoEvent;
 import org.mozilla.gecko.PrefsHelper;
 import org.mozilla.gecko.TouchEventInterceptor;
 import org.mozilla.gecko.util.FloatUtils;
+import org.mozilla.gecko.util.ThreadUtils;
 
 import android.graphics.PointF;
 import android.graphics.RectF;
 import android.os.SystemClock;
 import android.util.Log;
 import android.view.animation.DecelerateInterpolator;
 import android.view.MotionEvent;
 import android.view.View;
@@ -28,17 +29,19 @@ public class LayerMarginsAnimator implem
     private static final long MARGIN_ANIMATION_DURATION = 250;
     private static final String PREF_SHOW_MARGINS_THRESHOLD = "browser.ui.show-margins-threshold";
 
     /* This is the proportion of the viewport rect, minus maximum margins,
      * that needs to be travelled before margins will be exposed.
      */
     private float SHOW_MARGINS_THRESHOLD = 0.20f;
 
-    /* This rect stores the maximum value margins can grow to when scrolling */
+    /* This rect stores the maximum value margins can grow to when scrolling. When writing
+     * to this member variable, or when reading from this member variable on a non-UI thread,
+     * you must synchronize on the LayerMarginsAnimator instance. */
     private final RectF mMaxMargins;
     /* If this boolean is true, scroll changes will not affect margins */
     private boolean mMarginsPinned;
     /* The timer that handles showing/hiding margins */
     private Timer mAnimationTimer;
     /* This interpolator is used for the above mentioned animation */
     private final DecelerateInterpolator mInterpolator;
     /* The GeckoLayerClient whose margins will be animated */
@@ -81,25 +84,31 @@ public class LayerMarginsAnimator implem
             mPrefObserverId = null;
         }
     }
 
     /**
      * Sets the maximum values for margins to grow to, in pixels.
      */
     public synchronized void setMaxMargins(float left, float top, float right, float bottom) {
+        ThreadUtils.assertOnUiThread();
+
         mMaxMargins.set(left, top, right, bottom);
 
         // Update the Gecko-side global for fixed viewport margins.
         GeckoAppShell.sendEventToGecko(
             GeckoEvent.createBroadcastEvent("Viewport:FixedMarginsChanged",
                 "{ \"top\" : " + top + ", \"right\" : " + right
                 + ", \"bottom\" : " + bottom + ", \"left\" : " + left + " }"));
     }
 
+    RectF getMaxMargins() {
+        return mMaxMargins;
+    }
+
     private void animateMargins(final float left, final float top, final float right, final float bottom, boolean immediately) {
         if (mAnimationTimer != null) {
             mAnimationTimer.cancel();
             mAnimationTimer = null;
         }
 
         if (immediately) {
             ImmutableViewportMetrics newMetrics = mTarget.getViewportMetrics().setMargins(left, top, right, bottom);
--- a/mobile/android/base/gfx/PanZoomTarget.java
+++ b/mobile/android/base/gfx/PanZoomTarget.java
@@ -3,21 +3,23 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko.gfx;
 
 import org.mozilla.gecko.ZoomConstraints;
 
 import android.graphics.PointF;
+import android.graphics.RectF;
 
 public interface PanZoomTarget {
     public ImmutableViewportMetrics getViewportMetrics();
     public ZoomConstraints getZoomConstraints();
     public boolean isFullScreen();
+    public RectF getMaxMargins();
 
     public void setAnimationTarget(ImmutableViewportMetrics viewport);
     public void setViewportMetrics(ImmutableViewportMetrics viewport);
     public void scrollBy(float dx, float dy);
     public void onSubdocumentScrollBy(float dx, float dy);
     public void panZoomStopped();
     /** This triggers an (asynchronous) viewport update/redraw. */
     public void forceRedraw(DisplayPortMetrics displayPort);