Change Show in folder to single pulse + test

Bug: 994293
Change-Id: I2446ed845f08b5173ac25fe836e50b114f58cf45
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1756545
Commit-Queue: Theresa  <twellington@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#688295}

chrome/android/java/src/org/chromium/chrome/browser/bookmarks/ReorderBookmarkItemsAdapter.java

@@ -218,8 +218,10 @@ class ReorderBookmarkItemsAdapter extends DragReorderableListAdapter<BookmarkIte
             });
             // Turn on the highlight for the currently highlighted bookmark.
             if (id.equals(mHighlightedBookmark)) {
-                ViewHighlighter.turnOnHighlight(holder.itemView, false);
+                ViewHighlighter.pulseHighlight(holder.itemView, false, 1);
+                clearHighlight();
             } else {
+                // We need this in case we are change state during a pulse.
                 ViewHighlighter.turnOffHighlight(holder.itemView);
             }
         }

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

@@ -33,6 +33,32 @@ public class PulseDrawable extends Drawable implements Animatable {
     private static final long PULSE_DURATION_MS = 2500;
     private static final long FRAME_RATE = 60;
 
+    /**
+     * Informs the PulseDrawable about whether it can continue pulsing, and specifies a callback to
+     * be run when the PulseDrawable is finished pulsing.
+     */
+    public interface PulseEndAuthority {
+        /**
+         * Called at the end of one pulse animation, to decide whether the PulseDrawable can pulse
+         * again.
+         *
+         * @return True iff the PulseDrawable can continue pulsing.
+         */
+        boolean canPulseAgain();
+    }
+
+    /**
+     * A PulseEndAuthority which allows the PulseDrawable to pulse forever.
+     */
+    private static class EndlessPulser implements PulseEndAuthority {
+        // PulseEndAuthority implementation.
+
+        @Override
+        public boolean canPulseAgain() {
+            return true;
+        }
+    }
+
     /**
      * An interface that does the actual drawing work for this {@link Drawable}.  Not meant to be
      * stateful, as this could be shared across multiple instances of this drawable if it gets
@@ -80,7 +106,7 @@ public class PulseDrawable extends Drawable implements Animatable {
     }
 
     private static Painter createCirclePainter(Bounds boundsFn) {
-        return new PulseDrawable.Painter() {
+        return new Painter() {
             @Override
             public void modifyDrawable(PulseDrawable drawable, float interpolation) {
                 drawable.invalidateSelf();
@@ -93,7 +119,8 @@ public class PulseDrawable extends Drawable implements Animatable {
 
                 float minRadiusPx = boundsFn.getMinRadiusPx(bounds);
                 float maxRadiusPx = boundsFn.getMaxRadiusPx(bounds);
-                float radius = MathUtils.interpolate(minRadiusPx, maxRadiusPx, interpolation);
+                float radius =
+                        MathUtils.interpolate(minRadiusPx, maxRadiusPx, 1.0f - interpolation);
 
                 canvas.drawCircle(bounds.exactCenterX(), bounds.exactCenterY(), radius, paint);
             }
@@ -103,13 +130,15 @@ public class PulseDrawable extends Drawable implements Animatable {
     /**
      * Creates a {@link PulseDrawable} that will fill the bounds with a pulsing color.
      * @param context The {@link Context} under which the drawable is created.
+     * @param pulseEndAuthority The {@link PulseEndAuthority} associated with this drawable.
      * @return A new {@link PulseDrawable} instance.
      */
-    public static PulseDrawable createHighlight(Context context) {
-        PulseDrawable.Painter painter = new PulseDrawable.Painter() {
+    public static PulseDrawable createHighlight(
+            Context context, PulseEndAuthority pulseEndAuthority) {
+        Painter painter = new Painter() {
             @Override
             public void modifyDrawable(PulseDrawable drawable, float interpolation) {
-                drawable.setAlpha((int) MathUtils.interpolate(12, 75, 1.f - interpolation));
+                drawable.setAlpha((int) MathUtils.interpolate(12, 75, interpolation));
             }
 
             @Override
@@ -119,15 +148,28 @@ public class PulseDrawable extends Drawable implements Animatable {
             }
         };
 
-        return new PulseDrawable(context, new FastOutSlowInInterpolator(), painter);
+        return new PulseDrawable(
+                context, new FastOutSlowInInterpolator(), painter, pulseEndAuthority);
+    }
+
+    /**
+     * Creates a {@link PulseDrawable} that will fill the bounds with a pulsing color. The {@link
+     * PulseDrawable} will continue pulsing forever (if this is not the desired behavior, please use
+     * {@link PulseEndAuthority}).
+     * @param context The {@link Context} under which the drawable is created.
+     * @return A new {@link PulseDrawable} instance.
+     */
+    public static PulseDrawable createHighlight(Context context) {
+        return createHighlight(context, new EndlessPulser());
     }
 
     /**
      * Creates a {@link PulseDrawable} that will draw a pulsing circle inside the bounds.
      * @param context The {@link Context} under which the drawable is created.
+     * @param pulseEndAuthority The {@link PulseEndAuthority} associated with this drawable.
      * @return A new {@link PulseDrawable} instance.
      */
-    public static PulseDrawable createCircle(Context context) {
+    public static PulseDrawable createCircle(Context context, PulseEndAuthority pulseEndAuthority) {
         final int startingPulseRadiusPx =
                 context.getResources().getDimensionPixelSize(R.dimen.iph_pulse_baseline_radius);
 
@@ -142,7 +184,18 @@ public class PulseDrawable extends Drawable implements Animatable {
                 return Math.min(
                         startingPulseRadiusPx, Math.min(bounds.width(), bounds.height()) / 2.f);
             }
-        });
+        }, pulseEndAuthority);
+    }
+
+    /**
+     * Creates a {@link PulseDrawable} that will draw a pulsing circle inside the bounds. The {@link
+     * PulseDrawable} will continue pulsing forever (if this is not the desired behavior, please use
+     * {@link PulseEndAuthority}).
+     * @param context The {@link Context} under which the drawable is created.
+     * @return A new {@link PulseDrawable} instance.
+     */
+    public static PulseDrawable createCircle(Context context) {
+        return createCircle(context, new EndlessPulser());
     }
 
     /**
@@ -151,15 +204,26 @@ public class PulseDrawable extends Drawable implements Animatable {
      * @param context The {@link Context} under which the drawable is created.
      * @return A new {@link PulseDrawable} instance.
      */
-    public static PulseDrawable createCustomCircle(Context context, Bounds boundsfn) {
+    public static PulseDrawable createCustomCircle(
+            Context context, Bounds boundsfn, PulseEndAuthority pulseEndAuthority) {
         Painter painter = createCirclePainter(boundsfn);
 
-        PulseDrawable drawable = new PulseDrawable(
-                context, PathInterpolatorCompat.create(.8f, 0.f, .6f, 1.f), painter);
+        PulseDrawable drawable = new PulseDrawable(context,
+                PathInterpolatorCompat.create(.8f, 0.f, .6f, 1.f), painter, pulseEndAuthority);
         drawable.setAlpha(76);
         return drawable;
     }
 
+    /**
+     * Creates a {@link PulseDrawable} that will draw a pulsing circle as large as possible inside
+     * the bounds.
+     * @param context The {@link Context} under which the drawable is created.
+     * @return A new {@link PulseDrawable} instance.
+     */
+    public static PulseDrawable createCustomCircle(Context context, Bounds boundsfn) {
+        return createCustomCircle(context, boundsfn, new EndlessPulser());
+    }
+
     private final Runnable mNextFrame = new Runnable() {
         @Override
         public void run() {
@@ -176,20 +240,30 @@ public class PulseDrawable extends Drawable implements Animatable {
     private PulseState mState;
     private boolean mMutated;
     private boolean mRunning;
+    private long mLastUpdateTime;
+
+    private final PulseEndAuthority mPulseEndAuthority;
 
     /**
      * Creates a new {@link PulseDrawable} instance.
      * @param context The {@link Context} under which the drawable is created.
      * @param interpolator An {@link Interpolator} that defines how the pulse will fade in and out.
      * @param painter      The {@link Painter} that will be responsible for drawing the pulse.
+     * @param pulseEndAuthority The {@link PulseEndAuthority} that is associated with this drawable.
      */
-    private PulseDrawable(Context context, Interpolator interpolator, Painter painter) {
-        this(new PulseState(interpolator, painter));
+    private PulseDrawable(Context context, Interpolator interpolator, Painter painter,
+            PulseEndAuthority pulseEndAuthority) {
+        this(new PulseState(interpolator, painter), pulseEndAuthority);
         setUseLightPulseColor(context.getResources(), false);
     }
 
-    private PulseDrawable(PulseState state) {
+    private PulseDrawable(PulseState state, PulseEndAuthority pulseEndAuthority) {
         mState = state;
+        mPulseEndAuthority = pulseEndAuthority;
+    }
+
+    private PulseDrawable(PulseState state) {
+        this(state, new EndlessPulser());
     }
 
     /**
@@ -222,7 +296,10 @@ public class PulseDrawable extends Drawable implements Animatable {
             scheduleSelf(mNextFrame, SystemClock.uptimeMillis() + 1000 / FRAME_RATE);
         } else {
             mRunning = true;
-            if (mState.startTime == 0) mState.startTime = SystemClock.uptimeMillis();
+            if (mState.startTime == 0) {
+                mState.startTime = SystemClock.uptimeMillis();
+                mLastUpdateTime = mState.startTime;
+            }
             mNextFrame.run();
         }
     }
@@ -312,16 +389,25 @@ public class PulseDrawable extends Drawable implements Animatable {
 
     private void stepPulse() {
         long curTime = SystemClock.uptimeMillis();
+        // If we are on a new pulse
+        if ((mLastUpdateTime - mState.startTime) / PULSE_DURATION_MS
+                != (curTime - mState.startTime) / PULSE_DURATION_MS) {
+            if (!(mPulseEndAuthority.canPulseAgain())) {
+                stop();
+                return;
+            }
+        }
         long msIntoAnim = (curTime - mState.startTime) % PULSE_DURATION_MS;
-        float progress = ((float) msIntoAnim) / ((float) PULSE_DURATION_MS);
-        mState.progress = mState.interpolator.getInterpolation(progress);
+        float timeProgress = ((float) msIntoAnim) / ((float) PULSE_DURATION_MS);
+        mState.progress = mState.interpolator.getInterpolation(timeProgress);
         mState.painter.modifyDrawable(PulseDrawable.this, mState.progress);
+        mLastUpdateTime = curTime;
     }
 
     /**
      * The {@link ConstantState} subclass for this {@link PulseDrawable}.
      */
-    private static final class PulseState extends ConstantState {
+    static final class PulseState extends ConstantState {
         // Current Paint State.
         /** The current color, including alpha, to draw. */
         public int drawColor;

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

@@ -23,8 +23,8 @@ public class PulseInterpolator implements Interpolator {
 
     @Override
     public float getInterpolation(float input) {
-        if (input < 0.4f) return 0.f;
-        if (input < 0.8f) return mInterpolator.getInterpolation((input - 0.4f) / 0.4f);
-        return mInterpolator.getInterpolation(1.f - (input - 0.8f) / 0.2f);
+        if (input < 0.2) return mInterpolator.getInterpolation(input / 0.2f);
+        if (input < 0.6) return 1.f;
+        return mInterpolator.getInterpolation(1.f - (input - 0.6f) / 0.4f);
     }
 }
\ No newline at end of file

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

@@ -4,6 +4,8 @@
 
 package org.chromium.chrome.browser.widget;
 
+import static org.chromium.chrome.browser.widget.PulseDrawable.createCircle;
+
 import android.content.res.Resources;
 import android.graphics.drawable.Drawable;
 import android.graphics.drawable.LayerDrawable;
@@ -27,13 +29,44 @@ public class ViewHighlighter {
      */
     public static final int IPH_MIN_DELAY_BETWEEN_TWO_HIGHLIGHTS = 200;
 
+    /**
+     * Allows its associated PulseDrawable to pulse a specified number of times, then turns off the
+     * PulseDrawable highlight.
+     */
+    public static class NumberPulser implements PulseDrawable.PulseEndAuthority {
+        private final View mView;
+        private int mNumPulsesRemaining;
+
+        NumberPulser(View view, int numPulses) {
+            mView = view;
+            mNumPulsesRemaining = numPulses;
+        }
+
+        @Override
+        public boolean canPulseAgain() {
+            mNumPulsesRemaining--;
+            if (mNumPulsesRemaining == 0) ViewHighlighter.turnOffHighlight(mView);
+            return mNumPulsesRemaining > 0;
+        }
+    }
+
+    public static void pulseHighlight(View view, boolean circular, int numPulses) {
+        if (view == null) return;
+
+        PulseDrawable pulseDrawable = circular
+                ? createCircle(view.getContext(), new NumberPulser(view, numPulses))
+                : PulseDrawable.createHighlight(
+                        view.getContext(), new NumberPulser(view, numPulses));
+
+        attachViewAsHighlight(view, pulseDrawable);
+    }
+
     /**
      * Create a highlight layer over the view.
      * @param view The view to be highlighted.
      * @param circular Whether the highlight should be a circle or rectangle.
      */
     public static void turnOnHighlight(View view, boolean circular) {
-        if (view == null) return;
 
         PulseDrawable pulseDrawable = circular ? PulseDrawable.createCircle(view.getContext())
                                                : PulseDrawable.createHighlight(view.getContext());

chrome/android/javatests/src/org/chromium/chrome/browser/ViewHighlighterTestUtils.java

@@ -9,6 +9,7 @@ import android.graphics.drawable.LayerDrawable;
 import android.view.View;
 
 import org.chromium.chrome.browser.widget.PulseDrawable;
+import org.chromium.content_public.browser.test.util.CriteriaHelper;
 
 /**
  * Allows for testing of views which are highlightable via ViewHighlighter.
@@ -40,4 +41,39 @@ public class ViewHighlighterTestUtils {
     public static boolean checkHighlightOff(View view) {
         return !(view.getBackground() instanceof LayerDrawable);
     }
+
+    /**
+     * Checks that the view is highlighted with a pulse highlight.
+     *
+     * @param view The view of interest.
+     * @param timeoutDuration The timeout duration (should be set depending on the number of pulses
+     *         and the pulse duration).
+     * @return True iff the view was highlighted, and then turned off.
+     */
+    public static boolean checkHighlightPulse(View view, long timeoutDuration) {
+        try {
+            CriteriaHelper.pollUiThread(()
+                                                -> checkHighlightOn(view),
+                    "Expected highlight to pulse on!", timeoutDuration,
+                    CriteriaHelper.DEFAULT_POLLING_INTERVAL);
+            CriteriaHelper.pollUiThread(()
+                                                -> checkHighlightOff(view),
+                    "Expected highlight to turn off!", timeoutDuration,
+                    CriteriaHelper.DEFAULT_POLLING_INTERVAL);
+        } catch (AssertionError e) {
+            e.printStackTrace();
+            return false;
+        }
+        return true;
+    }
+
+    /**
+     * Checks that the view is highlighted with a pulse highlight.
+     *
+     * @param view The view of interest.
+     * @return True iff the view was highlighted, and then turned off.
+     */
+    public static boolean checkHighlightPulse(View view) {
+        return checkHighlightPulse(view, CriteriaHelper.DEFAULT_MAX_TIME_TO_POLL);
+    }
 }
\ No newline at end of file

chrome/android/javatests/src/org/chromium/chrome/browser/bookmarks/BookmarkReorderTest.java

@@ -10,7 +10,7 @@ import static android.support.test.espresso.assertion.ViewAssertions.doesNotExis
 import static android.support.test.espresso.matcher.ViewMatchers.withText;
 
 import static org.chromium.chrome.browser.ViewHighlighterTestUtils.checkHighlightOff;
-import static org.chromium.chrome.browser.ViewHighlighterTestUtils.checkHighlightOn;
+import static org.chromium.chrome.browser.ViewHighlighterTestUtils.checkHighlightPulse;
 
 import android.support.test.filters.MediumTest;
 import android.support.v7.widget.RecyclerView.ViewHolder;
@@ -628,9 +628,9 @@ public class BookmarkReorderTest extends BookmarkTest {
         TestThreadUtils.runOnUiThreadBlocking(more::performClick);
         onView(withText("Show in folder")).perform(click());
 
-        Assert.assertTrue(
-                "Expected bookmark row to be highlighted after clicking \"show in folder\"",
-                checkHighlightOn(testFolder));
+        // Assert that the view pulses.
+        Assert.assertTrue("Expected bookmark row to pulse after clicking \"show in folder\"!",
+                checkHighlightPulse(testFolder));
 
         // Enter search mode again.
         searchButton = mManager.getToolbarForTests().findViewById(R.id.search_menu_id);
@@ -645,11 +645,9 @@ public class BookmarkReorderTest extends BookmarkTest {
         // Click "Show in folder" again.
         TestThreadUtils.runOnUiThreadBlocking(more::performClick);
         onView(withText("Show in folder")).perform(click());
-
-        // Check that the highlight is on.
-        Assert.assertTrue("Expected highlight to successfully come back on"
-                        + " after clicking \"show in folder\" a 2nd time",
-                checkHighlightOn(testFolder));
+        Assert.assertTrue(
+                "Expected bookmark row to pulse after clicking \"show in folder\" a 2nd time!",
+                checkHighlightPulse(testFolder));
     }
 
     @Test
@@ -691,8 +689,8 @@ public class BookmarkReorderTest extends BookmarkTest {
                 "Expected list to scroll bookmark item into view", testFolderInList == null);
         Assert.assertEquals("Wrong bookmark item selected.", TEST_FOLDER_TITLE,
                 ((BookmarkFolderRow) testFolderInList.itemView).getTitle());
-        Assert.assertTrue("Expected bookmark item to be highlighted after scrolling to it.",
-                checkHighlightOn(testFolderInList.itemView));
+        Assert.assertTrue("Expected highlight to pulse on after scrolling to the item!",
+                checkHighlightPulse(testFolderInList.itemView));
     }
 
     @Test
@@ -726,9 +724,9 @@ public class BookmarkReorderTest extends BookmarkTest {
         View itemA = mItemsContainer.findViewHolderForAdapterPosition(1).itemView;
         Assert.assertEquals("Wrong bookmark item selected.", TEST_TITLE_A,
                 ((BookmarkItemRow) itemA).getTitle());
-        Assert.assertTrue(
-                "Expected bookmark item to be highlighted after opening it in new folder.",
-                checkHighlightOn(itemA));
+
+        Assert.assertTrue("Expected highlight to pulse after opening an item in another folder!",
+                checkHighlightPulse(itemA));
 
         // Open mobile bookmarks folder, then go back to the subfolder.
         TestThreadUtils.runOnUiThreadBlocking(() -> {
@@ -737,12 +735,12 @@ public class BookmarkReorderTest extends BookmarkTest {
         });
         RecyclerViewTestUtils.waitForStableRecyclerView(mItemsContainer);
 
-        itemA = mItemsContainer.findViewHolderForAdapterPosition(1).itemView;
+        View itemASecondView = mItemsContainer.findViewHolderForAdapterPosition(1).itemView;
         Assert.assertEquals("Wrong bookmark item selected.", TEST_TITLE_A,
-                ((BookmarkItemRow) itemA).getTitle());
-        Assert.assertTrue("Expected bookmark item to not be highlighted after "
-                        + "exiting and re-entering folder.",
-                checkHighlightOff(itemA));
+                ((BookmarkItemRow) itemASecondView).getTitle());
+        Assert.assertTrue(
+                "Expected highlight to not be highlighted after exiting and re-entering folder!",
+                checkHighlightOff(itemASecondView));
     }
 
     @Override