From 1fdc14b8a9b373a4219dc61f34baca9524e6c7ec Mon Sep 17 00:00:00 2001
From: Yohei Yukawa <yukawa@google.com>
Date: Tue, 14 Dec 2021 15:15:54 -0800
Subject: [PATCH] Fix an off-by-one bug in
 EditableInputConnection#endBatchEdit() return value

This is a follow up CL to our previous CL [1], which had an off-by-one
bug when determining the return value of

   EditableInputConnection#endBatchEdit().

According to the API document of InputConnection#endBatchEdit(), the
following test should pass.

   EditText editText = new EditText(context);
   EditorInfo editorInfo = new EditorInfo();
   InputConnection editableInputConnection =
           editText.onCreateInputConnection(editorInfo);
   assertThat(editableInputConnection.beginBatchEdit()).isTrue();
   assertThat(editableInputConnection.beginBatchEdit()).isTrue();
   assertThat(editableInputConnection.endBatchEdit()).isTrue();
   assertThat(editableInputConnection.endBatchEdit()).isFalse(); // (*)
   assertThat(editableInputConnection.endBatchEdit()).isFalse();

However, the last assertion marked with (*) actually fails due to an
off-by-one bug.  This CL finally fixes it.

The risk of app compat breakages because of fixing this long standing
bug is supposed to be low, mainly because:

 * the system has not relied on this return value yet.
 * Widgets like WebView have correctly implemented this API.
 * IME has always received true no matter what the app returned, which
   is the same behavior as other async InputConnection APIs.

This CL adds several notes to InputConnection#endBatchEdit() document
to help developers correctly implement and use this API.

 [1]: I1ec5518fdc16fb0551fbce9d13f5d92eb4bc78c0
      c478c171e92b2f255e9699d9c9306b001368ac20

Fix: 209958658
Fix: 210165648
Test: atest -c CtsInputMethodTestCases:EditTextImeSupportTest
Change-Id: Ibc40072fa11a4d6e3c24b8d7860c914ccdbcbc8a
Merged-In: Ibc40072fa11a4d6e3c24b8d7860c914ccdbcbc8a
(cherry picked from commit beda2b7f76e9372db5b4d52bbfac3910cb2fda4d)
---
 .../view/inputmethod/InputConnection.java     | 27 +++++++++++--------
 .../widget/EditableInputConnection.java       |  2 +-
 2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/core/java/android/view/inputmethod/InputConnection.java b/core/java/android/view/inputmethod/InputConnection.java
index 5f036a3488083..4d7d182f00137 100644
--- a/core/java/android/view/inputmethod/InputConnection.java
+++ b/core/java/android/view/inputmethod/InputConnection.java
@@ -770,20 +770,25 @@ public interface InputConnection {
     boolean beginBatchEdit();
 
     /**
-     * Tell the editor that you are done with a batch edit previously
-     * initiated with {@link #beginBatchEdit}. This ends the latest
-     * batch only.
+     * Tell the editor that you are done with a batch edit previously initiated with
+     * {@link #beginBatchEdit()}. This ends the latest batch only.
      *
-     * <p><strong>IME authors:</strong> make sure you call this
-     * exactly once for each call to {@link #beginBatchEdit}.</p>
+     * <p><strong>IME authors:</strong> make sure you call this exactly once for each call to
+     * {@link #beginBatchEdit()}.</p>
      *
-     * <p><strong>Editor authors:</strong> please be careful about
-     * batch edit nesting. Updates still to be held back until the end
-     * of the last batch edit.</p>
+     * <p><strong>Editor authors:</strong> please be careful about batch edit nesting. Updates still
+     * to be held back until the end of the last batch edit.  In case you are delegating this API
+     * call to the one obtained from
+     * {@link android.widget.EditText#onCreateInputConnection(EditorInfo)}, there was an off-by-one
+     * that had returned {@code true} when its nested batch edit count becomes {@code 0} as a result
+     * of invoking this API.  This bug is fixed in {@link android.os.Build.VERSION_CODES#TIRAMISU}.
+     * </p>
      *
-     * @return true if there is still a batch edit in progress after closing
-     * the latest one (in other words, if the nesting count is > 0), false
-     * otherwise or if the input connection is no longer valid.
+     * @return For editor authors, you must return {@code true} if a batch edit is still in progress
+     *         after closing the latest one (in other words, if the nesting count is still a
+     *         positive number). Return {@code false} otherwise.  For IME authors, you will
+     *         always receive {@code true} as long as the request was sent to the editor, and
+     *         receive {@code false} only if the input connection is no longer valid.
      */
     boolean endBatchEdit();
 
diff --git a/core/java/com/android/internal/widget/EditableInputConnection.java b/core/java/com/android/internal/widget/EditableInputConnection.java
index 02ffe8c5268ef..040d78b65afa1 100644
--- a/core/java/com/android/internal/widget/EditableInputConnection.java
+++ b/core/java/com/android/internal/widget/EditableInputConnection.java
@@ -92,7 +92,7 @@ public class EditableInputConnection extends BaseInputConnection
                 // contribution to mTextView's nested batch edit count is zero.
                 mTextView.endBatchEdit();
                 mBatchEditNesting--;
-                return true;
+                return mBatchEditNesting > 0;
             }
         }
         return false;