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

Commit 3b02520e authored by Nicolò Mazzucato's avatar Nicolò Mazzucato Committed by Android (Google) Code Review
Browse files

Merge "Fix CUJ UiThread detection when only a SurfaceControl is provided" into main

parents 66149b55 da21dd7f
Loading
Loading
Loading
Loading
+2 −1
Original line number Diff line number Diff line
@@ -920,7 +920,8 @@ public interface ImeTracker {
            final Configuration.Builder builder = Configuration.Builder.withSurface(
                            cujType,
                            jankContext.getDisplayContext(),
                            jankContext.getTargetSurfaceControl())
                            jankContext.getTargetSurfaceControl(),
                            jankContext.getDisplayContext().getMainThreadHandler())
                    .setTag(String.format(Locale.US, "%d@%d@%s", animType,
                            useSeparatedThread ? 0 : 1, jankContext.getHostPackageName()));
            InteractionJankMonitor.getInstance().begin(builder);
+40 −9
Original line number Diff line number Diff line
@@ -330,9 +330,10 @@ public class InteractionJankMonitor {
     * @param cujType the specific {@link Cuj.CujType}.
     * @return boolean true if the tracker is started successfully, false otherwise.
     */
    public boolean begin(SurfaceControl surface, Context context, @Cuj.CujType int cujType) {
    public boolean begin(SurfaceControl surface, Context context, Handler handler,
            @Cuj.CujType int cujType) {
        try {
            return begin(Configuration.Builder.withSurface(cujType, context, surface));
            return begin(Configuration.Builder.withSurface(cujType, context, surface, handler));
        } catch (IllegalArgumentException ex) {
            Log.d(TAG, "Build configuration failed!", ex);
            return false;
@@ -348,11 +349,12 @@ public class InteractionJankMonitor {
     * @param tag a tag containing extra information about the interaction.
     * @return boolean true if the tracker is started successfully, false otherwise.
     */
    public boolean begin(SurfaceControl surface, Context context, @Cuj.CujType int cujType,
    public boolean begin(SurfaceControl surface, Context context, Handler handler,
            @Cuj.CujType int cujType,
            String tag) {
        try {
            final Configuration.Builder builder =
                    Configuration.Builder.withSurface(cujType, context, surface);
                    Configuration.Builder.withSurface(cujType, context, surface, handler);
            if (!TextUtils.isEmpty(tag)) {
                builder.setTag(tag);
            }
@@ -689,20 +691,23 @@ public class InteractionJankMonitor {
            private SurfaceControl mAttrSurfaceControl;
            private final @Cuj.CujType int mAttrCujType;
            private boolean mAttrDeferMonitor = true;
            private Handler mHandler = null;

            /**
             * Creates a builder which instruments only surface.
             * @param cuj The enum defined in {@link Cuj.CujType}.
             * @param context context
             * @param surfaceControl surface control
             * @param uiThreadHandler UI thread for that surface
             * @return builder
             */
            public static Builder withSurface(@Cuj.CujType int cuj, @NonNull Context context,
                    @NonNull SurfaceControl surfaceControl) {
                    @NonNull SurfaceControl surfaceControl, @NonNull Handler uiThreadHandler) {
                return new Builder(cuj)
                        .setContext(context)
                        .setSurfaceControl(surfaceControl)
                        .setSurfaceOnly(true);
                        .setSurfaceOnly(true)
                        .setHandler(uiThreadHandler);
            }

            /**
@@ -721,6 +726,18 @@ public class InteractionJankMonitor {
                mAttrCujType = cuj;
            }

            /**
             * Specifies the UI thread handler. If not provided, the View's one will be used.
             * If only a surface is provided without handler, the app main thread will be used.
             *
             * @param uiThreadHandler handler associated to the cuj UI thread
             * @return builder
             */
            public Builder setHandler(Handler uiThreadHandler) {
                mHandler = uiThreadHandler;
                return this;
            }

            /**
             * Specifies a view, must be set if {@link #setSurfaceOnly(boolean)} is set to false.
             * @param view an attached view
@@ -798,13 +815,13 @@ public class InteractionJankMonitor {
                return new Configuration(
                        mAttrCujType, mAttrView, mAttrTag, mAttrTimeout,
                        mAttrSurfaceOnly, mAttrContext, mAttrSurfaceControl,
                        mAttrDeferMonitor);
                        mAttrDeferMonitor, mHandler);
            }
        }

        private Configuration(@Cuj.CujType int cuj, View view, @NonNull String tag, long timeout,
                boolean surfaceOnly, Context context, SurfaceControl surfaceControl,
                boolean deferMonitor) {
                boolean deferMonitor, Handler handler) {
            mCujType = cuj;
            mTag = tag;
            mSessionName = generateSessionName(Cuj.getNameOfCuj(cuj), tag);
@@ -816,8 +833,16 @@ public class InteractionJankMonitor {
                    : (view != null ? view.getContext().getApplicationContext() : null);
            mSurfaceControl = surfaceControl;
            mDeferMonitor = deferMonitor;
            if (handler != null) {
                mHandler = handler;
            } else if (mSurfaceOnly) {
                Log.w(TAG, "No UIThread provided for " + mSessionName
                        + " (surface only). Defaulting to app main thread.");
                mHandler = mContext.getMainThreadHandler();
            } else {
                mHandler = mView.getHandler();
            }
            validate();
            mHandler = mSurfaceOnly ? mContext.getMainThreadHandler() : mView.getHandler();
        }

        @VisibleForTesting
@@ -858,6 +883,12 @@ public class InteractionJankMonitor {
                    shouldThrow = true;
                    msg.append("Must pass in a valid surface control if only instrument surface; ");
                }
                if (mHandler == null) {
                    shouldThrow = true;
                    msg.append(
                            "Must pass a UI thread handler when only a surface control is "
                                    + "provided.");
                }
            } else {
                if (!hasValidView()) {
                    shouldThrow = true;
+21 −0
Original line number Diff line number Diff line
@@ -24,6 +24,7 @@ import static com.android.internal.jank.InteractionJankMonitor.Configuration.gen

import static com.google.common.truth.Truth.assertThat;

import static org.junit.Assert.assertThrows;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyLong;
@@ -117,6 +118,7 @@ public class InteractionJankMonitorTest {

        // Simulate a trace session and see if begin / end are invoked.
        assertThat(monitor.begin(mSurfaceControl, mActivity.getApplicationContext(),
                mActivity.getMainThreadHandler(),
                Cuj.CUJ_NOTIFICATION_SHADE_EXPAND_COLLAPSE)).isTrue();
        verify(tracker).begin();
        assertThat(monitor.end(Cuj.CUJ_NOTIFICATION_SHADE_EXPAND_COLLAPSE)).isTrue();
@@ -188,6 +190,25 @@ public class InteractionJankMonitorTest {
        assertThat(generateSessionName(cujName, tooLongTag)).isEqualTo(expectedTrimmedName);
    }

    @Test
    public void validateConfiguration_surfaceOnlyAndNotDeferMonitor_throwsError() {
        Configuration.Builder builder = Configuration.Builder.withSurface(1,
                mActivity.getApplicationContext(), mSurfaceControl,
                mActivity.getMainThreadHandler()).setDeferMonitorForAnimationStart(false);

        assertThrows(IllegalStateException.class, builder::build);
    }

    @Test
    public void validateConfiguration_surfaceOnlyAndDeferMonitor_doesNotThrowError() {
        Configuration.Builder builder = Configuration.Builder.withSurface(1,
                mActivity.getApplicationContext(),
                mSurfaceControl, mActivity.getMainThreadHandler()).setDeferMonitorForAnimationStart(
                true);

        builder.build(); // no exception.
    }

    private InteractionJankMonitor createMockedInteractionJankMonitor() {
        InteractionJankMonitor monitor = spy(new InteractionJankMonitor(mWorker));
        doReturn(true).when(monitor).shouldMonitor();
+11 −5
Original line number Diff line number Diff line
@@ -117,6 +117,7 @@ public class BackAnimationController implements RemoteCallable<BackAnimationCont
     */
    private static final long MAX_ANIMATION_DURATION = 2000;
    private final LatencyTracker mLatencyTracker;
    @ShellMainThread private final Handler mHandler;

    /** True when a back gesture is ongoing */
    private boolean mBackGestureStarted = false;
@@ -218,7 +219,8 @@ public class BackAnimationController implements RemoteCallable<BackAnimationCont
            @NonNull BackAnimationBackground backAnimationBackground,
            ShellBackAnimationRegistry shellBackAnimationRegistry,
            ShellCommandHandler shellCommandHandler,
            Transitions transitions) {
            Transitions transitions,
            @ShellMainThread Handler handler) {
        this(
                shellInit,
                shellController,
@@ -230,7 +232,8 @@ public class BackAnimationController implements RemoteCallable<BackAnimationCont
                backAnimationBackground,
                shellBackAnimationRegistry,
                shellCommandHandler,
                transitions);
                transitions,
                handler);
    }

    @VisibleForTesting
@@ -245,7 +248,8 @@ public class BackAnimationController implements RemoteCallable<BackAnimationCont
            @NonNull BackAnimationBackground backAnimationBackground,
            ShellBackAnimationRegistry shellBackAnimationRegistry,
            ShellCommandHandler shellCommandHandler,
            Transitions transitions) {
            Transitions transitions,
            @NonNull @ShellMainThread Handler handler) {
        mShellController = shellController;
        mShellExecutor = shellExecutor;
        mActivityTaskManager = activityTaskManager;
@@ -263,6 +267,7 @@ public class BackAnimationController implements RemoteCallable<BackAnimationCont
        mTransitions = transitions;
        mBackTransitionHandler = new BackTransitionHandler();
        mTransitions.addHandler(mBackTransitionHandler);
        mHandler = handler;
        updateTouchableArea();
    }

@@ -399,7 +404,7 @@ public class BackAnimationController implements RemoteCallable<BackAnimationCont
        }
    }

    private static class IBackAnimationImpl extends IBackAnimation.Stub
    private class IBackAnimationImpl extends IBackAnimation.Stub
            implements ExternalInterfaceBinder {
        private BackAnimationController mController;

@@ -417,7 +422,8 @@ public class BackAnimationController implements RemoteCallable<BackAnimationCont
                                    callback,
                                    runner,
                                    controller.mContext,
                                    CUJ_PREDICTIVE_BACK_HOME)));
                                    CUJ_PREDICTIVE_BACK_HOME,
                                    mHandler)));
        }

        @Override
+25 −4
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ import static android.view.WindowManager.TRANSIT_OLD_UNSET;

import android.annotation.NonNull;
import android.content.Context;
import android.os.Handler;
import android.os.RemoteException;
import android.util.Log;
import android.view.IRemoteAnimationFinishedCallback;
@@ -31,6 +32,7 @@ import android.window.IOnBackInvokedCallback;
import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.jank.Cuj.CujType;
import com.android.internal.jank.InteractionJankMonitor;
import com.android.wm.shell.shared.annotations.ShellMainThread;

/**
 * Used to register the animation callback and runner, it will trigger result if gesture was finish
@@ -45,6 +47,8 @@ public class BackAnimationRunner {
    private final IRemoteAnimationRunner mRunner;
    private final @CujType int mCujType;
    private final Context mContext;
    @ShellMainThread
    private final Handler mHandler;

    // Whether we are waiting to receive onAnimationStart
    private boolean mWaitingAnimation;
@@ -56,18 +60,35 @@ public class BackAnimationRunner {
            @NonNull IOnBackInvokedCallback callback,
            @NonNull IRemoteAnimationRunner runner,
            @NonNull Context context,
            @CujType int cujType) {
            @CujType int cujType,
            @ShellMainThread Handler handler) {
        mCallback = callback;
        mRunner = runner;
        mCujType = cujType;
        mContext = context;
        mHandler = handler;
    }

    public BackAnimationRunner(
            @NonNull IOnBackInvokedCallback callback,
            @NonNull IRemoteAnimationRunner runner,
            @NonNull Context context) {
        this(callback, runner, context, NO_CUJ);
            @NonNull Context context,
            @ShellMainThread Handler handler
    ) {
        this(callback, runner, context, NO_CUJ, handler);
    }

    /**
     * @deprecated Use {@link BackAnimationRunner} constructor providing an handler for the ui
     * thread of the animation.
     */
    @Deprecated
    public BackAnimationRunner(
            @NonNull IOnBackInvokedCallback callback,
            @NonNull IRemoteAnimationRunner runner,
            @NonNull Context context
    ) {
        this(callback, runner, context, NO_CUJ, context.getMainThreadHandler());
    }

    /** Returns the registered animation runner */
@@ -100,7 +121,7 @@ public class BackAnimationRunner {
        mWaitingAnimation = false;
        if (shouldMonitorCUJ(apps)) {
            interactionJankMonitor.begin(
                    apps[0].leash, mContext, mCujType);
                    apps[0].leash, mContext, mHandler, mCujType);
        }
        try {
            getRunner().onAnimationStart(TRANSIT_OLD_UNSET, apps, wallpapers,
Loading