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

Commit 6b104eec authored by Chilun's avatar Chilun
Browse files

Store pending override config instead of creating ActivityClientRecord

Create ActivityClientRecord early in preExecute may cause
NullPointerException. If two LaunchActivityItem using the same token
and the 1st postExecute() comes after 2nd preExecute(), the
corresponding launching activity will be removed and cause 2nd
execute() get NullPointerException.

Since the only use case to get ActivityClientRecord in preExecute() is
just to use it to store the pending override config. We can directly
store the latest pending override config from preExecute() in
ActivityThread instead of creating ActivityClientRecord.

Bug: 201668069
Test: atest TransactionExecutorTests
      atest ActivityThreadTest
Change-Id: If350e942254e54c9ec90bc63a6e50eb67d038183
parent 774ef9f7
Loading
Loading
Loading
Loading
+28 −41
Original line number Diff line number Diff line
@@ -243,6 +243,7 @@ import java.util.Map;
import java.util.Objects;
import java.util.TimeZone;
import java.util.concurrent.Executor;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Consumer;

/**
@@ -344,11 +345,9 @@ public final class ActivityThread extends ClientTransactionHandler
     */
    @UnsupportedAppUsage
    final ArrayMap<IBinder, ActivityClientRecord> mActivities = new ArrayMap<>();
    /**
     * Maps from activity token to local record of the activities that are preparing to be launched.
     */
    final Map<IBinder, ActivityClientRecord> mLaunchingActivities =
            Collections.synchronizedMap(new ArrayMap<IBinder, ActivityClientRecord>());
    /** Maps from activity token to the pending override configuration. */
    @GuardedBy("mPendingOverrideConfigs")
    private final ArrayMap<IBinder, Configuration> mPendingOverrideConfigs = new ArrayMap<>();
    /** The activities to be truly destroyed (not include relaunch). */
    final Map<IBinder, ClientTransactionItem> mActivitiesToBeDestroyed =
            Collections.synchronizedMap(new ArrayMap<IBinder, ClientTransactionItem>());
@@ -358,6 +357,7 @@ public final class ActivityThread extends ClientTransactionHandler
    // Number of activities that are currently visible on-screen.
    @UnsupportedAppUsage
    int mNumVisibleActivities = 0;
    private final AtomicInteger mNumLaunchingActivities = new AtomicInteger();
    @GuardedBy("mAppThread")
    private int mLastProcessState = PROCESS_STATE_UNKNOWN;
    @GuardedBy("mAppThread")
@@ -555,10 +555,6 @@ public final class ActivityThread extends ClientTransactionHandler
        boolean hideForNow;
        Configuration createdConfig;
        Configuration overrideConfig;
        // Used to save the last reported configuration from server side so that activity
        // configuration transactions can always use the latest configuration.
        @GuardedBy("this")
        private Configuration mPendingOverrideConfig;
        // Used for consolidating configs before sending on to Activity.
        private Configuration tmpConfig = new Configuration();
        // Callback used for updating activity override config and camera compat control state.
@@ -3360,21 +3356,6 @@ public final class ActivityThread extends ClientTransactionHandler
        return activityRecord != null ? activityRecord.activity : null;
    }

    @Override
    public void addLaunchingActivity(IBinder token, ActivityClientRecord activity) {
        mLaunchingActivities.put(token, activity);
    }

    @Override
    public ActivityClientRecord getLaunchingActivity(IBinder token) {
        return mLaunchingActivities.get(token);
    }

    @Override
    public void removeLaunchingActivity(IBinder token) {
        mLaunchingActivities.remove(token);
    }

    @Override
    public ActivityClientRecord getActivityClient(IBinder token) {
        return mActivities.get(token);
@@ -3419,7 +3400,7 @@ public final class ActivityThread extends ClientTransactionHandler
            // Defer the top state for VM to avoid aggressive JIT compilation affecting activity
            // launch time.
            if (processState == ActivityManager.PROCESS_STATE_TOP
                    && !mLaunchingActivities.isEmpty()) {
                    && mNumLaunchingActivities.get() > 0) {
                mPendingProcessState = processState;
                mH.postDelayed(this::applyPendingProcessState, PENDING_TOP_PROCESS_STATE_TIMEOUT);
            } else {
@@ -3435,7 +3416,7 @@ public final class ActivityThread extends ClientTransactionHandler
        // Handle the pending configuration if the process state is changed from cached to
        // non-cached. Except the case where there is a launching activity because the
        // LaunchActivityItem will handle it.
        if (wasCached && !isCachedProcessState() && mLaunchingActivities.isEmpty()) {
        if (wasCached && !isCachedProcessState() && mNumLaunchingActivities.get() == 0) {
            final Configuration pendingConfig =
                    mConfigurationController.getPendingConfiguration(false /* clearPending */);
            if (pendingConfig == null) {
@@ -3473,6 +3454,11 @@ public final class ActivityThread extends ClientTransactionHandler
        }
    }

    @Override
    public void countLaunchingActivities(int num) {
        mNumLaunchingActivities.getAndAdd(num);
    }

    @UnsupportedAppUsage
    public final void sendActivityResult(
            IBinder token, String id, int requestCode,
@@ -6096,31 +6082,31 @@ public final class ActivityThread extends ClientTransactionHandler
    }

    /**
     * Sets the supplied {@code overrideConfig} as pending for the {@code activityToken}. Calling
     * Sets the supplied {@code overrideConfig} as pending for the {@code token}. Calling
     * this method prevents any calls to
     * {@link #handleActivityConfigurationChanged(ActivityClientRecord, Configuration, int)} from
     * processing any configurations older than {@code overrideConfig}.
     */
    @Override
    public void updatePendingActivityConfiguration(ActivityClientRecord r,
            Configuration overrideConfig) {
        synchronized (r) {
            if (r.mPendingOverrideConfig != null
                    && !r.mPendingOverrideConfig.isOtherSeqNewer(overrideConfig)) {
    public void updatePendingActivityConfiguration(IBinder token, Configuration overrideConfig) {
        synchronized (mPendingOverrideConfigs) {
            final Configuration pendingOverrideConfig = mPendingOverrideConfigs.get(token);
            if (pendingOverrideConfig != null
                    && !pendingOverrideConfig.isOtherSeqNewer(overrideConfig)) {
                if (DEBUG_CONFIGURATION) {
                    Slog.v(TAG, "Activity has newer configuration pending so drop this"
                            + " transaction. overrideConfig=" + overrideConfig
                            + " r.mPendingOverrideConfig=" + r.mPendingOverrideConfig);
                    Slog.v(TAG, "Activity has newer configuration pending so this transaction will"
                            + " be dropped. overrideConfig=" + overrideConfig
                            + " pendingOverrideConfig=" + pendingOverrideConfig);
                }
                return;
            }
            r.mPendingOverrideConfig = overrideConfig;
            mPendingOverrideConfigs.put(token, overrideConfig);
        }
    }

    /**
     * Handle new activity configuration and/or move to a different display. This method is a noop
     * if {@link #updatePendingActivityConfiguration(ActivityClientRecord, Configuration)} has been
     * if {@link #updatePendingActivityConfiguration(IBinder, Configuration)} has been
     * called with a newer config than {@code overrideConfig}.
     *
     * @param r Target activity record.
@@ -6131,16 +6117,17 @@ public final class ActivityThread extends ClientTransactionHandler
    @Override
    public void handleActivityConfigurationChanged(ActivityClientRecord r,
            @NonNull Configuration overrideConfig, int displayId) {
        synchronized (r) {
            if (overrideConfig.isOtherSeqNewer(r.mPendingOverrideConfig)) {
        synchronized (mPendingOverrideConfigs) {
            final Configuration pendingOverrideConfig = mPendingOverrideConfigs.get(r.token);
            if (overrideConfig.isOtherSeqNewer(pendingOverrideConfig)) {
                if (DEBUG_CONFIGURATION) {
                    Slog.v(TAG, "Activity has newer configuration pending so drop this"
                            + " transaction. overrideConfig=" + overrideConfig
                            + " r.mPendingOverrideConfig=" + r.mPendingOverrideConfig);
                            + " pendingOverrideConfig=" + pendingOverrideConfig);
                }
                return;
            }
            r.mPendingOverrideConfig = null;
            mPendingOverrideConfigs.remove(r.token);
        }

        if (displayId == INVALID_DISPLAY) {
+4 −21
Original line number Diff line number Diff line
@@ -83,6 +83,9 @@ public abstract class ClientTransactionHandler {
    /** Set current process state. */
    public abstract void updateProcessState(int processState, boolean fromIpc);

    /** Count how many activities are launching. */
    public abstract void countLaunchingActivities(int num);

    // Execute phase related logic and handlers. Methods here execute actual lifecycle transactions
    // and deliver callbacks.

@@ -139,7 +142,7 @@ public abstract class ClientTransactionHandler {
    public abstract void performRestartActivity(@NonNull ActivityClientRecord r, boolean start);

    /** Set pending activity configuration in case it will be updated by other transaction item. */
    public abstract void updatePendingActivityConfiguration(@NonNull ActivityClientRecord r,
    public abstract void updatePendingActivityConfiguration(@NonNull IBinder token,
            Configuration overrideConfig);

    /** Deliver activity (override) configuration change. */
@@ -188,26 +191,6 @@ public abstract class ClientTransactionHandler {
    public abstract void handleFixedRotationAdjustments(IBinder token,
            FixedRotationAdjustments fixedRotationAdjustments);

    /**
     * Add {@link ActivityClientRecord} that is preparing to be launched.
     * @param token Activity token.
     * @param activity An initialized instance of {@link ActivityClientRecord} to use during launch.
     */
    public abstract void addLaunchingActivity(IBinder token, ActivityClientRecord activity);

    /**
     * Get {@link ActivityClientRecord} that is preparing to be launched.
     * @param token Activity token.
     * @return An initialized instance of {@link ActivityClientRecord} to use during launch.
     */
    public abstract ActivityClientRecord getLaunchingActivity(IBinder token);

    /**
     * Remove {@link ActivityClientRecord} from the launching activity list.
     * @param token Activity token.
     */
    public abstract void removeLaunchingActivity(IBinder token);

    /**
     * Get {@link android.app.ActivityThread.ActivityClientRecord} instance that corresponds to the
     * provided token.
+1 −3
Original line number Diff line number Diff line
@@ -40,11 +40,9 @@ public class ActivityConfigurationChangeItem extends ActivityTransactionItem {

    @Override
    public void preExecute(android.app.ClientTransactionHandler client, IBinder token) {
        final ActivityClientRecord r = getActivityClientRecord(client, token,
                true /* includeLaunching */);
        // Notify the client of an upcoming change in the token configuration. This ensures that
        // batches of config change items only process the newest configuration.
        client.updatePendingActivityConfiguration(r, mConfiguration);
        client.updatePendingActivityConfiguration(token, mConfiguration);
    }

    @Override
+6 −26
Original line number Diff line number Diff line
@@ -53,41 +53,21 @@ public abstract class ActivityTransactionItem extends ClientTransactionItem {
    public abstract void execute(@NonNull ClientTransactionHandler client,
            @NonNull ActivityClientRecord r, PendingTransactionActions pendingActions);

    @NonNull ActivityClientRecord getActivityClientRecord(
            @NonNull ClientTransactionHandler client, IBinder token) {
        return getActivityClientRecord(client, token, false /* includeLaunching */);
    }

    /**
     * Gets the {@link ActivityClientRecord} instance that corresponds to the provided token.
     * @param client Target client handler.
     * @param token Target activity token.
     * @param includeLaunching Indicate to find the {@link ActivityClientRecord} in launching
     *                         activity list.
     *                         <p>Note that there is no {@link android.app.Activity} instance in
     *                         {@link ActivityClientRecord} from the launching activity list.
     * @return The {@link ActivityClientRecord} instance that corresponds to the provided token.
     */
    @NonNull ActivityClientRecord getActivityClientRecord(
            @NonNull ClientTransactionHandler client, IBinder token, boolean includeLaunching) {
        ActivityClientRecord r = null;
        // Check launching Activity first to prevent race condition that activity instance has not
        // yet set to ActivityClientRecord.
        if (includeLaunching) {
            r = client.getLaunchingActivity(token);
        }
        // Then if we don't want to find launching Activity or the ActivityClientRecord doesn't
        // exist in launching Activity list. The ActivityClientRecord should have been initialized
        // and put in the Activity list.
            @NonNull ClientTransactionHandler client, IBinder token) {
        final ActivityClientRecord r = client.getActivityClient(token);
        if (r == null) {
            r = client.getActivityClient(token);
            if (r != null && client.getActivity(token) == null) {
                throw new IllegalArgumentException("Activity must not be null to execute "
            throw new IllegalArgumentException("Activity client record must not be null to execute "
                    + "transaction item");
        }
        }
        if (r == null) {
            throw new IllegalArgumentException("Activity client record must not be null to execute "
        if (client.getActivity(token) == null) {
            throw new IllegalArgumentException("Activity must not be null to execute "
                    + "transaction item");
        }
        return r;
+7 −8
Original line number Diff line number Diff line
@@ -82,12 +82,7 @@ public class LaunchActivityItem extends ClientTransactionItem {

    @Override
    public void preExecute(ClientTransactionHandler client, IBinder token) {
        ActivityClientRecord r = new ActivityClientRecord(token, mIntent, mIdent, mInfo,
                mOverrideConfig, mCompatInfo, mReferrer, mVoiceInteractor, mState, mPersistentState,
                mPendingResults, mPendingNewIntents, mActivityOptions, mIsForward, mProfilerInfo,
                client, mAssistToken, mFixedRotationAdjustments, mShareableActivityToken,
                mLaunchedFromBubble);
        client.addLaunchingActivity(token, r);
        client.countLaunchingActivities(1);
        client.updateProcessState(mProcState, false);
        client.updatePendingConfiguration(mCurConfig);
        if (mActivityClientController != null) {
@@ -99,7 +94,11 @@ public class LaunchActivityItem extends ClientTransactionItem {
    public void execute(ClientTransactionHandler client, IBinder token,
            PendingTransactionActions pendingActions) {
        Trace.traceBegin(TRACE_TAG_ACTIVITY_MANAGER, "activityStart");
        ActivityClientRecord r = client.getLaunchingActivity(token);
        ActivityClientRecord r = new ActivityClientRecord(token, mIntent, mIdent, mInfo,
                mOverrideConfig, mCompatInfo, mReferrer, mVoiceInteractor, mState, mPersistentState,
                mPendingResults, mPendingNewIntents, mActivityOptions, mIsForward, mProfilerInfo,
                client, mAssistToken, mFixedRotationAdjustments, mShareableActivityToken,
                mLaunchedFromBubble);
        client.handleLaunchActivity(r, pendingActions, null /* customIntent */);
        Trace.traceEnd(TRACE_TAG_ACTIVITY_MANAGER);
    }
@@ -107,7 +106,7 @@ public class LaunchActivityItem extends ClientTransactionItem {
    @Override
    public void postExecute(ClientTransactionHandler client, IBinder token,
            PendingTransactionActions pendingActions) {
        client.removeLaunchingActivity(token);
        client.countLaunchingActivities(-1);
    }


Loading