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
Merged-In: If350e942254e54c9ec90bc63a6e50eb67d038183
Change-Id: If350e942254e54c9ec90bc63a6e50eb67d038183
This commit is contained in:
Chilun
2021-12-23 15:51:07 +08:00
committed by Jared Duke
parent 86085aae14
commit 3be8e11bdc
7 changed files with 52 additions and 105 deletions

View File

@@ -244,6 +244,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;
/**
@@ -345,11 +346,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>());
@@ -359,6 +358,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")
@@ -556,10 +556,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.
@@ -3329,21 +3325,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);
@@ -3388,7 +3369,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 {
@@ -3404,7 +3385,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) {
@@ -3442,6 +3423,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,
@@ -6071,31 +6057,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.
@@ -6106,16 +6092,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) {

View File

@@ -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.

View File

@@ -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

View File

@@ -53,43 +53,23 @@ 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.
if (r == null) {
r = client.getActivityClient(token);
if (r != null && client.getActivity(token) == null) {
throw new IllegalArgumentException("Activity must not be null to execute "
+ "transaction item");
}
}
@NonNull ClientTransactionHandler client, IBinder token) {
final ActivityClientRecord r = client.getActivityClient(token);
if (r == null) {
throw new IllegalArgumentException("Activity client record must not be null to execute "
+ "transaction item");
}
if (client.getActivity(token) == null) {
throw new IllegalArgumentException("Activity must not be null to execute "
+ "transaction item");
}
return r;
}
}

View File

@@ -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);
}

View File

@@ -40,11 +40,9 @@ public class MoveToDisplayItem extends ActivityTransactionItem {
@Override
public void preExecute(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

View File

@@ -273,13 +273,14 @@ public class ActivityThreadTest {
newerConfig.orientation = orientation == ORIENTATION_LANDSCAPE
? ORIENTATION_PORTRAIT : ORIENTATION_LANDSCAPE;
newerConfig.seq = seq + 2;
final ActivityClientRecord r = getActivityClientRecord(activity);
activityThread.updatePendingActivityConfiguration(r, newerConfig);
activityThread.updatePendingActivityConfiguration(activity.getActivityToken(),
newerConfig);
final Configuration olderConfig = new Configuration();
olderConfig.orientation = orientation;
olderConfig.seq = seq + 1;
final ActivityClientRecord r = getActivityClientRecord(activity);
activityThread.handleActivityConfigurationChanged(r, olderConfig, INVALID_DISPLAY);
assertEquals(numOfConfig, activity.mNumOfConfigChanges);
assertEquals(olderConfig.orientation, activity.mConfig.orientation);
@@ -504,7 +505,8 @@ public class ActivityThreadTest {
? ORIENTATION_LANDSCAPE : ORIENTATION_PORTRAIT;
final ActivityClientRecord r = getActivityClientRecord(activity);
activityThread.updatePendingActivityConfiguration(r, newActivityConfig);
activityThread.updatePendingActivityConfiguration(activity.getActivityToken(),
newActivityConfig);
activityThread.handleActivityConfigurationChanged(r, newActivityConfig,
INVALID_DISPLAY);