Don't send the same values to onUpdateSelection repeatedly

If the IME is repeatedly changing the text in its
onUpdateSelection handler, this will crash it with a
stack overflow exception. It's better than the old behavior,
which would result in a busyloop likely to make the
device completely unresponsive.

Bug: 10301239
Change-Id: I170cfb8ef20fc056d4725931890a987aefcaea8b

2 files changed, 12 insertions, 2 deletions

diff --git a/core/java/android/inputmethodservice/InputMethodService.java b/core/java/android/inputmethodservice/InputMethodService.java
index 7f82ce3..9319d4a 100644
--- a/core/java/android/inputmethodservice/InputMethodService.java
+++ b/core/java/android/inputmethodservice/InputMethodService.java
@@ -1650,6 +1650,11 @@ public class InputMethodService extends AbstractInputMethodService {
      * the text.  This is called whether or not the input method has requested
      * extracted text updates, although if so it will not receive this call
      * if the extracted text has changed as well.
+     *
+     * <p>Be careful about changing the text in reaction to this call with
+     * methods such as setComposingText, commitText or
+     * deleteSurroundingText. If the cursor moves as a result, this method
+     * will be called again, which may result in an infinite loop.
      * 
      * <p>The default implementation takes care of updating the cursor in
      * the extract text, if it is being shown.
diff --git a/core/java/android/view/inputmethod/InputMethodManager.java b/core/java/android/view/inputmethod/InputMethodManager.java
index 54b87de..53f7c79 100644
--- a/core/java/android/view/inputmethod/InputMethodManager.java
+++ b/core/java/android/view/inputmethod/InputMethodManager.java
@@ -1412,12 +1412,17 @@ public final class InputMethodManager {
 
                 try {
                     if (DEBUG) Log.v(TAG, "SELECTION CHANGE: " + mCurMethod);
-                    mCurMethod.updateSelection(mCursorSelStart, mCursorSelEnd,
-                            selStart, selEnd, candidatesStart, candidatesEnd);
+                    final int oldSelStart = mCursorSelStart;
+                    final int oldSelEnd = mCursorSelEnd;
+                    // Update internal values before sending updateSelection to the IME, because
+                    // if it changes the text within its onUpdateSelection handler in a way that
+                    // does not move the cursor we don't want to call it again with the same values.
                     mCursorSelStart = selStart;
                     mCursorSelEnd = selEnd;
                     mCursorCandStart = candidatesStart;
                     mCursorCandEnd = candidatesEnd;
+                    mCurMethod.updateSelection(oldSelStart, oldSelEnd,
+                            selStart, selEnd, candidatesStart, candidatesEnd);
                 } catch (RemoteException e) {
                     Log.w(TAG, "IME died: " + mCurId, e);
                 }