Introduce Queue-Based Notification Snackbars

chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarManager.java

Index: chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarManager.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarManager.java b/chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarManager.java
index 688d122e955d5c961e2706331777500532e66119..9d36db0149bee0aed5ecc3c42e7ce9fc0ac0cfb4 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarManager.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarManager.java
@@ -15,25 +15,23 @@ import android.view.Window;
 import org.chromium.base.ApiCompatibilityUtils;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.chrome.R;
-import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.device.DeviceClassManager;
 import org.chromium.ui.UiUtils;
 import org.chromium.ui.base.DeviceFormFactor;
 
-import java.util.Stack;
+import java.util.Deque;
+import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.Queue;
 
 /**
- * Manager for the snackbar showing at the bottom of activity.
+ * Manager for the snackbar showing at the bottom of activity. There should be only one
+ * SnackbarManager and one snackbar in the activity.
  * <p/>
- * There should be only one SnackbarManager and one snackbar in the activity. The manager maintains
- * a stack to store all entries that should be displayed. When showing a new snackbar, old entry
- * will be pushed to stack and text/button will be updated to the newest entry.
- * <p/>
- * When action button is clicked, this manager will call
- * {@link SnackbarController#onAction(Object)} in corresponding listener, and show the next
- * entry in stack. Otherwise if no action is taken by user during
- * {@link #DEFAULT_SNACKBAR_DURATION_MS} milliseconds, it will clear the stack and call
- * {@link SnackbarController#onDismissNoAction(Object)} to all listeners.
+ * When action button is clicked, this manager will call {@link SnackbarController#onAction(Object)}
+ * in corresponding listener, and show the next entry. Otherwise if no action is taken by user
+ * during {@link #DEFAULT_SNACKBAR_DURATION_MS} milliseconds, it will call
+ * {@link SnackbarController#onDismissNoAction(Object)}.
  */
 public class SnackbarManager implements OnClickListener, OnGlobalLayoutListener {
 
@@ -78,17 +76,18 @@ public class SnackbarManager implements OnClickListener, OnGlobalLayoutListener
 
     private View mDecor;
     private final Handler mUIThreadHandler;
-    private Stack<Snackbar> mStack = new Stack<Snackbar>();
+    private SnackbarCollection mSnackbars = new SnackbarCollection();
     private SnackbarPopupWindow mPopup;
     private boolean mActivityInForeground;
     private final Runnable mHideRunnable = new Runnable() {
         @Override
         public void run() {
-            dismissAllSnackbars(true);
+            mSnackbars.jumpToQueue();

[newt (away), 2016/01/27 02:13:41]

Instead of calling jumpToQueue(), I'd call something like
mSnackbars().removeCurrentDueToTimeout().

[Ian Wen, 2016/01/27 18:14:07]

Done.

+            updatePopup();
         }
     };
 
-    // Variables used and reused in local calculations.
+    // Variables used and reused in popup position calculations.
     private int[] mTempDecorPosition = new int[2];
     private Rect mTempVisibleDisplayFrame = new Rect();
 
@@ -112,95 +111,31 @@ public class SnackbarManager implements OnClickListener, OnGlobalLayoutListener
      * Notifies the snackbar manager that the activity has been pushed to background.
      */
     public void onStop() {
-        dismissAllSnackbars(false);
+        mSnackbars.clear();
+        updatePopup();
         mActivityInForeground = false;
     }
 
     /**
      * Shows a snackbar at the bottom of the screen, or above the keyboard if the keyboard is
-     * visible. If the currently displayed snackbar is forcing display, the new snackbar is added as
-     * the next to be displayed on the stack.
+     * visible.
      */
     public void showSnackbar(Snackbar snackbar) {
         if (!mActivityInForeground) return;
-
-        if (mPopup != null && !mStack.empty() && mStack.peek().getForceDisplay()) {
-            mStack.add(mStack.size() - 1, snackbar);
-            return;
-        }
-
-        int durationMs = snackbar.getDuration();
-        if (durationMs == 0) {
-            durationMs = DeviceClassManager.isAccessibilityModeEnabled(mDecor.getContext())
-                    ? sAccessibilitySnackbarDurationMs : sSnackbarDurationMs;
-        }
-
-        mUIThreadHandler.removeCallbacks(mHideRunnable);
-        mUIThreadHandler.postDelayed(mHideRunnable, durationMs);
-
-        mStack.push(snackbar);
-        if (mPopup == null) {
-            mPopup = new SnackbarPopupWindow(mDecor, this, snackbar);
-            showPopupAtBottom();
-            mDecor.getViewTreeObserver().addOnGlobalLayoutListener(this);
-        } else {
-            mPopup.update(snackbar, true);
-        }
-
+        mSnackbars.add(snackbar);
+        updatePopup();
         mPopup.announceforAccessibility();
     }
 
     /**
-     * Warning: Calling this method might cause cascading destroy loop, because you might trigger
-     * callbacks for other {@link SnackbarController}. This method is only meant to be used during
-     * {@link ChromeActivity}'s destruction routine. For other purposes, use
-     * {@link #dismissSnackbars(SnackbarController)} instead.
-     * <p>
-     * Dismisses all snackbars in stack. This will call
-     * {@link SnackbarController#onDismissNoAction(Object)} for every closing snackbar.
-     *
-     * @param isTimeout Whether dismissal was triggered by timeout.
-     */
-    public void dismissAllSnackbars(boolean isTimeout) {
-        mUIThreadHandler.removeCallbacks(mHideRunnable);
-
-        if (!mActivityInForeground) return;
-
-        if (mPopup != null) {
-            mPopup.dismiss();
-            mPopup = null;
-        }
-
-        while (!mStack.isEmpty()) {
-            Snackbar snackbar = mStack.pop();
-            snackbar.getController().onDismissNoAction(snackbar.getActionData());
-
-            if (isTimeout && !mStack.isEmpty() && mStack.peek().getForceDisplay()) {
-                showSnackbar(mStack.pop());
-                return;
-            }
-        }
-        mDecor.getViewTreeObserver().removeOnGlobalLayoutListener(this);
-    }
-
-    /**
      * Dismisses snackbars that are associated with the given {@link SnackbarController}.
      *
      * @param controller Only snackbars with this controller will be removed.
      */
     public void dismissSnackbars(SnackbarController controller) {
-        boolean isFound = false;
-        Snackbar[] snackbars = new Snackbar[mStack.size()];
-        mStack.toArray(snackbars);
-        for (Snackbar snackbar : snackbars) {
-            if (snackbar.getController() == controller) {
-                mStack.remove(snackbar);
-                isFound = true;
-            }
+        if (mSnackbars.removeMatchingSnackbars(controller)) {
+            updatePopup();
         }
-        if (!isFound) return;
-
-        finishSnackbarRemoval();
     }
 
     /**
@@ -210,26 +145,8 @@ public class SnackbarManager implements OnClickListener, OnGlobalLayoutListener
      * @param actionData Only snackbars whose action data is equal to actionData will be removed.
      */
     public void dismissSnackbars(SnackbarController controller, Object actionData) {
-        boolean isFound = false;
-        for (Snackbar snackbar : mStack) {
-            if (snackbar.getActionData() != null && snackbar.getActionData().equals(actionData)
-                    && snackbar.getController() == controller) {
-                mStack.remove(snackbar);
-                isFound = true;
-                break;
-            }
-        }
-        if (!isFound) return;
-
-        finishSnackbarRemoval();
-    }
-
-    private void finishSnackbarRemoval() {
-        if (mStack.isEmpty()) {
-            dismissAllSnackbars(false);
-        } else {
-            // Refresh the snackbar to let it show top of stack and have full timeout.
-            showSnackbar(mStack.pop());
+        if (mSnackbars.removeMatchingSnackbars(controller, actionData)) {
+            updatePopup();
         }
     }
 
@@ -238,34 +155,16 @@ public class SnackbarManager implements OnClickListener, OnGlobalLayoutListener
      */
     @Override
     public void onClick(View v) {
-        assert !mStack.isEmpty();
-
-        Snackbar snackbar = mStack.pop();
-        snackbar.getController().onAction(snackbar.getActionData());
-
-        if (!mStack.isEmpty()) {
-            showSnackbar(mStack.pop());
-        } else {
-            dismissAllSnackbars(false);
-        }
+        mSnackbars.removeCurrent(true);
+        updatePopup();
     }
 
-    private void showPopupAtBottom() {
-        // When the keyboard is showing, translating the snackbar upwards looks bad because it
-        // overlaps the keyboard. In this case, use an alternative animation without translation.
-        boolean isKeyboardShowing = UiUtils.isKeyboardShowing(mDecor.getContext(), mDecor);
-        mPopup.setAnimationStyle(isKeyboardShowing ? R.style.SnackbarAnimationWithKeyboard
-                : R.style.SnackbarAnimation);
-
-        mDecor.getLocationInWindow(mTempDecorPosition);
-        mDecor.getWindowVisibleDisplayFrame(mTempVisibleDisplayFrame);
-        int decorBottom = mTempDecorPosition[1] + mDecor.getHeight();
-        int visibleBottom = Math.min(mTempVisibleDisplayFrame.bottom, decorBottom);
-        int margin = mIsTablet ? mDecor.getResources().getDimensionPixelSize(
-                R.dimen.snackbar_tablet_margin) : 0;
-
-        mPopup.showAtLocation(mDecor, Gravity.START | Gravity.BOTTOM, margin,
-                decorBottom - visibleBottom + margin);
+    /**
+     * @return Whether there is a snackbar on screen.
+     */
+    public boolean isShowing() {
+        if (mPopup == null) return false;

[newt (away), 2016/01/27 02:13:40]

or just "return mPopup != null && mPopup.isShowing()"

[Ian Wen, 2016/01/27 18:14:07]

Done.

+        return mPopup.isShowing();
     }
 
     /**
@@ -296,11 +195,61 @@ public class SnackbarManager implements OnClickListener, OnGlobalLayoutListener
     }
 
     /**
-     * @return Whether there is a snackbar on screen.
+     * If {@link #mSnackbars} is empty, dismisses the popup; otherwise it updates the popup

[newt (away), 2016/01/27 02:13:40]

"Updates the snackbar popup window to reflect the value of
mSnackbars.currentSnackbar(), which may be null. This may show, change, or hide
the popup."

[Ian Wen, 2016/01/27 18:14:07]

Done.

      */
-    public boolean isShowing() {
-        if (mPopup == null) return false;
-        return mPopup.isShowing();
+    private void updatePopup() {
+        if (!mActivityInForeground) return;
+        Snackbar currentSnackbar = mSnackbars.getCurrent();
+        if (currentSnackbar == null) {
+            mUIThreadHandler.removeCallbacks(mHideRunnable);
+            if (mPopup != null) {
+                mPopup.dismiss();
+                mPopup = null;
+            }
+            mDecor.getViewTreeObserver().removeOnGlobalLayoutListener(this);
+        } else {
+            boolean refreshTimeout = true;

[newt (away), 2016/01/27 02:13:40]

or you could call this "popupChanged". That's a more accurate name (since this
also controls announcing for accessibility) and makes the comment below ("Only
refresh timeout when the popup's content has changed") unnecessary.

[Ian Wen, 2016/01/27 18:14:07]

Done.

+            if (mPopup == null) {
+                mPopup = new SnackbarPopupWindow(mDecor, this, currentSnackbar);
+                // When the keyboard is showing, translating the snackbar upwards looks bad because
+                // it overlaps the keyboard. In this case, use an alternative animation without
+                // translation.
+                boolean isKeyboardShowing = UiUtils.isKeyboardShowing(mDecor.getContext(), mDecor);
+                mPopup.setAnimationStyle(isKeyboardShowing ? R.style.SnackbarAnimationWithKeyboard
+                        : R.style.SnackbarAnimation);
+
+                mDecor.getLocationInWindow(mTempDecorPosition);
+                mDecor.getWindowVisibleDisplayFrame(mTempVisibleDisplayFrame);
+                int decorBottom = mTempDecorPosition[1] + mDecor.getHeight();
+                int visibleBottom = Math.min(mTempVisibleDisplayFrame.bottom, decorBottom);
+                int margin = mIsTablet ? mDecor.getResources().getDimensionPixelSize(
+                        R.dimen.snackbar_tablet_margin) : 0;
+
+                mPopup.showAtLocation(mDecor, Gravity.START | Gravity.BOTTOM, margin,
+                        decorBottom - visibleBottom + margin);
+                mDecor.getViewTreeObserver().addOnGlobalLayoutListener(this);
+            } else {
+                // Only refresh timeout when the popup's content has changed.
+                refreshTimeout = mPopup.update(currentSnackbar);
+            }
+
+            if (refreshTimeout) {
+                int durationMs = getDuration(currentSnackbar);
+                mUIThreadHandler.removeCallbacks(mHideRunnable);
+                mUIThreadHandler.postDelayed(mHideRunnable, durationMs);
+                mPopup.announceforAccessibility();
+            }
+        }
+
+    }
+
+    private int getDuration(Snackbar snackbar) {
+        int durationMs = snackbar.getDuration();
+        if (durationMs == 0) {
+            durationMs = DeviceClassManager.isAccessibilityModeEnabled(mDecor.getContext())
+                    ? sAccessibilitySnackbarDurationMs : sSnackbarDurationMs;
+        }
+        return durationMs;
     }
 
     /**
@@ -312,4 +261,105 @@ public class SnackbarManager implements OnClickListener, OnGlobalLayoutListener
         sSnackbarDurationMs = durationMs;
         sAccessibilitySnackbarDurationMs = durationMs;
     }
+
+    /**
+     * @return The currently showing snackbar. For testing only.
+     */
+    Snackbar getCurrentSnackbarForTesting() {
+        return mSnackbars.getCurrent();
+    }
+
+    private static class SnackbarCollection {
+        private Deque<Snackbar> mStack = new LinkedList<>();
+        private Queue<Snackbar> mQueue = new LinkedList<>();
+
+        public void add(Snackbar snackbar) {
+            if (snackbar.isOnStack()) {
+                if (getCurrent() != null && !getCurrent().isOnStack()) {

[newt (away), 2016/01/27 02:13:40]

this subtle behavior is worthy of an explanatory comment

[Ian Wen, 2016/01/27 18:14:07]

Done.

+                    removeCurrent(false);
+                }
+                mStack.push(snackbar);
+            } else {
+                mQueue.offer(snackbar);
+            }
+        }
+
+        public void removeCurrent(boolean isAction) {

[newt (away), 2016/01/27 02:13:40]

I'd explain what "isAction" means

[Ian Wen, 2016/01/27 18:14:07]

Done.

+            Snackbar current = null;

[newt (away), 2016/01/27 02:13:40]

simpler:

    Snackbar current = !mStack.isEmpty() ? mStack.pop() : mQueue.poll();

[Ian Wen, 2016/01/27 18:14:07]

Done.

+            if (mStack.isEmpty()) {
+                if (!mQueue.isEmpty()) {
+                    current = mQueue.poll();
+                }
+            } else {
+                current = mStack.pop();
+            }
+            if (current != null) {
+                SnackbarController controller = current.getController();
+                if (isAction) controller.onAction(current.getActionData());
+                else controller.onDismissNoAction(current.getActionData());
+            }
+        }
+
+        /**
+         * @return The snackbar that is currently displayed.
+         */
+        public Snackbar getCurrent() {
+            return !mStack.isEmpty() ? mStack.peek() : mQueue.peek();
+        }
+
+        public boolean isEmpty() {
+            return mStack.isEmpty() && mQueue.isEmpty();
+        }
+
+        public void clear() {
+            while (!isEmpty()) {
+                removeCurrent(false);
+            }
+        }
+
+        /**
+         * Removes all snackbars in stack, and show the snackbar in queue.
+         */
+        public void jumpToQueue() {

[newt (away), 2016/01/27 02:13:40]

This method should be internal -- SnackbarManager shouldn't know about the Queue
vs Stack snackbars.

[Ian Wen, 2016/01/27 18:14:07]

Renamed it to removeCurrentDueToTimeout().

+            Snackbar current = getCurrent();

[newt (away), 2016/01/27 02:13:41]

nit: remove these two lines. This function could be

    removeCurrent(false);
    Snackbar current;
    while (...) { ... }
    

[Ian Wen, 2016/01/27 18:14:07]

Done.

+            if (current == null) return;
+            removeCurrent(false);
+            while ((current = getCurrent()) != null && current.isOnStack()) {
+                removeCurrent(false);
+            }
+        }
+
+        public boolean removeMatchingSnackbars(SnackbarController controller) {
+            boolean isFound = false;

[newt (away), 2016/01/27 02:13:40]

s/isFound/snackbarRemoved/

[Ian Wen, 2016/01/27 18:14:07]

Done.

+            Iterator<Snackbar> iter = mStack.iterator();
+            while (iter.hasNext()) {
+                Snackbar snackbar = iter.next();
+                if (snackbar.getController() == controller) {
+                    iter.remove();
+                    isFound = true;
+                }
+            }
+            return isFound;
+        }
+
+        public boolean removeMatchingSnackbars(SnackbarController controller, Object data) {
+            boolean isFound = false;

[newt (away), 2016/01/27 02:13:40]

s/isFound/snackbarRemoved/

[Ian Wen, 2016/01/27 18:14:07]

Done.

+            Iterator<Snackbar> iter = mStack.iterator();
+            while (iter.hasNext()) {
+                Snackbar snackbar = iter.next();
+                if (snackbar.getController() == controller
+                        && objectsAreEqual(snackbar.getActionData(), data)) {
+                    iter.remove();
+                    isFound = true;
+                }
+            }
+            return isFound;
+        }
+
+        private static boolean objectsAreEqual(Object a, Object b) {
+            if (a == null && b == null) return true;
+            if (a == null || b == null) return false;
+            return a.equals(b);
+        }
+    }
 }