From c8def0695b73b6ca5494ea28dda89db41edea060 Mon Sep 17 00:00:00 2001 From: Feng Cao Date: Thu, 4 Feb 2021 15:05:41 -0800 Subject: [PATCH] Update the content capture text changed event merge logic * This is only applicable when two consecutive CC events are both TYPE_VIEW_TEXT_CHANGED event for the same view * Before this change: we would merge if both events have non-empty text. However, if the user types fast, CC will not send the updates for individual word. Moreover, if the user adds a word and then delete it fast, CC may not capture the word since it'd be merged away. * After this change, we change the criteria to consider whether the current text has a ComposingSpan if it's a Spannable. The logic is described in the code comment. Bug: 181906241 Test: atest CtsContentCaptureServiceTestCases Change-Id: I08d447f557f7e48ef34c5a6241c497db3ce4c5fc --- .../contentcapture/ContentCaptureEvent.java | 19 ++++- .../MainContentCaptureSession.java | 80 ++++++++++++++----- .../ContentCaptureEventTest.java | 11 ++- 3 files changed, 85 insertions(+), 25 deletions(-) diff --git a/core/java/android/view/contentcapture/ContentCaptureEvent.java b/core/java/android/view/contentcapture/ContentCaptureEvent.java index 2b12230510bf7..ce014693c4c44 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 5ca793e3c394a..f196f75861ec9 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; /** @@ -146,6 +150,12 @@ public final class MainContentCaptureSession extends ContentCaptureSession { @Nullable 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 mLastComposingSpan = new ArrayMap<>(); + /** * Binder object used to update the session state. */ @@ -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); + 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. - // 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())); + 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 67614bb22e4e6..e6a25d00ff109 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)); }