Simplify and add tests for the progress bar

This change refactors much of the progress bar code in an effort to
simplify the logic that has grown out of control. The significant
changes are as follows:

- The smooth progress animator runs until progress is finished instead
  of ending when progress reaches the current complete percentage.
- 'finish(...)' is blocked until the progress animators are complete
  unless it is called with 'fadeOut' set to false (see todo in code).
- 'updateVisibleProgress' has been removed as its function had become
  unclear.
- More complete tests to exercise the different functionality.

BUG=

Change-Id: I95ff4c5918bccaac5bc1ef025254caea13a862c2
Reviewed-on: https://chromium-review.googlesource.com/769772
Commit-Queue: Matthew Jones <mdjones@chromium.org>
Reviewed-by: Yusuf Ozuysal <yusufo@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522161}

chrome/android/java/src/org/chromium/chrome/browser/widget/ClipDrawableProgressBar.java

@@ -15,6 +15,7 @@ import android.view.ViewGroup.LayoutParams;
 import android.widget.ImageView;
 
 import org.chromium.base.ApiCompatibilityUtils;
+import org.chromium.base.VisibleForTesting;
 import org.chromium.chrome.R;
 
 /**
@@ -32,6 +33,23 @@ public class ClipDrawableProgressBar extends ImageView {
         public int progressBarBackgroundColor;
     }
 
+    /**
+     * An observer for visible progress updates.
+     */
+    @VisibleForTesting
+    interface ProgressBarObserver {
+        /**
+         * A notification that the visible progress has been updated. This may not coincide with
+         * updates from the web page due to animations for the progress bar running.
+         */
+        void onVisibleProgressUpdated();
+
+        /**
+         * A notification that the visibility of the progress bar has changed.
+         */
+        void onVisibilityChanged();
+    }
+
     // ClipDrawable's max is a fixed constant 10000.
     // http://developer.android.com/reference/android/graphics/drawable/ClipDrawable.html
     private static final int CLIP_DRAWABLE_MAX = 10000;
@@ -41,6 +59,9 @@ public class ClipDrawableProgressBar extends ImageView {
     private float mProgress;
     private int mDesiredVisibility;
 
+    /** An observer of updates to the progress bar. */
+    private ProgressBarObserver mProgressBarObserver;
+
     /**
      * Create the progress bar with a custom height.
      * @param context An Android context.
@@ -64,6 +85,15 @@ public class ClipDrawableProgressBar extends ImageView {
         setLayoutParams(new LayoutParams(LayoutParams.MATCH_PARENT, height));
     }
 
+    /**
+     * @param observer An update observer for the progress bar.
+     */
+    @VisibleForTesting
+    void setProgressBarObserver(ProgressBarObserver observer) {
+        assert mProgressBarObserver == null;
+        mProgressBarObserver = observer;
+    }
+
     /**
      * Get the progress bar's current level of progress.
      *
@@ -84,6 +114,7 @@ public class ClipDrawableProgressBar extends ImageView {
 
         mProgress = progress;
         getDrawable().setLevel(Math.round(progress * CLIP_DRAWABLE_MAX));
+        if (mProgressBarObserver != null) mProgressBarObserver.onVisibleProgressUpdated();
     }
 
     /**
@@ -132,7 +163,10 @@ public class ClipDrawableProgressBar extends ImageView {
         int oldVisibility = getVisibility();
         int newVisibility = mDesiredVisibility;
         if (getAlpha() == 0 && mDesiredVisibility == VISIBLE) newVisibility = INVISIBLE;
-        if (oldVisibility != newVisibility) super.setVisibility(newVisibility);
+        if (oldVisibility != newVisibility) {
+            super.setVisibility(newVisibility);
+            if (mProgressBarObserver != null) mProgressBarObserver.onVisibilityChanged();
+        }
     }
 
     private int applyAlpha(int color, float alpha) {

chrome/android/java/src/org/chromium/chrome/browser/widget/ToolbarProgressBar.java

@@ -20,6 +20,7 @@ import android.widget.FrameLayout.LayoutParams;
 import android.widget.ProgressBar;
 
 import org.chromium.base.ApiCompatibilityUtils;
+import org.chromium.base.ThreadUtils;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ChromeFeatureList;
@@ -61,6 +62,7 @@ public class ToolbarProgressBar extends ClipDrawableProgressBar {
      * animation starts.
      */
     private static final long ANIMATION_START_THRESHOLD = 5000;
+    private static final long HIDE_DELAY_MS = 100;
 
     private static final float THEMED_BACKGROUND_WHITE_FRACTION = 0.2f;
     private static final float ANIMATION_WHITE_FRACTION = 0.4f;
@@ -69,8 +71,7 @@ public class ToolbarProgressBar extends ClipDrawableProgressBar {
     private static final float PROGRESS_THROTTLE_MAX_UPDATE_AMOUNT = 0.03f;
 
     private static final long PROGRESS_FRAME_TIME_CAP_MS = 50;
-    private long mAlphaAnimationDurationMs = 140;
-    private long mHidingDelayMs = 100;
+    private static final long ALPHA_ANIMATION_DURATION_MS = 140;
 
     /** Whether or not the progress bar has started processing updates. */
     private boolean mIsStarted;
@@ -99,9 +100,6 @@ public class ToolbarProgressBar extends ClipDrawableProgressBar {
     /** Whether or not to use the status bar color as the background of the toolbar. */
     private boolean mUseStatusBarColorAsBackground;
 
-    /** Whether the smooth animation should be started if it is stopped. */
-    private boolean mStartSmoothAnimation;
-
     /** The animator responsible for updating progress once it has been throttled. */
     private TimeAnimator mProgressThrottle;
 
@@ -110,29 +108,19 @@ public class ToolbarProgressBar extends ClipDrawableProgressBar {
 
     /**
      * The indeterminate animating view for the progress bar. This will be null for Android
-     * versions < J.
+     * versions < K.
      */
     private ToolbarProgressBarAnimatingView mAnimatingView;
 
     /** Whether or not the progress bar is attached to the window. */
     private boolean mIsAttachedToWindow;
 
-    private final Runnable mHideRunnable = new Runnable() {
-        @Override
-        public void run() {
-            animateAlphaTo(0.0f);
-            mStartSmoothAnimation = false;
-            if (mAnimatingView != null) mAnimatingView.cancelAnimation();
-        }
-    };
-
     private final Runnable mStartSmoothIndeterminate = new Runnable() {
         @Override
         public void run() {
             if (!mIsStarted) return;
-            mStartSmoothAnimation = true;
             mAnimationLogic.reset(getProgress());
-            mProgressAnimator.start();
+            mSmoothProgressAnimator.start();
 
             if (mAnimatingView != null) {
                 int width =
@@ -143,16 +131,23 @@ public class ToolbarProgressBar extends ClipDrawableProgressBar {
         }
     };
 
-    private final TimeAnimator mProgressAnimator = new TimeAnimator();
+    private final TimeAnimator mSmoothProgressAnimator = new TimeAnimator();
     {
-        mProgressAnimator.setTimeListener(new TimeListener() {
+        mSmoothProgressAnimator.setTimeListener(new TimeListener() {
             @Override
             public void onTimeUpdate(TimeAnimator animation, long totalTimeMs, long deltaTimeMs) {
+                // If we are at the target progress already, do nothing.
+                if (MathUtils.areFloatsEqual(getProgress(), mTargetProgress)) return;
+
                 // Cap progress bar animation frame time so that it doesn't jump too much even when
                 // the animation is janky.
                 float progress = mAnimationLogic.updateProgress(mTargetProgress,
                         Math.min(deltaTimeMs, PROGRESS_FRAME_TIME_CAP_MS) * 0.001f, getWidth());
                 progress = Math.max(progress, 0);
+
+                // TODO(mdjones): Find a sane way to have this call setProgressInternal so the
+                // finish logic can be recycled. Consider stopping the progress throttle if the
+                // smooth animation is running.
                 ToolbarProgressBar.super.setProgress(progress);
 
                 if (mAnimatingView != null) {
@@ -161,12 +156,8 @@ public class ToolbarProgressBar extends ClipDrawableProgressBar {
                     mAnimatingView.update(progress * width);
                 }
 
-                if (MathUtils.areFloatsEqual(getProgress(), mTargetProgress)) {
-                    if (!mIsStarted) postOnAnimationDelayed(mHideRunnable, mHidingDelayMs);
-                    mProgressAnimator.end();
-                    if (MathUtils.areFloatsEqual(getProgress(), 1.f)) finish(false);
-                    return;
-                }
+                // If progress is at 100%, start hiding the progress bar.
+                if (MathUtils.areFloatsEqual(getProgress(), 1.f)) finish(true);
             }
         });
     }
@@ -217,11 +208,9 @@ public class ToolbarProgressBar extends ClipDrawableProgressBar {
                     * mExpectedDuration);
 
             float updatedProgress = getProgress() + PROGRESS_THROTTLE_MAX_UPDATE_AMOUNT;
-            setProgressInternal(MathUtils.clamp(updatedProgress, 0f, mThrottledProgressTarget));
-
             if (updatedProgress >= mThrottledProgressTarget) animation.end();
 
-            if (updatedProgress >= 1f) finish(true);
+            setProgressInternal(MathUtils.clamp(updatedProgress, 0f, mThrottledProgressTarget));
         }
     }
 
@@ -325,16 +314,16 @@ public class ToolbarProgressBar extends ClipDrawableProgressBar {
      * Start showing progress bar animation.
      */
     public void start() {
+        ThreadUtils.assertOnUiThread();
+
         mIsStarted = true;
         mProgressStartCount++;
 
         removeCallbacks(mStartSmoothIndeterminate);
         postDelayed(mStartSmoothIndeterminate, ANIMATION_START_THRESHOLD);
-        mStartSmoothAnimation = false;
 
         super.setProgress(0.0f);
         mAnimationLogic.reset(0.0f);
-        removeCallbacks(mHideRunnable);
         animateAlphaTo(1.0f);
     }
 
@@ -346,71 +335,74 @@ public class ToolbarProgressBar extends ClipDrawableProgressBar {
     }
 
     /**
-     * Start hiding progress bar animation.
-     * @param delayed Whether a delayed fading out animation should be posted.
+     * Start hiding progress bar animation. Progress does not necessarily need to be at 100% to
+     * finish. If 'fadeOut' is set to true, progress will forced to 100% (if not already) and then
+     * fade out. If false, the progress will hide regardless of where it currently is.
+     * @param fadeOut Whether the progress bar should fade out. If false, the progress bar will
+     *                disappear immediately, regardless of animation.
+     *                TODO(mdjones): This param should be "force" but involves inverting all calls
+     *                to this method.
      */
-    public void finish(boolean delayed) {
-        if (mProgressThrottle != null && mProgressThrottle.isRunning()
-                || mAnimatingView != null && mAnimatingView.isRunning()) {
-            return;
+    public void finish(boolean fadeOut) {
+        ThreadUtils.assertOnUiThread();
+
+        if (!MathUtils.areFloatsEqual(getProgress(), 1.0f)) {
+            // If any of the animators are running while this method is called, set the internal
+            // progress and wait for the animation to end.
+            setProgress(1.0f);
+            if (areProgressAnimatorsRunning() && fadeOut) return;
         }
 
         mIsStarted = false;
+        mTargetProgress = 0;
 
-        if (delayed) {
-            updateVisibleProgress();
+        removeCallbacks(mStartSmoothIndeterminate);
+        if (mAnimatingView != null) mAnimatingView.cancelAnimation();
+        if (mProgressThrottle != null) mProgressThrottle.cancel();
+        mSmoothProgressAnimator.cancel();
+
+        if (fadeOut) {
+            postDelayed(() -> hideProgressBar(true), HIDE_DELAY_MS);
         } else {
-            removeCallbacks(mHideRunnable);
-            animate().cancel();
-            if (mAnimatingView != null) {
-                removeCallbacks(mStartSmoothIndeterminate);
-                mAnimatingView.cancelAnimation();
-            }
-            mTargetProgress = 0;
-            mStartSmoothAnimation = false;
-            setAlpha(0.0f);
+            hideProgressBar(false);
         }
     }
 
     /**
-     * Set alpha show&hide animation duration. This is for faster testing.
-     * @param alphaAnimationDurationMs Alpha animation duration in milliseconds.
+     * Hide the progress bar.
+     * @param animate Whether to animate the opacity.
      */
-    @VisibleForTesting
-    public void setAlphaAnimationDuration(long alphaAnimationDurationMs) {
-        mAlphaAnimationDurationMs = alphaAnimationDurationMs;
-    }
+    private void hideProgressBar(boolean animate) {
+        ThreadUtils.assertOnUiThread();
 
-    /**
-     * Set hiding delay duration. This is for faster testing.
-     * @param hidngDelayMs Hiding delay duration in milliseconds.
-     */
-    @VisibleForTesting
-    public void setHidingDelay(long hidngDelayMs) {
-        mHidingDelayMs = hidngDelayMs;
+        if (mIsStarted) return;
+        if (!animate) animate().cancel();
+
+        // Make invisible.
+        if (animate) {
+            animateAlphaTo(0.0f);
+        } else {
+            setAlpha(0.0f);
+        }
     }
 
     /**
-     * @return The number of times the progress bar has been triggered.
+     * @return Whether any animator that delays the showing of progress is running.
      */
-    @VisibleForTesting
-    public int getStartCountForTesting() {
-        return mProgressStartCount;
+    private boolean areProgressAnimatorsRunning() {
+        return (mProgressThrottle != null && mProgressThrottle.isRunning())
+                || mSmoothProgressAnimator.isRunning();
     }
 
     /**
-     * Reset the number of times the progress bar has been triggered.
+     * Animate the alpha of all of the parts of the progress bar.
+     * @param targetAlpha The alpha in range [0, 1] to animate to.
      */
-    @VisibleForTesting
-    public void resetStartCountForTesting() {
-        mProgressStartCount = 0;
-    }
-
     private void animateAlphaTo(float targetAlpha) {
         float alphaDiff = targetAlpha - getAlpha();
         if (alphaDiff == 0.0f) return;
 
-        long duration = (long) Math.abs(alphaDiff * mAlphaAnimationDurationMs);
+        long duration = (long) Math.abs(alphaDiff * ALPHA_ANIMATION_DURATION_MS);
 
         BakedBezierInterpolator interpolator = BakedBezierInterpolator.FADE_IN_CURVE;
         if (alphaDiff < 0) interpolator = BakedBezierInterpolator.FADE_OUT_CURVE;
@@ -426,22 +418,12 @@ public class ToolbarProgressBar extends ClipDrawableProgressBar {
         }
     }
 
-    private void updateVisibleProgress() {
-        if (mStartSmoothAnimation || (mAnimatingView != null && mAnimatingView.isRunning())) {
-            // The progress animator will stop if the animation reaches the target progress. If the
-            // animation was running for the current page load, keep running it.
-            mProgressAnimator.start();
-        } else {
-            super.setProgress(mTargetProgress);
-            if (!mIsStarted) postOnAnimationDelayed(mHideRunnable, mHidingDelayMs);
-        }
-        sendAccessibilityEvent(AccessibilityEvent.TYPE_VIEW_SELECTED);
-    }
-
     // ClipDrawableProgressBar implementation.
 
     @Override
     public void setProgress(float progress) {
+        ThreadUtils.assertOnUiThread();
+
         // TODO(mdjones): Maybe subclass this to be ThrottledToolbarProgressBar.
         if (mProgressThrottle == null && ChromeFeatureList.isInitialized()
                 && ChromeFeatureList.isEnabled(ChromeFeatureList.PROGRESS_BAR_THROTTLE)) {
@@ -469,22 +451,21 @@ public class ToolbarProgressBar extends ClipDrawableProgressBar {
      * @param progress The current progress.
      */
     private void setProgressInternal(float progress) {
-        if (!mIsStarted || mTargetProgress == progress) return;
+        if (!mIsStarted || MathUtils.areFloatsEqual(mTargetProgress, progress)) return;
+        mTargetProgress = progress;
 
         // If the progress bar was updated, reset the callback that triggers the
         // smooth-indeterminate animation.
         removeCallbacks(mStartSmoothIndeterminate);
 
-        if (mAnimatingView != null) {
-            if (progress == 1.0) {
-                mAnimatingView.cancelAnimation();
-            } else if (!mAnimatingView.isRunning()) {
-                postDelayed(mStartSmoothIndeterminate, ANIMATION_START_THRESHOLD);
-            }
+        if (!mSmoothProgressAnimator.isRunning()) {
+            postDelayed(mStartSmoothIndeterminate, ANIMATION_START_THRESHOLD);
+            super.setProgress(mTargetProgress);
         }
 
-        mTargetProgress = progress;
-        updateVisibleProgress();
+        sendAccessibilityEvent(AccessibilityEvent.TYPE_VIEW_SELECTED);
+
+        if (MathUtils.areFloatsEqual(progress, 1.0f) || progress > 1.0f) finish(true);
     }
 
     @Override
@@ -552,4 +533,36 @@ public class ToolbarProgressBar extends ClipDrawableProgressBar {
         event.setCurrentItemIndex((int) (mTargetProgress * 100));
         event.setItemCount(100);
     }
+
+    /**
+     * @return The number of times the progress bar has been triggered.
+     */
+    @VisibleForTesting
+    public int getStartCountForTesting() {
+        return mProgressStartCount;
+    }
+
+    /**
+     * Reset the number of times the progress bar has been triggered.
+     */
+    @VisibleForTesting
+    public void resetStartCountForTesting() {
+        mProgressStartCount = 0;
+    }
+
+    /**
+     * Start the indeterminate progress bar animation.
+     */
+    @VisibleForTesting
+    public void startIndeterminateAnimationForTesting() {
+        mStartSmoothIndeterminate.run();
+    }
+
+    /**
+     * @return The indeterminate animator.
+     */
+    @VisibleForTesting
+    public Animator getIndeterminateAnimatorForTesting() {
+        return mSmoothProgressAnimator;
+    }
 }

chrome/android/java/src/org/chromium/chrome/browser/widget/ToolbarProgressBarAnimatingView.java

@@ -107,6 +107,7 @@ public class ToolbarProgressBarAnimatingView extends ImageView {
     public ToolbarProgressBarAnimatingView(Context context, LayoutParams layoutParams) {
         super(context);
         setLayoutParams(layoutParams);
+        mIsCanceled = true;
         mIsRtl = LocalizationUtils.isLayoutRtl();
         mDpToPx = getResources().getDisplayMetrics().density;
 
@@ -241,7 +242,7 @@ public class ToolbarProgressBarAnimatingView extends ImageView {
      * @return True if the animation is running.
      */
     public boolean isRunning() {
-        return mAnimatorSet.isStarted();
+        return !mIsCanceled;
     }
 
     /**

chrome/android/javatests/src/org/chromium/chrome/browser/widget/ToolbarProgressBarTest.java

@@ -4,29 +4,31 @@
 
 package org.chromium.chrome.browser.widget;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
 import android.animation.Animator;
-import android.animation.Animator.AnimatorListener;
 import android.support.test.InstrumentationRegistry;
 import android.support.test.filters.MediumTest;
 import android.view.View;
 
-import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
 import org.chromium.base.ThreadUtils;
+import org.chromium.base.test.util.CallbackHelper;
 import org.chromium.base.test.util.CommandLineFlags;
 import org.chromium.base.test.util.Feature;
 import org.chromium.base.test.util.Restriction;
-import org.chromium.base.test.util.RetryOnFailure;
 import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.ChromeSwitches;
+import org.chromium.chrome.browser.util.MathUtils;
+import org.chromium.chrome.browser.widget.ClipDrawableProgressBar.ProgressBarObserver;
 import org.chromium.chrome.test.ChromeActivityTestRule;
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
-import org.chromium.content.browser.test.util.Criteria;
-import org.chromium.content.browser.test.util.CriteriaHelper;
 import org.chromium.content.browser.test.util.JavaScriptUtils;
 import org.chromium.content.browser.test.util.TestCallbackHelperContainer.OnPageFinishedHelper;
 import org.chromium.content.browser.test.util.TestCallbackHelperContainer.OnPageStartedHelper;
@@ -35,38 +37,89 @@ import org.chromium.content_public.browser.WebContents;
 import org.chromium.net.test.EmbeddedTestServer;
 import org.chromium.ui.test.util.UiRestriction;
 
+import java.util.concurrent.Callable;
 import java.util.concurrent.TimeoutException;
-import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.concurrent.atomic.AtomicReference;
 
 /**
  * Tests related to the ToolbarProgressBar.
  */
 @RunWith(ChromeJUnit4ClassRunner.class)
-@RetryOnFailure
 @CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE,
         ChromeActivityTestRule.DISABLE_NETWORK_PREDICTION_FLAG})
+@Restriction(UiRestriction.RESTRICTION_TYPE_PHONE)
 public class ToolbarProgressBarTest {
     @Rule
     public ChromeActivityTestRule<ChromeActivity> mActivityTestRule =
             new ChromeActivityTestRule<>(ChromeActivity.class);
 
-    static final int TEST_WAIT_TIME_MS = 60000;
     private static final String TEST_PAGE = "/chrome/test/data/android/progressbar_test.html";
 
+    private final CallbackHelper mProgressUpdateHelper = new CallbackHelper();
+    private final CallbackHelper mProgressVisibilityHelper = new CallbackHelper();
+    private ToolbarProgressBar mProgressBar;
+
     @Before
-    public void setUp() throws InterruptedException {
+    public void setUp() throws InterruptedException, TimeoutException {
         mActivityTestRule.startMainActivityOnBlankPage();
+        mProgressBar = mActivityTestRule.getActivity()
+                               .getToolbarManager()
+                               .getToolbarLayout()
+                               .getProgressBar();
+
+        mProgressBar.resetStartCountForTesting();
+
+        mProgressBar.setProgressBarObserver(new ProgressBarObserver() {
+            @Override
+            public void onVisibleProgressUpdated() {
+                mProgressUpdateHelper.notifyCalled();
+            }
+
+            @Override
+            public void onVisibilityChanged() {
+                mProgressVisibilityHelper.notifyCalled();
+            }
+        });
+
+        // Make sure the progress bar is invisible before starting any of the tests.
+        if (mProgressBar.getVisibility() == View.VISIBLE) {
+            int count = mProgressVisibilityHelper.getCallCount();
+            mProgressVisibilityHelper.waitForCallback(count, 1);
+        }
+    }
+
+    /**
+     * Get the current progress from the UI thread.
+     * @return The current progress displayed by the progress bar.
+     */
+    private float getProgress() {
+        return ThreadUtils.runOnUiThreadBlockingNoException(new Callable<Float>() {
+            @Override
+            public Float call() {
+                return mProgressBar.getProgress();
+            }
+        });
+    }
+
+    /**
+     * Get the current progress bar visibility from the UI thread.
+     * @return The current progress displayed by the progress bar.
+     */
+    private boolean isProgressBarVisible() {
+        return ThreadUtils.runOnUiThreadBlockingNoException(new Callable<Boolean>() {
+            @Override
+            public Boolean call() {
+                return mProgressBar.getVisibility() == View.VISIBLE;
+            }
+        });
     }
 
     /**
      * Test that the progress bar only traverses the page a single time per navigation.
      */
     @Test
-    @Feature({"Android-Toolbar"})
+    @Feature({"Android-Progress-Bar"})
     @MediumTest
-    @Restriction(UiRestriction.RESTRICTION_TYPE_PHONE)
-    public void testToolbarTraversesScreenOnce() throws InterruptedException, TimeoutException {
+    public void testProgressBarTraversesScreenOnce() throws InterruptedException, TimeoutException {
         EmbeddedTestServer testServer =
                 EmbeddedTestServer.createAndStartServer(InstrumentationRegistry.getContext());
 
@@ -79,17 +132,9 @@ public class ToolbarProgressBarTest {
         OnPageStartedHelper startHelper = observer.getOnPageStartedHelper();
         OnPageFinishedHelper finishHelper = observer.getOnPageFinishedHelper();
 
-        ToolbarProgressBar progressBar = mActivityTestRule.getActivity()
-                                                 .getToolbarManager()
-                                                 .getToolbarLayout()
-                                                 .getProgressBar();
-
-        // Reset progress bar start count in case anything else triggered it.
-        progressBar.resetStartCountForTesting();
-
         // Ensure no load events have occurred yet.
-        Assert.assertEquals(0, startHelper.getCallCount());
-        Assert.assertEquals(0, finishHelper.getCallCount());
+        assertEquals("Page load should not have started.", 0, startHelper.getCallCount());
+        assertEquals("Page load should not have finished.", 0, finishHelper.getCallCount());
 
         mActivityTestRule.loadUrl(testServer.getURL(TEST_PAGE));
 
@@ -99,115 +144,209 @@ public class ToolbarProgressBarTest {
         }
 
         // Exactly one start load and one finish load event should have occurred.
-        Assert.assertEquals(1, startHelper.getCallCount());
-        Assert.assertEquals(1, finishHelper.getCallCount());
+        assertEquals("Page load should have started.", 1, startHelper.getCallCount());
+        assertEquals("Page load should have finished.", 1, finishHelper.getCallCount());
 
         // Load content in the iframe of the test page to trigger another load.
         JavaScriptUtils.executeJavaScript(webContents, "loadIframeInPage();");
 
         // A load start will be triggered.
         startHelper.waitForCallback(startHelper.getCallCount(), 1);
-        Assert.assertEquals(2, startHelper.getCallCount());
+        assertEquals("Iframe should have triggered page load.", 2, startHelper.getCallCount());
 
         // Wait for the iframe to finish loading.
         finishHelper.waitForCallback(finishHelper.getCallCount(), 1);
-        Assert.assertEquals(2, finishHelper.getCallCount());
+        assertEquals("Iframe should have finished loading.", 2, finishHelper.getCallCount());
 
         // Though the page triggered two load events, the progress bar should have only appeared a
         // single time.
-        Assert.assertEquals(1, progressBar.getStartCountForTesting());
+        assertEquals("The progress bar should have only started once.", 1,
+                mProgressBar.getStartCountForTesting());
     }
 
     /**
-     * Test that calling progressBar.setProgress(# > 0) followed by progressBar.setProgress(0)
-     * results in a hidden progress bar.
-     * @throws InterruptedException
+     * Test that the progress bar indeterminate animation completely traverses the screen.
      */
     @Test
-    @Feature({"Android-Toolbar"})
+    @Feature({"Android-Progress-Bar"})
     @MediumTest
-    @Restriction(UiRestriction.RESTRICTION_TYPE_PHONE)
-    public void testProgressBarDisappearsAfterFastShowHide() throws InterruptedException {
-        // onAnimationEnd will be signaled on progress bar showing/hiding animation end.
-        final Object onAnimationEnd = new Object();
-        final AtomicBoolean animationEnded = new AtomicBoolean(false);
-        final AtomicReference<ToolbarProgressBar> progressBar =
-                new AtomicReference<>();
-        ThreadUtils.runOnUiThreadBlocking(new Runnable() {
-            @Override
-            public void run() {
-                progressBar.set(mActivityTestRule.getActivity()
-                                        .getToolbarManager()
-                                        .getToolbarLayout()
-                                        .getProgressBar());
-                progressBar.get().setAlphaAnimationDuration(10);
-                progressBar.get().setHidingDelay(10);
-                progressBar.get().animate().setListener(new AnimatorListener() {
-                    @Override
-                    public void onAnimationStart(Animator animation) {
-                    }
-
-                    @Override
-                    public void onAnimationRepeat(Animator animation) {
-                    }
-
-                    @Override
-                    public void onAnimationEnd(Animator animation) {
-                        synchronized (onAnimationEnd) {
-                            animationEnded.set(true);
-                            onAnimationEnd.notify();
-                        }
-                    }
-
-                    @Override
-                    public void onAnimationCancel(Animator animation) {
-                    }
-                });
-            }
+    public void testProgressBarCompletion_indeterminateAnimation()
+            throws InterruptedException, TimeoutException {
+        Animator progressAnimator = mProgressBar.getIndeterminateAnimatorForTesting();
+
+        int currentVisibilityCallCount = mProgressVisibilityHelper.getCallCount();
+
+        ThreadUtils.runOnUiThreadBlocking(() -> mProgressBar.start());
+        assertFalse("Indeterminate animation should not be running.", progressAnimator.isRunning());
+
+        ThreadUtils.runOnUiThreadBlocking(() -> {
+            mProgressBar.startIndeterminateAnimationForTesting();
+            mProgressBar.setProgress(0.5f);
         });
 
-        CriteriaHelper.pollUiThread(new Criteria("Progress bar not hidden at start") {
-            @Override
-            public boolean isSatisfied() {
-                return progressBar.get().getVisibility() == View.INVISIBLE;
-            }
+        // Wait for a visibility change.
+        mProgressVisibilityHelper.waitForCallback(currentVisibilityCallCount, 1);
+        currentVisibilityCallCount++;
+
+        assertTrue("Indeterminate animation should be running.", progressAnimator.isRunning());
+
+        // Wait for progress updates to reach 50%.
+        int currentProgressCallCount = mProgressUpdateHelper.getCallCount();
+        while (!MathUtils.areFloatsEqual(getProgress(), 0.5f)) {
+            mProgressUpdateHelper.waitForCallback(currentProgressCallCount, 1);
+            currentProgressCallCount++;
+        }
+
+        ThreadUtils.runOnUiThreadBlocking(() -> mProgressBar.finish(true));
+
+        // Wait for progress updates to reach 100%.
+        currentProgressCallCount = mProgressUpdateHelper.getCallCount();
+        while (!MathUtils.areFloatsEqual(getProgress(), 1.0f)) {
+            mProgressUpdateHelper.waitForCallback(currentProgressCallCount, 1);
+            currentProgressCallCount++;
+        }
+
+        // Make sure the progress bar remains visible through completion.
+        assertTrue("Progress bar should still be visible.", isProgressBarVisible());
+
+        assertEquals("Progress should have reached 100%.", 1.0f, getProgress(), MathUtils.EPSILON);
+
+        // Wait for a visibility change now that progress has completed.
+        mProgressVisibilityHelper.waitForCallback(currentVisibilityCallCount, 1);
+
+        assertFalse("Indeterminate animation should not be running.", progressAnimator.isRunning());
+        assertFalse("Progress bar should not be visible.", isProgressBarVisible());
+    }
+
+    /**
+     * Test that the progress bar completely traverses the screen without animation.
+     */
+    @Test
+    @Feature({"Android-Progress-Bar"})
+    @MediumTest
+    public void testProgressBarCompletion_noAnimation()
+            throws InterruptedException, TimeoutException {
+        int currentVisibilityCallCount = mProgressVisibilityHelper.getCallCount();
+        int currentProgressCallCount = mProgressUpdateHelper.getCallCount();
+
+        ThreadUtils.runOnUiThreadBlocking(() -> {
+            mProgressBar.start();
+            mProgressBar.setProgress(0.5f);
         });
 
-        // Make some progress and check that the progress bar is fully visible.
-        animationEnded.set(false);
-        synchronized (onAnimationEnd) {
-            ThreadUtils.runOnUiThread(new Runnable() {
-                @Override
-                public void run() {
-                    progressBar.get().start();
-                    progressBar.get().setProgress(0.5f);
-                }
-            });
-
-            long deadline = System.currentTimeMillis() + TEST_WAIT_TIME_MS;
-            while (!animationEnded.get() && System.currentTimeMillis() < deadline) {
-                onAnimationEnd.wait(deadline - System.currentTimeMillis());
-            }
-            Assert.assertEquals(1.0f, progressBar.get().getAlpha(), 0);
-            Assert.assertEquals(View.VISIBLE, progressBar.get().getVisibility());
+        // Wait for a visibility change.
+        mProgressVisibilityHelper.waitForCallback(currentVisibilityCallCount, 1);
+        currentVisibilityCallCount++;
+
+        // Wait for progress updates to reach 50%.
+        mProgressUpdateHelper.waitForCallback(currentProgressCallCount, 1);
+        currentProgressCallCount++;
+        assertEquals("Progress should have reached 50%.", 0.5f, getProgress(), MathUtils.EPSILON);
+
+        currentProgressCallCount = mProgressUpdateHelper.getCallCount();
+        ThreadUtils.runOnUiThreadBlocking(() -> mProgressBar.finish(true));
+
+        // Wait for progress updates to reach 100%.
+        mProgressUpdateHelper.waitForCallback(currentProgressCallCount, 1);
+        currentProgressCallCount++;
+        assertEquals("Progress should have reached 100%.", 1.0f, getProgress(), MathUtils.EPSILON);
+
+        // Make sure the progress bar remains visible through completion.
+        assertTrue("Progress bar should still be visible.", isProgressBarVisible());
+
+        // Wait for a visibility change now that progress has completed.
+        mProgressVisibilityHelper.waitForCallback(currentVisibilityCallCount, 1);
+
+        assertFalse("Progress bar should not be visible.", isProgressBarVisible());
+    }
+
+    /**
+     * Test that the progress bar ends immediately if #finish(...) is called with delay = false.
+     */
+    @Test
+    @Feature({"Android-Progress-Bar"})
+    @MediumTest
+    public void testProgressBarCompletion_indeterminateAnimation_noDelay()
+            throws InterruptedException, TimeoutException {
+        Animator progressAnimator = mProgressBar.getIndeterminateAnimatorForTesting();
+
+        int currentVisibilityCallCount = mProgressVisibilityHelper.getCallCount();
+
+        ThreadUtils.runOnUiThreadBlocking(() -> mProgressBar.start());
+        assertFalse("Indeterminate animation should not be running.", progressAnimator.isRunning());
+
+        ThreadUtils.runOnUiThreadBlocking(() -> {
+            mProgressBar.startIndeterminateAnimationForTesting();
+            mProgressBar.setProgress(0.5f);
+        });
+
+        // Wait for a visibility change.
+        mProgressVisibilityHelper.waitForCallback(currentVisibilityCallCount, 1);
+        currentVisibilityCallCount++;
+
+        assertTrue("Indeterminate animation should be running.", progressAnimator.isRunning());
+
+        // Wait for progress updates to reach 50%.
+        int currentProgressCallCount = mProgressUpdateHelper.getCallCount();
+        while (!MathUtils.areFloatsEqual(getProgress(), 0.5f)) {
+            mProgressUpdateHelper.waitForCallback(currentProgressCallCount, 1);
+            currentProgressCallCount++;
         }
 
-        // Clear progress and check that the progress bar is hidden.
-        animationEnded.set(false);
-        synchronized (onAnimationEnd) {
-            ThreadUtils.runOnUiThread(new Runnable() {
-                @Override
-                public void run() {
-                    progressBar.get().finish(true);
-                }
-            });
-
-            long deadline = System.currentTimeMillis() + TEST_WAIT_TIME_MS;
-            while (!animationEnded.get() && System.currentTimeMillis() < deadline) {
-                onAnimationEnd.wait(deadline - System.currentTimeMillis());
-            }
-            Assert.assertEquals(0.0f, progressBar.get().getAlpha(), 0);
-            Assert.assertNotSame(View.VISIBLE, progressBar.get().getVisibility());
+        // Finish progress with no delay.
+        ThreadUtils.runOnUiThreadBlocking(() -> mProgressBar.finish(false));
+
+        // The progress bar should immediately be invisible.
+        assertFalse("Progress bar should be invisible.", isProgressBarVisible());
+
+        assertFalse("Indeterminate animation should not be running.", progressAnimator.isRunning());
+    }
+
+    /**
+     * Test that the progress bar resets if a navigation occurs mid-progress while the indeterminate
+     * animation is running.
+     */
+    @Test
+    @Feature({"Android-Progress-Bar"})
+    @MediumTest
+    public void testProgressBarReset_indeterminateAnimation()
+            throws InterruptedException, TimeoutException {
+        Animator progressAnimator = mProgressBar.getIndeterminateAnimatorForTesting();
+
+        int currentVisibilityCallCount = mProgressVisibilityHelper.getCallCount();
+
+        ThreadUtils.runOnUiThreadBlocking(() -> mProgressBar.start());
+        assertFalse("Indeterminate animation should not be running.", progressAnimator.isRunning());
+
+        ThreadUtils.runOnUiThreadBlocking(() -> {
+            mProgressBar.startIndeterminateAnimationForTesting();
+            mProgressBar.setProgress(0.5f);
+        });
+
+        // Wait for a visibility change.
+        mProgressVisibilityHelper.waitForCallback(currentVisibilityCallCount, 1);
+        currentVisibilityCallCount++;
+
+        assertTrue("Indeterminate animation should be running.", progressAnimator.isRunning());
+
+        // Wait for progress updates to reach 50%.
+        int currentProgressCallCount = mProgressUpdateHelper.getCallCount();
+        while (!MathUtils.areFloatsEqual(getProgress(), 0.5f)) {
+            mProgressUpdateHelper.waitForCallback(currentProgressCallCount, 1);
+            currentProgressCallCount++;
         }
+
+        // Restart the progress bar.
+        currentProgressCallCount = mProgressUpdateHelper.getCallCount();
+        ThreadUtils.runOnUiThreadBlocking(() -> mProgressBar.start());
+
+        // Wait for progress update.
+        mProgressUpdateHelper.waitForCallback(currentProgressCallCount, 1);
+        currentProgressCallCount++;
+
+        // Make sure the progress bar remains visible through completion.
+        assertTrue("Progress bar should still be visible.", isProgressBarVisible());
+
+        assertEquals("Progress should be at 0%.", 0.0f, getProgress(), MathUtils.EPSILON);
     }
 }