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

Commit d58c1eaa authored by Felipe Leme's avatar Felipe Leme
Browse files

Improved how Content Capture events are flushed when activity is resumed / paused.

We were flushing right after the activity resumed, but the relevant events (views added / removed)
were not generated yet, which made such flushes useless.

This CL changes the workflow to flush them after the ViewRoot finishes doing its work.

Test: atest CtsContentCaptureServiceTestCases

Bug: 125395044
Bug: 122315042

Change-Id: I05bf27069b00c285643b2d23ad6708a6ad7bc8f3
parent f4ac4e7f
Loading
Loading
Loading
Loading
+1 −18
Original line number Diff line number Diff line
@@ -127,7 +127,6 @@ import android.view.autofill.AutofillPopupWindow;
import android.view.autofill.IAutofillWindowPresenter;
import android.view.contentcapture.ContentCaptureContext;
import android.view.contentcapture.ContentCaptureManager;
import android.view.contentcapture.ContentCaptureSession;
import android.widget.AdapterView;
import android.widget.Toast;
import android.widget.Toolbar;
@@ -1038,15 +1037,11 @@ public class Activity extends ContextThemeWrapper
    }

    /** @hide */ private static final int CONTENT_CAPTURE_START = 1;
    /** @hide */ private static final int CONTENT_CAPTURE_PAUSE = 2;
    /** @hide */ private static final int CONTENT_CAPTURE_RESUME = 3;
    /** @hide */ private static final int CONTENT_CAPTURE_STOP = 4;
    /** @hide */ private static final int CONTENT_CAPTURE_STOP = 2;

    /** @hide */
    @IntDef(prefix = { "CONTENT_CAPTURE_" }, value = {
            CONTENT_CAPTURE_START,
            CONTENT_CAPTURE_PAUSE,
            CONTENT_CAPTURE_RESUME,
            CONTENT_CAPTURE_STOP
    })
    @Retention(RetentionPolicy.SOURCE)
@@ -1056,10 +1051,6 @@ public class Activity extends ContextThemeWrapper
        switch (type) {
            case CONTENT_CAPTURE_START:
                return "START";
            case CONTENT_CAPTURE_PAUSE:
                return "PAUSE";
            case CONTENT_CAPTURE_RESUME:
                return "RESUME";
            case CONTENT_CAPTURE_STOP:
                return "STOP";
            default:
@@ -1088,12 +1079,6 @@ public class Activity extends ContextThemeWrapper
                    }
                    cm.onActivityStarted(mToken, getComponentName(), flags);
                    break;
                case CONTENT_CAPTURE_PAUSE:
                    cm.flush(ContentCaptureSession.FLUSH_REASON_ACTIVITY_PAUSED);
                    break;
                case CONTENT_CAPTURE_RESUME:
                    cm.flush(ContentCaptureSession.FLUSH_REASON_ACTIVITY_RESUMED);
                    break;
                case CONTENT_CAPTURE_STOP:
                    cm.onActivityStopped();
                    break;
@@ -1781,7 +1766,6 @@ public class Activity extends ContextThemeWrapper
            }
        }
        mCalled = true;
        notifyContentCaptureManagerIfNeeded(CONTENT_CAPTURE_RESUME);
    }

    /**
@@ -2198,7 +2182,6 @@ public class Activity extends ContextThemeWrapper
            }
        }
        mCalled = true;
        notifyContentCaptureManagerIfNeeded(CONTENT_CAPTURE_PAUSE);
    }

    /**
+9 −3
Original line number Diff line number Diff line
@@ -9408,7 +9408,7 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
            }
            setNotifiedContentCaptureAppeared();
            // TODO(b/123307965): instead of post, we should queue it on AttachInfo and then
            // TODO(b/125395044): instead of post, we should queue it on AttachInfo and then
            // dispatch on RootImpl, as we're doing with the removed ones (in that case, we should
            // merge the delayNotifyContentCaptureDisappeared() into a more generic method that
            // takes a session and a command, where the command is either view added or removed
@@ -9703,13 +9703,19 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
     *
     * @hide
     */
    public void dispatchInitialProvideContentCaptureStructure(@NonNull ContentCaptureManager ccm) {
    public void dispatchInitialProvideContentCaptureStructure() {
        AttachInfo ai = mAttachInfo;
        if (ai == null) {
            Log.w(CONTENT_CAPTURE_LOG_TAG,
                    "dispatchProvideContentCaptureStructure(): no AttachInfo for " + this);
            return;
        }
        ContentCaptureManager ccm = ai.mContentCaptureManager;
        if (ccm == null) {
            Log.w(CONTENT_CAPTURE_LOG_TAG, "dispatchProvideContentCaptureStructure(): "
                    + "no ContentCaptureManager for " + this);
            return;
        }
        // We must set it before checkign if the view itself is important, because it might
        // initially not be (for example, if it's empty), although that might change later (for
@@ -28405,7 +28411,7 @@ public class View implements Drawable.Callback, KeyEvent.Callback,
        }
        @Nullable
        private ContentCaptureManager getContentCaptureManager(@NonNull Context context) {
        ContentCaptureManager getContentCaptureManager(@NonNull Context context) {
            if (mContentCaptureManager != null) {
                return mContentCaptureManager;
            }
+97 −12
Original line number Diff line number Diff line
@@ -108,6 +108,7 @@ import android.view.animation.Interpolator;
import android.view.autofill.AutofillId;
import android.view.autofill.AutofillManager;
import android.view.contentcapture.ContentCaptureManager;
import android.view.contentcapture.ContentCaptureSession;
import android.view.contentcapture.MainContentCaptureSession;
import android.view.inputmethod.InputMethodManager;
import android.widget.Scroller;
@@ -223,10 +224,25 @@ public final class ViewRootImpl implements ViewParent,
     */
    static final int MAX_TRACKBALL_DELAY = 250;

    /**
     * Initial value for {@link #mContentCaptureEnabled}.
     */
    private static final int CONTENT_CAPTURE_ENABLED_NOT_CHECKED = 0;

    /**
     * Value for {@link #mContentCaptureEnabled} when it was checked and set to {@code true}.
     */
    private static final int CONTENT_CAPTURE_ENABLED_TRUE = 1;

    /**
     * Value for {@link #mContentCaptureEnabled} when it was checked and set to {@code false}.
     */
    private static final int CONTENT_CAPTURE_ENABLED_FALSE = 2;

    @UnsupportedAppUsage
    static final ThreadLocal<HandlerActionQueue> sRunQueues = new ThreadLocal<HandlerActionQueue>();

    static final ArrayList<Runnable> sFirstDrawHandlers = new ArrayList();
    static final ArrayList<Runnable> sFirstDrawHandlers = new ArrayList<>();
    static boolean sFirstDrawComplete = false;

    /**
@@ -418,7 +434,11 @@ public final class ViewRootImpl implements ViewParent,
    boolean mApplyInsetsRequested;
    boolean mLayoutRequested;
    boolean mFirst;

    @Nullable
    int mContentCaptureEnabled = CONTENT_CAPTURE_ENABLED_NOT_CHECKED;
    boolean mPerformContentCapture;

    boolean mReportNextDraw;
    boolean mFullRedrawNeeded;
    boolean mNewSurfaceNeeded;
@@ -2776,6 +2796,7 @@ public final class ViewRootImpl implements ViewParent,
            }
        }

        // TODO(b/125395044): might need to check for added events too and flush them
        if (mAttachInfo.mContentCaptureRemovedIds != null) {
            MainContentCaptureSession mainSession = mAttachInfo.mContentCaptureManager
                    .getMainContentCaptureSession();
@@ -2789,6 +2810,8 @@ public final class ViewRootImpl implements ViewParent,
                    mainSession.notifyViewsDisappeared(sessionId, ids);
                }
                mAttachInfo.mContentCaptureRemovedIds = null;
                mAttachInfo.mContentCaptureManager
                        .flush(ContentCaptureSession.FLUSH_REASON_POST_VIEW_ROOT_TRAVERSAL);
            } finally {
                Trace.traceEnd(Trace.TRACE_TAG_VIEW);
            }
@@ -2927,6 +2950,14 @@ public final class ViewRootImpl implements ViewParent,
            }
        }
        mFirstInputStage.onWindowFocusChanged(hasWindowFocus);
        // TODO(b/125395044): right now the list of events is always empty on
        // when hasWindowFocus is false, as the removed events are effectively flushed on
        // FLUSH_REASON_POST_VIEW_ROOT_TRAVERSAL. If after the final refactorings that's still the
        // case, we should add another reason for FLUSH_REASON_VIEW_ROOT_EXITED
        if (hasWindowFocus) {
            performContentCaptureFlushIfNecessary(
                    ContentCaptureSession.FLUSH_REASON_VIEW_ROOT_ENTERED);
        }
    }

    private void fireAccessibilityFocusEventIfHasFocusedNode() {
@@ -3494,36 +3525,90 @@ public final class ViewRootImpl implements ViewParent,
            }
        }
        if (mPerformContentCapture) {
            performContentCapture();
            performContentCaptureInitialReport();
        }
    }

    private void performContentCapture() {
    /**
     * Checks (and caches) if content capture is enabled for this context.
     */
    private boolean isContentCaptureEnabled() {
        switch (mContentCaptureEnabled) {
            case CONTENT_CAPTURE_ENABLED_TRUE:
                return true;
            case CONTENT_CAPTURE_ENABLED_FALSE:
                return false;
            case CONTENT_CAPTURE_ENABLED_NOT_CHECKED:
                final boolean reallyEnabled = isContentCaptureReallyEnabled();
                mContentCaptureEnabled = reallyEnabled ? CONTENT_CAPTURE_ENABLED_TRUE
                        : CONTENT_CAPTURE_ENABLED_FALSE;
                return reallyEnabled;
            default:
                Log.w(TAG, "isContentCaptureEnabled(): invalid state " + mContentCaptureEnabled);
                return false;
        }

    }

    /**
     * Checks (without caching) if content capture is enabled for this context.
     */
    private boolean isContentCaptureReallyEnabled() {
        // First check if context supports it, so it saves a service lookup when it doesn't
        if (mContext.getContentCaptureOptions() == null) return false;

        final ContentCaptureManager ccm = mAttachInfo.getContentCaptureManager(mContext);
        // Then check if it's enabled in the contex itself.
        if (ccm == null || !ccm.isContentCaptureEnabled()) return false;

        return true;
    }

    private void performContentCaptureInitialReport() {
        mPerformContentCapture = false; // One-time offer!
        final View rootView = mView;
        if (DEBUG_CONTENT_CAPTURE) {
            Log.v(mTag, "dispatchContentCapture() on " + rootView);
            Log.v(mTag, "performContentCaptureInitialReport() on " + rootView);
        }
        if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) {
            Trace.traceBegin(Trace.TRACE_TAG_VIEW, "dispatchContentCapture() for "
                    + getClass().getSimpleName());
        }
        try {
            // First check if context supports it, so it saves a service lookup when it doesn't
            if (mContext.getContentCaptureOptions() == null) return;

            // Then check if it's enabled in the contex itself.
            final ContentCaptureManager ccm = mContext
                    .getSystemService(ContentCaptureManager.class);
            if (ccm == null || !ccm.isContentCaptureEnabled()) return;
            if (!isContentCaptureEnabled()) return;

            // Content capture is a go!
            rootView.dispatchInitialProvideContentCaptureStructure(ccm);
            rootView.dispatchInitialProvideContentCaptureStructure();
        } finally {
            Trace.traceEnd(Trace.TRACE_TAG_VIEW);
        }
    }

    private void performContentCaptureFlushIfNecessary(
            @ContentCaptureSession.FlushReason int flushReason) {
        if (DEBUG_CONTENT_CAPTURE) {
            Log.v(mTag, "performContentCaptureFlushIfNecessary("
                    + ContentCaptureSession.getFlushReasonAsString(flushReason) + ")");
        }
        if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) {
            Trace.traceBegin(Trace.TRACE_TAG_VIEW, "flushContentCapture for "
                    + getClass().getSimpleName());
        }
        try {
            if (!isContentCaptureEnabled()) return;

            final ContentCaptureManager ccm = mAttachInfo.mContentCaptureManager;
            if (ccm == null) {
                Log.w(TAG, "flush content capture: no ContentCapture on AttachInfo");
                return;
            }
            ccm.flush(flushReason);
        } finally {
            Trace.traceEnd(Trace.TRACE_TAG_VIEW);
        }
    }


    private boolean draw(boolean fullRedrawNeeded) {
        Surface surface = mSurface;
        if (!surface.isValid()) {
+11 −11
Original line number Diff line number Diff line
@@ -132,9 +132,9 @@ public abstract class ContentCaptureSession implements AutoCloseable {
    /** @hide */
    public static final int FLUSH_REASON_FULL = 1;
    /** @hide */
    public static final int FLUSH_REASON_ACTIVITY_PAUSED = 2;
    public static final int FLUSH_REASON_VIEW_ROOT_ENTERED = 2;
    /** @hide */
    public static final int FLUSH_REASON_ACTIVITY_RESUMED = 3;
    public static final int FLUSH_REASON_POST_VIEW_ROOT_TRAVERSAL = 3;
    /** @hide */
    public static final int FLUSH_REASON_SESSION_STARTED = 4;
    /** @hide */
@@ -145,14 +145,14 @@ public abstract class ContentCaptureSession implements AutoCloseable {
    /** @hide */
    @IntDef(prefix = { "FLUSH_REASON_" }, value = {
            FLUSH_REASON_FULL,
            FLUSH_REASON_ACTIVITY_PAUSED,
            FLUSH_REASON_ACTIVITY_RESUMED,
            FLUSH_REASON_SESSION_STARTED,
            FLUSH_REASON_SESSION_FINISHED,
            FLUSH_REASON_IDLE_TIMEOUT
            FLUSH_REASON_IDLE_TIMEOUT,
            FLUSH_REASON_VIEW_ROOT_ENTERED,
            FLUSH_REASON_POST_VIEW_ROOT_TRAVERSAL
    })
    @Retention(RetentionPolicy.SOURCE)
    @interface FlushReason{}
    public @interface FlushReason{}

    private final Object mLock = new Object();

@@ -500,20 +500,20 @@ public abstract class ContentCaptureSession implements AutoCloseable {

    /** @hide */
    @NonNull
    static String getflushReasonAsString(@FlushReason int reason) {
    public static String getFlushReasonAsString(@FlushReason int reason) {
        switch (reason) {
            case FLUSH_REASON_FULL:
                return "FULL";
            case FLUSH_REASON_ACTIVITY_PAUSED:
                return "PAUSED";
            case FLUSH_REASON_ACTIVITY_RESUMED:
                return "RESUMED";
            case FLUSH_REASON_SESSION_STARTED:
                return "STARTED";
            case FLUSH_REASON_SESSION_FINISHED:
                return "FINISHED";
            case FLUSH_REASON_IDLE_TIMEOUT:
                return "IDLE";
            case FLUSH_REASON_VIEW_ROOT_ENTERED:
                return "ENTERED";
            case FLUSH_REASON_POST_VIEW_ROOT_TRAVERSAL:
                return "TRAVERSAL";
            default:
                return "UNKOWN-" + reason;
        }
+2 −2
Original line number Diff line number Diff line
@@ -451,7 +451,7 @@ public final class MainContentCaptureSession extends ContentCaptureSession {
        }

        final int numberEvents = mEvents.size();
        final String reasonString = getflushReasonAsString(reason);
        final String reasonString = getFlushReasonAsString(reason);
        if (sDebug) {
            Log.d(TAG, "Flushing " + numberEvents + " event(s) for " + getDebugState(reason));
        }
@@ -684,6 +684,6 @@ public final class MainContentCaptureSession extends ContentCaptureSession {

    @NonNull
    private String getDebugState(@FlushReason int reason) {
        return getDebugState() + ", reason=" + getflushReasonAsString(reason);
        return getDebugState() + ", reason=" + getFlushReasonAsString(reason);
    }
}