Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit c8def069 authored by Feng Cao's avatar Feng Cao Committed by TYM Tsai
Browse files

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
parent 3be701e7
Loading
Loading
Loading
Loading
+16 −3
Original line number Diff line number Diff line
@@ -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());
            }
+63 −19
Original line number Diff line number Diff line
@@ -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<AutofillId, Boolean> mLastComposingSpan = new ArrayMap<>();

    /**
     * Binder object used to update the session state.
     */
@@ -335,27 +345,48 @@ 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 (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) {
                        } else if (bothNonEmpty && lastEventHasComposingSpan) {
                            lastEvent.mergeEvent(event);
                            addEvent = false;
                        }
                        if (!addEvent && sVerbose) {
                            Log.v(TAG, "Buffering VIEW_TEXT_CHANGED event, updated text="
                            + getSanitizedString(event.getText()));
                                    + getSanitizedString(text));
                        }
                    }
                }
            }
            mLastComposingSpan.put(event.getId(), textHasComposingSpan);
        }

        if (!mEvents.isEmpty() && eventType == TYPE_VIEW_DISAPPEARED) {
            final ContentCaptureEvent lastEvent = mEvents.get(mEvents.size() - 1);
@@ -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 */
+7 −4
Original line number Diff line number Diff line
@@ -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));
    }