Ensure AdapterView doesn't post selection notifications forever

Previously we would loop forever if a selection notification was
posted after data had changed but the data changed bit was never
reset (e.g. a layout pass never occurred). This moves the pending
notification to occur as part of a layoutChildren() /
checkSelectionChanged() pass. If the client does horrible things
to prevent layout, no notification will occur -- but we won't loop
forever.

BUG: 17736536
Change-Id: I9773a769ad402c92dcbe2af7b8982d4443001961

1 files changed, 35 insertions, 9 deletions

diff --git a/core/java/android/widget/AdapterView.java b/core/java/android/widget/AdapterView.java
index b9f891ce71c9..5fa6e60c0dbc 100644
--- a/core/java/android/widget/AdapterView.java
+++ b/core/java/android/widget/AdapterView.java
@@ -215,7 +215,12 @@ public abstract class AdapterView<T extends Adapter> extends ViewGroup {
     private boolean mDesiredFocusableState;
     private boolean mDesiredFocusableInTouchModeState;
 
+    /** Lazily-constructed runnable for dispatching selection events. */
     private SelectionNotifier mSelectionNotifier;
+
+    /** Selection notifier that's waiting for the next layout pass. */
+    private SelectionNotifier mPendingSelectionNotifier;
+
     /**
      * When set to true, calls to requestLayout() will not propagate up the parent hierarchy.
      * This is used to layout the children during a layout pass.
@@ -854,39 +859,50 @@ public abstract class AdapterView<T extends Adapter> extends ViewGroup {
 
     private class SelectionNotifier implements Runnable {
         public void run() {
+            mPendingSelectionNotifier = null;
+
             if (mDataChanged) {
-                // Data has changed between when this SelectionNotifier
-                // was posted and now. We need to wait until the AdapterView
-                // has been synched to the new data.
+                // Data has changed between when this SelectionNotifier was
+                // posted and now. Postpone the notification until the next
+                // layout is complete and we run checkSelectionChanged().
                 if (getAdapter() != null) {
-                    post(this);
+                    mPendingSelectionNotifier = this;
                 }
             } else {
-                fireOnSelected();
-                performAccessibilityActionsOnSelected();
+                dispatchOnItemSelected();
             }
         }
     }
 
     void selectionChanged() {
+        // We're about to post or run the selection notifier, so we don't need
+        // a pending notifier.
+        mPendingSelectionNotifier = null;
+
         if (mOnItemSelectedListener != null
                 || AccessibilityManager.getInstance(mContext).isEnabled()) {
             if (mInLayout || mBlockLayoutRequests) {
                 // If we are in a layout traversal, defer notification
                 // by posting. This ensures that the view tree is
-                // in a consistent state and is able to accomodate
+                // in a consistent state and is able to accommodate
                 // new layout or invalidate requests.
                 if (mSelectionNotifier == null) {
                     mSelectionNotifier = new SelectionNotifier();
+                } else {
+                    removeCallbacks(mSelectionNotifier);
                 }
                 post(mSelectionNotifier);
             } else {
-                fireOnSelected();
-                performAccessibilityActionsOnSelected();
+                dispatchOnItemSelected();
             }
         }
     }
 
+    private void dispatchOnItemSelected() {
+        fireOnSelected();
+        performAccessibilityActionsOnSelected();
+    }
+
     private void fireOnSelected() {
         if (mOnItemSelectedListener == null) {
             return;
@@ -1042,12 +1058,22 @@ public abstract class AdapterView<T extends Adapter> extends ViewGroup {
         notifySubtreeAccessibilityStateChangedIfNeeded();
     }
 
+    /**
+     * Called after layout to determine whether the selection position needs to
+     * be updated. Also used to fire any pending selection events.
+     */
     void checkSelectionChanged() {
         if ((mSelectedPosition != mOldSelectedPosition) || (mSelectedRowId != mOldSelectedRowId)) {
             selectionChanged();
             mOldSelectedPosition = mSelectedPosition;
             mOldSelectedRowId = mSelectedRowId;
         }
+
+        // If we have a pending selection notification -- and we won't if we
+        // just fired one in selectionChanged() -- run it now.
+        if (mPendingSelectionNotifier != null) {
+            mPendingSelectionNotifier.run();
+        }
     }
 
     /**