From ec8e720ae209d975ac34dc66e51f280d9a9482ef Mon Sep 17 00:00:00 2001 From: Alan Viverette Date: Mon, 6 Oct 2014 15:33:24 -0700 Subject: [PATCH] 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 --- core/java/android/widget/AdapterView.java | 44 ++++++++++++++++++----- 1 file changed, 35 insertions(+), 9 deletions(-) diff --git a/core/java/android/widget/AdapterView.java b/core/java/android/widget/AdapterView.java index b9f891ce71c99..5fa6e60c0dbc9 100644 --- a/core/java/android/widget/AdapterView.java +++ b/core/java/android/widget/AdapterView.java @@ -215,7 +215,12 @@ public abstract class AdapterView 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 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 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(); + } } /**