Adding animations for view pager transitions

chrome/android/java/src/org/chromium/chrome/browser/contextmenu/TabularContextMenuViewPager.java

Index: chrome/android/java/src/org/chromium/chrome/browser/contextmenu/TabularContextMenuViewPager.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/contextmenu/TabularContextMenuViewPager.java b/chrome/android/java/src/org/chromium/chrome/browser/contextmenu/TabularContextMenuViewPager.java
index dd76948cbbe1e9b0f01ef264b72dd508555bea10..e499a96271deb481672b0d3a1c85c975cd42fcd4 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/contextmenu/TabularContextMenuViewPager.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/contextmenu/TabularContextMenuViewPager.java
@@ -4,18 +4,38 @@
 
 package org.chromium.chrome.browser.contextmenu;
 
+import android.animation.Animator;
+import android.animation.Animator.AnimatorListener;
+import android.animation.ValueAnimator;
+import android.animation.ValueAnimator.AnimatorUpdateListener;
 import android.content.Context;
+import android.graphics.Canvas;
+import android.graphics.Rect;
+import android.graphics.drawable.Drawable;
+import android.support.v4.view.ViewCompat;
 import android.support.v4.view.ViewPager;
+import android.support.v4.view.animation.LinearOutSlowInInterpolator;
 import android.util.AttributeSet;
 import android.view.View;
 
+import org.chromium.base.ApiCompatibilityUtils;
 import org.chromium.chrome.R;
+import org.chromium.chrome.browser.util.MathUtils;
 
 /**
  * When there is more than one view for the context menu to display, it wraps the display in a view
  * pager.
  */
 public class TabularContextMenuViewPager extends ViewPager {
+    private static final int ANIMATION_DURATION_MS = 250;
+
+    private ValueAnimator mAnimator;
+    private int mOldHeight;
+    private int mCanvasWidth;
+    private int mClipHeight;
+
+    private int mDifferenceInHeight = 0;
+    private int mPreviousChildIndex = 1;
     private final int mContextMenuMinimumPaddingPx =
             getResources().getDimensionPixelSize(R.dimen.context_menu_min_padding);
 
@@ -33,49 +53,130 @@ public class TabularContextMenuViewPager extends ViewPager {
      */
     @Override
     protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
-        int menuHeight = 0;
         int tabHeight = 0;
-
-        // getCurrentItem() does not take into account the tab layout unlike getChildCount().
-        int currentItemsIndex = getCurrentItem() + 1;
+        int menuHeight = 0;
 
         // The width of the context menu is defined so that it leaves space between itself and the
         // screen's edges. It is also bounded to a max size to prevent the menu from stretching
         // across a large display (e.g. a tablet screen).
-        int deviceWidthPx = getResources().getDisplayMetrics().widthPixels;
-        int contextMenuWidth = Math.min(deviceWidthPx - 2 * mContextMenuMinimumPaddingPx,
+        int appWindowWidthPx = getResources().getDisplayMetrics().widthPixels;
+        int contextMenuWidth = Math.min(appWindowWidthPx - 2 * mContextMenuMinimumPaddingPx,
                 getResources().getDimensionPixelSize(R.dimen.context_menu_max_width));
 
         widthMeasureSpec = MeasureSpec.makeMeasureSpec(contextMenuWidth, MeasureSpec.EXACTLY);
 
-        // The height of the context menu is calculated as the sum of:
-        // 1. The tab bar's height, which is only visible when the context menu requires it
-        //    (i.e. an ImageLink is clicked)
-        // 2. The height of the View being displayed for the current tab.
-        for (int i = 0; i < getChildCount(); i++) {
-            View child = getChildAt(i);
-
-            child.measure(
-                    widthMeasureSpec, MeasureSpec.makeMeasureSpec(0, MeasureSpec.UNSPECIFIED));
-            int measuredHeight = child.getMeasuredHeight();
-
-            // The ViewPager also considers the tab layout one of its children, and needs to be
-            // treated separately from getting the largest height.
-            if (child.getId() == R.id.tab_layout && child.getVisibility() != GONE) {
-                tabHeight = measuredHeight;
-            } else if (i == currentItemsIndex) {
-                menuHeight = child.getMeasuredHeight();
-                break;
+        // getCurrentItem() does not take into account the tab layout unlike getChildCount().
+        int currentChildIndex = getCurrentItem() + 1;
+
+        // Handles the case where this is called while the pager is scrolling between views.
+        if (getScrollX() != 0 && getScrollX() != mCanvasWidth) {
+            heightMeasureSpec = MeasureSpec.makeMeasureSpec(mOldHeight, MeasureSpec.EXACTLY);
+        } else {
+            // The height of the context menu is calculated as the sum of:
+            // 1. The tab bar's height, which is only visible when the context menu requires it
+            //    (i.e. an ImageLink is clicked)
+            // 2. The height of the View being displayed for the current tab.
+            for (int i = 0; i < getChildCount(); i++) {
+                View child = getChildAt(i);
+                child.measure(
+                        widthMeasureSpec, MeasureSpec.makeMeasureSpec(0, MeasureSpec.UNSPECIFIED));
+                int measuredHeight = child.getMeasuredHeight();
+
+                // The ViewPager also considers the tab layout one of its children, and needs to be
+                // treated separately from getting the largest height.
+                if (child.getId() == R.id.tab_layout && child.getVisibility() != GONE) {
+                    tabHeight = measuredHeight;
+                } else if (i == currentChildIndex) {
+                    menuHeight = child.getMeasuredHeight();
+                    break;
+                }
+            }
+            int fullHeight = menuHeight + tabHeight;
+            int appWindowHeightPx = getResources().getDisplayMetrics().heightPixels;
+            fullHeight = Math.min(fullHeight, appWindowHeightPx - 2 * mContextMenuMinimumPaddingPx);
+            mDifferenceInHeight = fullHeight - mOldHeight;
+
+            if (currentChildIndex == mPreviousChildIndex) {
+                // Handles the snapping and re-clipping of the view when its height changes
+                // (i.e. an image finished loading or the link became fully visible).
+                mClipHeight = fullHeight;
+                if (menuHeight != 0) mOldHeight = fullHeight;
+                heightMeasureSpec = MeasureSpec.makeMeasureSpec(fullHeight, MeasureSpec.EXACTLY);
+                // Clip logic is not called on orientation changes, so it must be set here.
+                setClipDimensionsForResize(contextMenuWidth);
+            } else {
+                // Handles the case where the view pager has completely scrolled to a different
+                // child.
+                if (mAnimator == null) initAnimator();
+                heightMeasureSpec = MeasureSpec.makeMeasureSpec(
+                        Math.max(mOldHeight, fullHeight), MeasureSpec.EXACTLY);
+                mAnimator.start();
             }
         }
+        super.onMeasure(widthMeasureSpec, heightMeasureSpec);
+        mPreviousChildIndex = currentChildIndex;
+    }
 
-        // Cap the height of the context menu so that it fits on the screen without touching the
-        // screen's edges.
-        int fullHeight = menuHeight + tabHeight;
-        int deviceHeightPx = getResources().getDisplayMetrics().heightPixels;
-        fullHeight = Math.min(fullHeight, deviceHeightPx - 2 * mContextMenuMinimumPaddingPx);
+    public void setClipDimensionsForResize(int width) {
+        // TODO(injae): Find a different way to mimic animations

[Theresa, 2017/07/17 21:28:25]

I think we should address this TODO now, since it may change the overall
approach for the animation (hopefully it doesn't, but it'd be good to land
something that works on all OS versions reasonably well). 

[Daniel Park, 2017/07/19 00:30:09]

Done.

+        ViewCompat.setClipBounds(this, new Rect(0, 0, width, mClipHeight));
+    }
 
-        heightMeasureSpec = MeasureSpec.makeMeasureSpec(fullHeight, MeasureSpec.EXACTLY);
-        super.onMeasure(widthMeasureSpec, heightMeasureSpec);
+    private void initAnimator() {
+        mAnimator = ValueAnimator.ofFloat(0f, 1f);

[Ted C, 2017/07/17 21:25:31]

I would do:

if (mAnimator != null && mAnimator.isRunning()) mAnimator.cancel();

Oh, we are reusing the animator, so maybe start on it doesn't cause it to jump. 
If we are reusing it, I would do the check here:

if (mAnimator != null) return;

Then it is harder for callers to misuse it.

[Daniel Park, 2017/07/19 00:30:09]

Done.

[Theresa, 2017/07/19 15:23:20]

If the animator is running and another layout change occurs, you may see some
odd experience. For example, if the height change is animating due to the tab
switching and the user rotates the device, what happens?

[Daniel Park, 2017/07/19 20:26:44]

The animation finishes and is laid out properly; nothing unusual happens.

+        mAnimator.setDuration(ANIMATION_DURATION_MS);
+        mAnimator.setInterpolator(new LinearOutSlowInInterpolator());
+        mAnimator.addUpdateListener(new AnimatorUpdateListener() {
+
+            @Override
+            public void onAnimationUpdate(ValueAnimator animation) {
+                float animatedValue = (float) animation.getAnimatedValue();
+                // If the animation duration is long enough (e.g. 5x, 10x), animatedValue will
+                // undesirably be set to 1f more than once.
+                if (mDifferenceInHeight < 0) {
+                    setTranslationY(animatedValue * -mDifferenceInHeight / 2);
+                } else {
+                    setTranslationY((1 - animatedValue) * mDifferenceInHeight / 2);
+                }
+                mClipHeight = mOldHeight + (int) (mDifferenceInHeight * animatedValue);
+                ViewCompat.setClipBounds(TabularContextMenuViewPager.this,
+                        new Rect(0, 0, mCanvasWidth, mClipHeight));
+                invalidate();
+                if (MathUtils.areFloatsEqual(animatedValue, 1f)) {
+                    return;

[Theresa, 2017/07/17 21:28:25]

We're already at the end of the method so this early return doesn't skip
anything and isn't necessary.

[Daniel Park, 2017/07/19 00:30:08]

the animated value will hit 1 twice if the animation duration is set to 5x or
10x which shouldn't happen outside of debugging purposes. this handles that
case.

[Theresa, 2017/07/19 15:23:20]

My point is that nothing happens in this method after this code block, so
returning early is a no-op. Am I'm missing something?

[Daniel Park, 2017/07/19 20:26:44]

Removed; the issue's not appearing anymore.

+                }
+            }
+        });
+        mAnimator.addListener(new AnimatorListener() {

[Ted C, 2017/07/17 21:25:31]

take a look at CancelAwareAnimatorListener or AnimatorListenerAdapter to avoid
overwriting all the unneeded methods

[Theresa, 2017/07/17 21:28:25]

You can use AnimatorListenerAdapter to avoid providing empty implementations for
onAnimationStart(), onAnimationRepeat() and onAnimationCancel().

[Daniel Park, 2017/07/19 00:30:08]

Done.

+
+            @Override
+            public void onAnimationStart(Animator animation) {}
+
+            @Override
+            public void onAnimationRepeat(Animator animation) {}
+
+            @Override
+            public void onAnimationEnd(Animator animation) {
+                mOldHeight = mClipHeight;
+                requestLayout();

[Ted C, 2017/07/17 21:25:31]

do we only need to request layout if the new height is smaller?  If we've grown
to the bigger height, the layout "should" be unneeded.

[Daniel Park, 2017/07/19 00:30:08]

Done.

+                setTranslationY(0);

[Theresa, 2017/07/17 21:28:25]

nit: if you can flip setTranslationY(0) and requestLayout(), I think that
ordering makes a bit more sense.

[Daniel Park, 2017/07/19 00:30:08]

Done.

+            }
+
+            @Override
+            public void onAnimationCancel(Animator animation) {}
+        });
+    }
+
+    @Override
+    public void onDraw(Canvas canvas) {
+        mCanvasWidth = canvas.getWidth();
+        int backgroundOffsetX = getScrollX();
+        Drawable drawable = ApiCompatibilityUtils.getDrawable(

[Ted C, 2017/07/17 21:25:31]

we might want to save this to a class variable as mutate will cause all the
state to be duplicated and we don't really need that to be done per frame (we
can mutate it once and just manipulate it forever).

[Daniel Park, 2017/07/19 00:30:09]

Done.

+                getResources(), R.drawable.white_with_rounded_corners);
+        drawable.mutate();
+        drawable.setBounds(
+                backgroundOffsetX, 0, canvas.getWidth() + backgroundOffsetX, mClipHeight);
+        drawable.draw(canvas);
+        super.onDraw(canvas);
     }
 }