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

Commit 34191ff7 authored by MingWei's avatar MingWei
Browse files

Prepare ViewStructures on UiThread 

1. onProvideContentCaptureStructure should always be called from the
   UiThread to prevent concurrrent issue. Therefore,
   notifyContentCaptureEvents needs to prepare ViewStructures on the
   UiThread first before sending event out.
2. Fork MainContentCaptureSessionV2 to hold the logic of running parts
   of the operations in the background thread. And let
   ContentCaptureManager assign different implementation dependes on the
   flag. This should reduce the potential impact on the control arm.

Test: CtsContentCaptureServiceTestCases, MainContentCaptureSessionTest.
BUG: 309411951

Change-Id: I6efee6e94e2b9857b2ad93036aa28dcd5d3a263c
parent 78478fcb

core/java/android/view/contentcapture/ChildContentCaptureSession.java

+0 −3
Original line number Diff line number Diff line
@@ -47,9 +47,6 @@ final class ChildContentCaptureSession extends ContentCaptureSession { 


    @Override

    ContentCaptureSession getMainCaptureSession() {

        if (mParent instanceof MainContentCaptureSession) {

            return (MainContentCaptureSession) mParent;

        }

        return mParent.getMainCaptureSession();

    }

core/java/android/view/contentcapture/ContentCaptureManager.java

+34 −10
Original line number Diff line number Diff line
@@ -499,7 +499,11 @@ public final class ContentCaptureManager { 


    @Nullable

    @GuardedBy("mLock")

    private Handler mHandler;

    private Handler mUiHandler;



    @Nullable

    @GuardedBy("mLock")

    private Handler mContentCaptureHandler;



    @GuardedBy("mLock")

    private ContentCaptureSession mMainSession;
@@ -590,8 +594,7 @@ public final class ContentCaptureManager { 
    public ContentCaptureSession getMainContentCaptureSession() {

        synchronized (mLock) {

            if (mMainSession == null) {

                mMainSession = new MainContentCaptureSession(

                        mContext, this, prepareContentCaptureHandler(), mService);

                mMainSession = prepareMainSession();

                if (sVerbose) Log.v(TAG, "getMainContentCaptureSession(): created " + mMainSession);

            }

            return mMainSession;
@@ -600,15 +603,36 @@ public final class ContentCaptureManager { 


    @NonNull

    @GuardedBy("mLock")

    private Handler prepareContentCaptureHandler() {

        if (mHandler == null) {

    private ContentCaptureSession prepareMainSession() {

        if (runOnBackgroundThreadEnabled()) {

                mHandler = BackgroundThread.getHandler();

            return new MainContentCaptureSessionV2(

                    mContext,

                    this,

                    prepareUiHandler(),

                    prepareContentCaptureHandler(),

                    mService

            );

        } else {

                mHandler = Handler.createAsync(Looper.getMainLooper());

            return new MainContentCaptureSession(mContext, this, prepareUiHandler(), mService);

        }

    }



    @NonNull

    @GuardedBy("mLock")

    private Handler prepareContentCaptureHandler() {

        if (mContentCaptureHandler == null) {

            mContentCaptureHandler = BackgroundThread.getHandler();

        }

        return mContentCaptureHandler;

    }



    @NonNull

    @GuardedBy("mLock")

    private Handler prepareUiHandler() {

        if (mUiHandler == null) {

            mUiHandler = Handler.createAsync(Looper.getMainLooper());

        }

        return mHandler;

        return mUiHandler;

    }



    /** @hide */

core/java/android/view/contentcapture/MainContentCaptureSession.java

+35 −146
Original line number Diff line number Diff line
@@ -31,7 +31,6 @@ import static android.view.contentcapture.ContentCaptureHelper.getSanitizedStrin 
import static android.view.contentcapture.ContentCaptureHelper.sDebug;

import static android.view.contentcapture.ContentCaptureHelper.sVerbose;

import static android.view.contentcapture.ContentCaptureManager.RESULT_CODE_FALSE;

import static android.view.contentcapture.flags.Flags.runOnBackgroundThreadEnabled;



import android.annotation.NonNull;

import android.annotation.Nullable;
@@ -70,10 +69,10 @@ import java.util.ArrayList; 
import java.util.Collections;

import java.util.List;

import java.util.NoSuchElementException;

import java.util.concurrent.ConcurrentLinkedQueue;

import java.util.concurrent.atomic.AtomicBoolean;

import java.util.concurrent.atomic.AtomicInteger;



// TODO(b/309411951): Replace V2 as the only main session once the experiment is done.

/**

 * Main session associated with a context.

 *
@@ -142,15 +141,6 @@ public final class MainContentCaptureSession extends ContentCaptureSession { 
    @Nullable

    public ComponentName mComponentName;



    /**

     * Thread-safe queue of events held to be processed as a batch.

     *

     * Because it is not guaranteed that the events will be enqueued from a single thread, the

     * implementation must be thread-safe to prevent unexpected behaviour.

     */

    @NonNull

    private final ConcurrentLinkedQueue<ContentCaptureEvent> mEventProcessQueue;



    /**

     * List of events held to be sent to the {@link ContentCaptureService} as a batch.

     *
@@ -210,14 +200,14 @@ public final class MainContentCaptureSession extends ContentCaptureSession { 
                binder = resultData.getBinder(EXTRA_BINDER);

                if (binder == null) {

                    Log.wtf(TAG, "No " + EXTRA_BINDER + " extra result");

                    mainSession.runOnContentCaptureThread(() -> mainSession.resetSession(

                    mainSession.mHandler.post(() -> mainSession.resetSession(

                            STATE_DISABLED | STATE_INTERNAL_ERROR));

                    return;

                }

            } else {

                binder = null;

            }

            mainSession.runOnContentCaptureThread(() ->

            mainSession.mHandler.post(() ->

                    mainSession.onSessionStarted(resultCode, binder));

        }

    }
@@ -238,8 +228,6 @@ public final class MainContentCaptureSession extends ContentCaptureSession { 
        mFlushHistory = logHistorySize > 0 ? new LocalLog(logHistorySize) : null;



        mSessionStateReceiver = new SessionStateReceiver(this);



        mEventProcessQueue = new ConcurrentLinkedQueue<>();

    }



    @Override
@@ -260,18 +248,7 @@ public final class MainContentCaptureSession extends ContentCaptureSession { 
    @Override

    void start(@NonNull IBinder token, @NonNull IBinder shareableActivityToken,

            @NonNull ComponentName component, int flags) {

        if (runOnBackgroundThreadEnabled()) {

            runOnContentCaptureThread(

                    () -> startImpl(token, shareableActivityToken, component, flags));

        } else {

            // Preserve the control arm behaviour.

            startImpl(token, shareableActivityToken, component, flags);

        }

    }



    private void startImpl(@NonNull IBinder token, @NonNull IBinder shareableActivityToken,

               @NonNull ComponentName component, int flags) {

        checkOnContentCaptureThread();

        checkOnUiThread();

        if (!isContentCaptureEnabled()) return;



        if (sVerbose) {
@@ -305,15 +282,17 @@ public final class MainContentCaptureSession extends ContentCaptureSession { 
            Log.w(TAG, "Error starting session for " + component.flattenToShortString() + ": " + e);

        }

    }



    @Override

    void onDestroy() {

        clearAndRunOnContentCaptureThread(() -> {

        mHandler.removeMessages(MSG_FLUSH);

        mHandler.post(() -> {

            try {

                flush(FLUSH_REASON_SESSION_FINISHED);

            } finally {

                destroySession();

            }

        }, MSG_FLUSH);

        });

    }



    /**
@@ -326,7 +305,7 @@ public final class MainContentCaptureSession extends ContentCaptureSession { 
     */

    @VisibleForTesting(visibility = VisibleForTesting.Visibility.PRIVATE)

    public void onSessionStarted(int resultCode, @Nullable IBinder binder) {

        checkOnContentCaptureThread();

        checkOnUiThread();

        if (binder != null) {

            mDirectServiceInterface = IContentCaptureDirectManager.Stub.asInterface(binder);

            mDirectServiceVulture = () -> {
@@ -375,7 +354,7 @@ public final class MainContentCaptureSession extends ContentCaptureSession { 
    }



    private void sendEvent(@NonNull ContentCaptureEvent event, boolean forceFlush) {

        checkOnContentCaptureThread();

        checkOnUiThread();

        final int eventType = event.getType();

        if (sVerbose) Log.v(TAG, "handleSendEvent(" + getDebugState() + "): " + event);

        if (!hasStarted() && eventType != ContentCaptureEvent.TYPE_SESSION_STARTED
@@ -419,14 +398,14 @@ public final class MainContentCaptureSession extends ContentCaptureSession { 
    }



    private void sendContentProtectionEvent(@NonNull ContentCaptureEvent event) {

        checkOnContentCaptureThread();

        checkOnUiThread();

        if (mContentProtectionEventProcessor != null) {

            mContentProtectionEventProcessor.processEvent(event);

        }

    }



    private void sendContentCaptureEvent(@NonNull ContentCaptureEvent event, boolean forceFlush) {

        checkOnContentCaptureThread();

        checkOnUiThread();

        final int eventType = event.getType();

        final int maxBufferSize = mManager.mOptions.maxBufferSize;

        if (mEvents == null) {
@@ -561,12 +540,12 @@ public final class MainContentCaptureSession extends ContentCaptureSession { 
    }



    private boolean hasStarted() {

        checkOnContentCaptureThread();

        checkOnUiThread();

        return mState != UNKNOWN_STATE;

    }



    private void scheduleFlush(@FlushReason int reason, boolean checkExisting) {

        checkOnContentCaptureThread();

        checkOnUiThread();

        if (sVerbose) {

            Log.v(TAG, "handleScheduleFlush(" + getDebugState(reason)

                    + ", checkExisting=" + checkExisting);
@@ -612,7 +591,7 @@ public final class MainContentCaptureSession extends ContentCaptureSession { 
    }



    private void flushIfNeeded(@FlushReason int reason) {

        checkOnContentCaptureThread();

        checkOnUiThread();

        if (mEvents == null || mEvents.isEmpty()) {

            if (sVerbose) Log.v(TAG, "Nothing to flush");

            return;
@@ -624,16 +603,7 @@ public final class MainContentCaptureSession extends ContentCaptureSession { 
    @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE)

    @Override

    public void flush(@FlushReason int reason) {

        if (runOnBackgroundThreadEnabled()) {

            runOnContentCaptureThread(() -> flushImpl(reason));

        } else {

            // Preserve the control arm behaviour.

            flushImpl(reason);

        }

    }



    private void flushImpl(@FlushReason int reason) {

        checkOnContentCaptureThread();

        checkOnUiThread();

        if (mEvents == null || mEvents.size() == 0) {

            if (sVerbose) {

                Log.v(TAG, "Don't flush for empty event buffer.");
@@ -702,7 +672,7 @@ public final class MainContentCaptureSession extends ContentCaptureSession { 
     */

    @NonNull

    private ParceledListSlice<ContentCaptureEvent> clearEvents() {

        checkOnContentCaptureThread();

        checkOnUiThread();

        // NOTE: we must save a reference to the current mEvents and then set it to to null,

        // otherwise clearing it would clear it in the receiving side if the service is also local.

        if (mEvents == null) {
@@ -717,7 +687,7 @@ public final class MainContentCaptureSession extends ContentCaptureSession { 
    /** hide */

    @VisibleForTesting(visibility = VisibleForTesting.Visibility.PRIVATE)

    public void destroySession() {

        checkOnContentCaptureThread();

        checkOnUiThread();

        if (sDebug) {

            Log.d(TAG, "Destroying session (ctx=" + mContext + ", id=" + mId + ") with "

                    + (mEvents == null ? 0 : mEvents.size()) + " event(s) for "
@@ -737,9 +707,6 @@ public final class MainContentCaptureSession extends ContentCaptureSession { 
        }

        mDirectServiceInterface = null;

        mContentProtectionEventProcessor = null;

        if (runOnBackgroundThreadEnabled()) {

            mEventProcessQueue.clear();

        }

    }



    // TODO(b/122454205): once we support multiple sessions, we might need to move some of these
@@ -747,7 +714,7 @@ public final class MainContentCaptureSession extends ContentCaptureSession { 
    /** @hide */

    @VisibleForTesting(visibility = VisibleForTesting.Visibility.PRIVATE)

    public void resetSession(int newState) {

        checkOnContentCaptureThread();

        checkOnUiThread();

        if (sVerbose) {

            Log.v(TAG, "handleResetSession(" + getActivityName() + "): from "

                    + getStateAsString(mState) + " to " + getStateAsString(newState));
@@ -775,14 +742,14 @@ public final class MainContentCaptureSession extends ContentCaptureSession { 
    void internalNotifyViewAppeared(int sessionId, @NonNull ViewStructureImpl node) {

        final ContentCaptureEvent event = new ContentCaptureEvent(sessionId, TYPE_VIEW_APPEARED)

                .setViewNode(node.mNode);

        enqueueEvent(event);

        mHandler.post(() -> sendEvent(event));

    }



    @Override

    void internalNotifyViewDisappeared(int sessionId, @NonNull AutofillId id) {

        final ContentCaptureEvent event = new ContentCaptureEvent(sessionId, TYPE_VIEW_DISAPPEARED)

                .setAutofillId(id);

        enqueueEvent(event);

        mHandler.post(() -> sendEvent(event));

    }



    @Override
@@ -813,7 +780,7 @@ public final class MainContentCaptureSession extends ContentCaptureSession { 
                .setAutofillId(id).setText(eventText)

                .setComposingIndex(composingStart, composingEnd)

                .setSelectionIndex(startIndex, endIndex);

        enqueueEvent(event);

        mHandler.post(() -> sendEvent(event));

    }



    @Override
@@ -821,7 +788,7 @@ public final class MainContentCaptureSession extends ContentCaptureSession { 
        final ContentCaptureEvent event =

                new ContentCaptureEvent(sessionId, TYPE_VIEW_INSETS_CHANGED)

                        .setInsets(viewInsets);

        enqueueEvent(event);

        mHandler.post(() -> sendEvent(event));

    }



    @Override
@@ -831,19 +798,19 @@ public final class MainContentCaptureSession extends ContentCaptureSession { 
        final boolean forceFlush = disableFlush ? !started : FORCE_FLUSH;



        final ContentCaptureEvent event = new ContentCaptureEvent(sessionId, type);

        enqueueEvent(event, forceFlush);

        mHandler.post(() -> sendEvent(event, FORCE_FLUSH));

    }



    @Override

    public void internalNotifySessionResumed() {

        final ContentCaptureEvent event = new ContentCaptureEvent(mId, TYPE_SESSION_RESUMED);

        enqueueEvent(event, FORCE_FLUSH);

        mHandler.post(() -> sendEvent(event, FORCE_FLUSH));

    }



    @Override

    public void internalNotifySessionPaused() {

        final ContentCaptureEvent event = new ContentCaptureEvent(mId, TYPE_SESSION_PAUSED);

        enqueueEvent(event, FORCE_FLUSH);

        mHandler.post(() -> sendEvent(event, FORCE_FLUSH));

    }



    @Override
@@ -868,7 +835,7 @@ public final class MainContentCaptureSession extends ContentCaptureSession { 
                new ContentCaptureEvent(childSessionId, TYPE_SESSION_STARTED)

                        .setParentSessionId(parentSessionId)

                        .setClientContext(clientContext);

        enqueueEvent(event, FORCE_FLUSH);

        mHandler.post(() -> sendEvent(event, FORCE_FLUSH));

    }



    @Override
@@ -876,14 +843,14 @@ public final class MainContentCaptureSession extends ContentCaptureSession { 
        final ContentCaptureEvent event =

                new ContentCaptureEvent(childSessionId, TYPE_SESSION_FINISHED)

                        .setParentSessionId(parentSessionId);

        enqueueEvent(event, FORCE_FLUSH);

        mHandler.post(() -> sendEvent(event, FORCE_FLUSH));

    }



    @Override

    void internalNotifyContextUpdated(int sessionId, @Nullable ContentCaptureContext context) {

        final ContentCaptureEvent event = new ContentCaptureEvent(sessionId, TYPE_CONTEXT_UPDATED)

                .setClientContext(context);

        enqueueEvent(event, FORCE_FLUSH);

        mHandler.post(() -> sendEvent(event, FORCE_FLUSH));

    }



    @Override
@@ -891,62 +858,18 @@ public final class MainContentCaptureSession extends ContentCaptureSession { 
        final ContentCaptureEvent event =

                new ContentCaptureEvent(sessionId, TYPE_WINDOW_BOUNDS_CHANGED)

                        .setBounds(bounds);

        enqueueEvent(event);

    }



    private List<ContentCaptureEvent> clearBufferEvents() {

        final ArrayList<ContentCaptureEvent> bufferEvents = new ArrayList<>();

        ContentCaptureEvent event;

        while ((event = mEventProcessQueue.poll()) != null) {

            bufferEvents.add(event);

        }

        return bufferEvents;

    }



    private void enqueueEvent(@NonNull final ContentCaptureEvent event) {

        enqueueEvent(event, /* forceFlush */ false);

    }



    /**

     * Enqueue the event into {@code mEventProcessBuffer} if it is not an urgent request. Otherwise,

     * clear the buffer events then starting sending out current event.

     */

    private void enqueueEvent(@NonNull final ContentCaptureEvent event, boolean forceFlush) {

        if (runOnBackgroundThreadEnabled()) {

            if (forceFlush) {

                // The buffer events are cleared in the same thread first to prevent new events

                // being added during the time of context switch. This would disrupt the sequence

                // of events.

                final List<ContentCaptureEvent> batchEvents = clearBufferEvents();

                runOnContentCaptureThread(() -> {

                    for (int i = 0; i < batchEvents.size(); i++) {

                        sendEvent(batchEvents.get(i));

                    }

                    sendEvent(event, /* forceFlush= */ true);

                });

            } else {

                mEventProcessQueue.offer(event);

            }

        } else {

            mHandler.post(() -> sendEvent(event, forceFlush));

        }

        mHandler.post(() -> sendEvent(event));

    }



    @Override

    public void notifyContentCaptureEvents(

            @NonNull SparseArray<ArrayList<Object>> contentCaptureEvents) {

        if (runOnBackgroundThreadEnabled()) {

            runOnContentCaptureThread(() ->

                    notifyContentCaptureEventsImpl(contentCaptureEvents));

        } else {

            // Preserve the control arm behaviour.

        notifyContentCaptureEventsImpl(contentCaptureEvents);

    }

    }



    private void notifyContentCaptureEventsImpl(

            @NonNull SparseArray<ArrayList<Object>> contentCaptureEvents) {

        checkOnContentCaptureThread();

        checkOnUiThread();

        try {

            if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) {

                Trace.traceBegin(Trace.TRACE_TAG_VIEW, "notifyContentCaptureEvents");
@@ -1092,9 +1015,9 @@ public final class MainContentCaptureSession extends ContentCaptureSession { 
     * Therefore, accessing internal properties in {@link MainContentCaptureSession} should

     * always delegate to the assigned thread from {@code mHandler} for synchronization.</p>

     */

    private void checkOnContentCaptureThread() {

        final boolean onContentCaptureThread = mHandler.getLooper().isCurrentThread();

        if (!onContentCaptureThread) {

    private void checkOnUiThread() {

        final boolean onUiThread = mHandler.getLooper().isCurrentThread();

        if (!onUiThread) {

            mWrongThreadCount.incrementAndGet();

            Log.e(TAG, "MainContentCaptureSession running on " + Thread.currentThread());

        }
@@ -1105,38 +1028,4 @@ public final class MainContentCaptureSession extends ContentCaptureSession { 
        Counter.logIncrement(

                CONTENT_CAPTURE_WRONG_THREAD_METRIC_ID, mWrongThreadCount.getAndSet(0));

    }



    /**

     * Ensures that {@code r} will be running on the assigned thread.

     *

     * <p>This is to prevent unnecessary delegation to Handler that results in fragmented runnable.

     * </p>

     */

    private void runOnContentCaptureThread(@NonNull Runnable r) {

        if (runOnBackgroundThreadEnabled()) {

            if (!mHandler.getLooper().isCurrentThread()) {

                mHandler.post(r);

            } else {

                r.run();

            }

        } else {

            // Preserve the control arm behaviour to always post to the handler.

            mHandler.post(r);

        }

    }



    private void clearAndRunOnContentCaptureThread(@NonNull Runnable r, int what) {

        if (runOnBackgroundThreadEnabled()) {

            if (!mHandler.getLooper().isCurrentThread()) {

                mHandler.removeMessages(what);

                mHandler.post(r);

            } else {

                r.run();

            }

        } else {

            // Preserve the control arm behaviour to always post to the handler.

            mHandler.removeMessages(what);

            mHandler.post(r);

        }

    }

}

core/java/android/view/contentcapture/MainContentCaptureSessionV2.java

0 → 100644
+1184 −0

File added.

Preview size limit exceeded, changes collapsed.

core/tests/coretests/src/android/view/contentcapture/MainContentCaptureSessionV2Test.java

0 → 100644
+530 −0

File added.

Preview size limit exceeded, changes collapsed.