Merge "Update the content capture text changed event merge logic" into sc-dev

3 files changed, 86 insertions, 26 deletions

diff --git a/core/java/android/view/contentcapture/ContentCaptureEvent.java b/core/java/android/view/contentcapture/ContentCaptureEvent.java
index 2b12230510bf..ce014693c4c4 100644
--- a/core/java/android/view/contentcapture/ContentCaptureEvent.java
+++ b/core/java/android/view/contentcapture/ContentCaptureEvent.java
@@ -143,6 +143,9 @@ public final class ContentCaptureEvent implements Parcelable {
     private @Nullable ContentCaptureContext mClientContext;
     private @Nullable Insets mInsets;
 
+    /** Only used in the main Content Capture session, no need to parcel */
+    private boolean mTextHasComposingSpan;
+
     /** @hide */
     public ContentCaptureEvent(int sessionId, int type, long eventTime) {
         mSessionId = sessionId;
@@ -243,11 +246,21 @@ public final class ContentCaptureEvent implements Parcelable {
 
     /** @hide */
     @NonNull
-    public ContentCaptureEvent setText(@Nullable CharSequence text) {
+    public ContentCaptureEvent setText(@Nullable CharSequence text, boolean hasComposingSpan) {
         mText = text;
+        mTextHasComposingSpan = hasComposingSpan;
         return this;
     }
 
+    /**
+     * The value is not parcelled, become false after parcelled.
+     * @hide
+     */
+    @NonNull
+    public boolean getTextHasComposingSpan() {
+        return mTextHasComposingSpan;
+    }
+
     /** @hide */
     @NonNull
     public ContentCaptureEvent setInsets(@NonNull Insets insets) {
@@ -361,7 +374,7 @@ public final class ContentCaptureEvent implements Parcelable {
             throw new IllegalArgumentException("mergeEvent(): got "
                     + "TYPE_VIEW_DISAPPEARED event with neither id or ids: " + event);
         } else if (eventType == TYPE_VIEW_TEXT_CHANGED) {
-            setText(event.getText());
+            setText(event.getText(), event.getTextHasComposingSpan());
         } else {
             Log.e(TAG, "mergeEvent(" + getTypeAsString(eventType)
                     + ") does not support this event type.");
@@ -479,7 +492,7 @@ public final class ContentCaptureEvent implements Parcelable {
             if (node != null) {
                 event.setViewNode(node);
             }
-            event.setText(parcel.readCharSequence());
+            event.setText(parcel.readCharSequence(), false);
             if (type == TYPE_SESSION_STARTED || type == TYPE_SESSION_FINISHED) {
                 event.setParentSessionId(parcel.readInt());
             }
diff --git a/core/java/android/view/contentcapture/MainContentCaptureSession.java b/core/java/android/view/contentcapture/MainContentCaptureSession.java
index 5ca793e3c394..f196f75861ec 100644
--- a/core/java/android/view/contentcapture/MainContentCaptureSession.java
+++ b/core/java/android/view/contentcapture/MainContentCaptureSession.java
@@ -43,12 +43,15 @@ import android.os.Handler;
 import android.os.IBinder;
 import android.os.IBinder.DeathRecipient;
 import android.os.RemoteException;
+import android.text.Spannable;
 import android.text.TextUtils;
+import android.util.ArrayMap;
 import android.util.LocalLog;
 import android.util.Log;
 import android.util.TimeUtils;
 import android.view.autofill.AutofillId;
 import android.view.contentcapture.ViewNode.ViewStructureImpl;
+import android.view.inputmethod.BaseInputConnection;
 
 import com.android.internal.os.IResultReceiver;
 
@@ -57,6 +60,7 @@ import java.lang.ref.WeakReference;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.Map;
 import java.util.concurrent.atomic.AtomicBoolean;
 
 /**
@@ -147,6 +151,12 @@ public final class MainContentCaptureSession extends ContentCaptureSession {
     private final LocalLog mFlushHistory;
 
     /**
+     * If the event in the buffer is of type {@link TYPE_VIEW_TEXT_CHANGED}, this value
+     * indicates whether the event has composing span or not.
+     */
+    private final Map<AutofillId, Boolean> mLastComposingSpan = new ArrayMap<>();
+
+    /**
      * Binder object used to update the session state.
      */
     @NonNull
@@ -335,26 +345,47 @@ public final class MainContentCaptureSession extends ContentCaptureSession {
         // Some type of events can be merged together
         boolean addEvent = true;
 
-        if (!mEvents.isEmpty() && eventType == TYPE_VIEW_TEXT_CHANGED) {
-            final ContentCaptureEvent lastEvent = mEvents.get(mEvents.size() - 1);
-
-            // We merge two consecutive text change event, unless one of them clears the text.
-            if (lastEvent.getType() == TYPE_VIEW_TEXT_CHANGED
-                    && lastEvent.getId().equals(event.getId())) {
-                boolean bothNonEmpty = !TextUtils.isEmpty(lastEvent.getText())
-                        && !TextUtils.isEmpty(event.getText());
-                boolean equalContent = TextUtils.equals(lastEvent.getText(), event.getText());
-                if (equalContent) {
-                    addEvent = false;
-                } else if (bothNonEmpty) {
-                    lastEvent.mergeEvent(event);
-                    addEvent = false;
-                }
-                if (!addEvent && sVerbose) {
-                    Log.v(TAG, "Buffering VIEW_TEXT_CHANGED event, updated text="
-                            + getSanitizedString(event.getText()));
+        if (eventType == TYPE_VIEW_TEXT_CHANGED) {
+            // We determine whether to add or merge the current event by following criteria:
+            // 1. Don't have composing span: always add.
+            // 2. Have composing span:
+            //    2.1 either last or current text is empty: add.
+            //    2.2 last event doesn't have composing span: add.
+            // Otherwise, merge.
+
+            final CharSequence text = event.getText();
+            final boolean textHasComposingSpan = event.getTextHasComposingSpan();
+
+            if (textHasComposingSpan && !mLastComposingSpan.isEmpty()) {
+                final Boolean lastEventHasComposingSpan = mLastComposingSpan.get(event.getId());
+                if (lastEventHasComposingSpan != null && lastEventHasComposingSpan.booleanValue()) {
+                    ContentCaptureEvent lastEvent = null;
+                    for (int index = mEvents.size() - 1; index >= 0; index--) {
+                        final ContentCaptureEvent tmpEvent = mEvents.get(index);
+                        if (event.getId().equals(tmpEvent.getId())) {
+                            lastEvent = tmpEvent;
+                            break;
+                        }
+                    }
+                    if (lastEvent != null) {
+                        final CharSequence lastText = lastEvent.getText();
+                        final boolean bothNonEmpty = !TextUtils.isEmpty(lastText)
+                                && !TextUtils.isEmpty(text);
+                        boolean equalContent = TextUtils.equals(lastText, text);
+                        if (equalContent) {
+                            addEvent = false;
+                        } else if (bothNonEmpty && lastEventHasComposingSpan) {
+                            lastEvent.mergeEvent(event);
+                            addEvent = false;
+                        }
+                        if (!addEvent && sVerbose) {
+                            Log.v(TAG, "Buffering VIEW_TEXT_CHANGED event, updated text="
+                                    + getSanitizedString(text));
+                        }
+                    }
                 }
             }
+            mLastComposingSpan.put(event.getId(), textHasComposingSpan);
         }
 
         if (!mEvents.isEmpty() && eventType == TYPE_VIEW_DISAPPEARED) {
@@ -374,6 +405,11 @@ public final class MainContentCaptureSession extends ContentCaptureSession {
             mEvents.add(event);
         }
 
+        // TODO: we need to change when the flush happens so that we don't flush while the
+        //  composing span hasn't changed. But we might need to keep flushing the events for the
+        //  non-editable views and views that don't have the composing state; otherwise some other
+        //  Content Capture features may be delayed.
+
         final int numberEvents = mEvents.size();
 
         final boolean bufferEvent = numberEvents < maxBufferSize;
@@ -550,6 +586,7 @@ public final class MainContentCaptureSession extends ContentCaptureSession {
                 ? Collections.emptyList()
                 : mEvents;
         mEvents = null;
+        mLastComposingSpan.clear();
         return new ParceledListSlice<>(events);
     }
 
@@ -677,9 +714,16 @@ public final class MainContentCaptureSession extends ContentCaptureSession {
     }
 
     void notifyViewTextChanged(int sessionId, @NonNull AutofillId id, @Nullable CharSequence text) {
+        // Since the same CharSequence instance may be reused in the TextView, we need to make
+        // a copy of its content so that its value will not be changed by subsequent updates
+        // in the TextView.
+        final String eventText = text == null ? null : text.toString();
+        final boolean textHasComposingSpan =
+                text instanceof Spannable && BaseInputConnection.getComposingSpanStart(
+                        (Spannable) text) >= 0;
         mHandler.post(() -> sendEvent(
                 new ContentCaptureEvent(sessionId, TYPE_VIEW_TEXT_CHANGED)
-                        .setAutofillId(id).setText(text)));
+                        .setAutofillId(id).setText(eventText, textHasComposingSpan)));
     }
 
     /** Public because is also used by ViewRootImpl */
diff --git a/core/tests/coretests/src/android/view/contentcapture/ContentCaptureEventTest.java b/core/tests/coretests/src/android/view/contentcapture/ContentCaptureEventTest.java
index 67614bb22e4e..e6a25d00ff10 100644
--- a/core/tests/coretests/src/android/view/contentcapture/ContentCaptureEventTest.java
+++ b/core/tests/coretests/src/android/view/contentcapture/ContentCaptureEventTest.java
@@ -236,12 +236,13 @@ public class ContentCaptureEventTest {
     @Test
     public void testMergeEvent_typeViewTextChanged() {
         final ContentCaptureEvent event = new ContentCaptureEvent(42, TYPE_VIEW_TEXT_CHANGED)
-                .setText("test");
+                .setText("test", false);
         final ContentCaptureEvent event2 = new ContentCaptureEvent(43, TYPE_VIEW_TEXT_CHANGED)
-                .setText("empty");
+                .setText("empty", true);
 
         event.mergeEvent(event2);
         assertThat(event.getText()).isEqualTo(event2.getText());
+        assertThat(event.getTextHasComposingSpan()).isEqualTo(event2.getTextHasComposingSpan());
     }
 
     @Test
@@ -282,16 +283,18 @@ public class ContentCaptureEventTest {
     @Test
     public void testMergeEvent_differentEventTypes() {
         final ContentCaptureEvent event = new ContentCaptureEvent(42, TYPE_VIEW_DISAPPEARED)
-                .setText("test").setAutofillId(new AutofillId(1));
+                .setText("test", false).setAutofillId(new AutofillId(1));
         final ContentCaptureEvent event2 = new ContentCaptureEvent(17, TYPE_VIEW_TEXT_CHANGED)
-                .setText("empty").setAutofillId(new AutofillId(2));
+                .setText("empty", true).setAutofillId(new AutofillId(2));
 
         event.mergeEvent(event2);
         assertThat(event.getText()).isEqualTo("test");
+        assertThat(event.getTextHasComposingSpan()).isFalse();
         assertThat(event.getId()).isEqualTo(new AutofillId(1));
 
         event2.mergeEvent(event);
         assertThat(event2.getText()).isEqualTo("empty");
+        assertThat(event2.getTextHasComposingSpan()).isTrue();
         assertThat(event2.getId()).isEqualTo(new AutofillId(2));
     }