Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Make sure config change items are executed in the order dispatched.

Browse Source 
In the previous implementation a batch of process/activity config
changes would effectively be executed out of order. When the server
would dispatch changes in config in quick succession the config change
items would update the pending configs first through the preexecute()
calls and then apply the activity config before the process config
is applied even though the process config was dispatched before the activity
config change item. See b/148639784 for more detail.

Fixes: 148639784

Test: ActivityThreadTest#testHandleActivityConfigurationChanged_EnsureUpdatesProcessedInOrder
Test: ActivityThreadTest#testHandleActivityConfigurationChanged_SkipWhenNewerConfigurationPending

Change-Id: I3c926076ac8dba73eb0471c7bc91313df519cf92
This commit is contained in:
Darryl L Johnson
2020-04-21 09:50:14 -07:00
parent 2dfc9f85fe
commit 2b9720c694
5 changed files with 145 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

36
core/java/android/app/ActivityThread.java
View File

@@ -5902,6 +5902,12 @@ public final class ActivityThread extends ClientTransactionHandler {
}
}
/**
* Sets the supplied {@code overrideConfig} as pending for the {@code activityToken}. Calling
* this method prevents any calls to
* {@link #handleActivityConfigurationChanged(IBinder, Configuration, int, boolean)} from
* processing any configurations older than {@code overrideConfig}.
*/
@Override
public void updatePendingActivityConfiguration(IBinder activityToken,
Configuration overrideConfig) {
@@ -5918,13 +5924,22 @@ public final class ActivityThread extends ClientTransactionHandler {
}
synchronized (r) {
if (r.mPendingOverrideConfig != null
&& !r.mPendingOverrideConfig.isOtherSeqNewer(overrideConfig)) {
if (DEBUG_CONFIGURATION) {
Slog.v(TAG, "Activity has newer configuration pending so drop this"
+ " transaction. overrideConfig=" + overrideConfig
+ " r.mPendingOverrideConfig=" + r.mPendingOverrideConfig);
}
return;
}
r.mPendingOverrideConfig = overrideConfig;
}
}
@Override
public void handleActivityConfigurationChanged(IBinder activityToken,
Configuration overrideConfig, int displayId) {
@NonNull Configuration overrideConfig, int displayId) {
handleActivityConfigurationChanged(activityToken, overrideConfig, displayId,
// This is the only place that uses alwaysReportChange=true. The entry point should
// be from ActivityConfigurationChangeItem or MoveToDisplayItem, so the server side
@@ -5935,15 +5950,18 @@ public final class ActivityThread extends ClientTransactionHandler {
}
/**
* Handle new activity configuration and/or move to a different display.
* Handle new activity configuration and/or move to a different display. This method is a noop
* if {@link #updatePendingActivityConfiguration(IBinder, Configuration)} has been called with
* a newer config than {@code overrideConfig}.
*
* @param activityToken Target activity token.
* @param overrideConfig Activity override config.
* @param displayId Id of the display where activity was moved to, -1 if there was no move and
* value didn't change.
* @param alwaysReportChange If the configuration is changed, always report to activity.
*/
void handleActivityConfigurationChanged(IBinder activityToken, Configuration overrideConfig,
int displayId, boolean alwaysReportChange) {
void handleActivityConfigurationChanged(IBinder activityToken,
@NonNull Configuration overrideConfig, int displayId, boolean alwaysReportChange) {
ActivityClientRecord r = mActivities.get(activityToken);
// Check input params.
if (r == null || r.activity == null) {
@@ -5954,9 +5972,13 @@ public final class ActivityThread extends ClientTransactionHandler {
&& displayId != r.activity.getDisplayId();
synchronized (r) {
if (r.mPendingOverrideConfig != null
&& !r.mPendingOverrideConfig.isOtherSeqNewer(overrideConfig)) {
overrideConfig = r.mPendingOverrideConfig;
if (overrideConfig.isOtherSeqNewer(r.mPendingOverrideConfig)) {
if (DEBUG_CONFIGURATION) {
Slog.v(TAG, "Activity has newer configuration pending so drop this"
+ " transaction. overrideConfig=" + overrideConfig
+ " r.mPendingOverrideConfig=" + r.mPendingOverrideConfig);
}
return;
}
r.mPendingOverrideConfig = null;
}

11
core/java/android/app/servertransaction/ActivityConfigurationChangeItem.java
View File

@@ -19,6 +19,7 @@ package android.app.servertransaction;
import static android.os.Trace.TRACE_TAG_ACTIVITY_MANAGER;
import static android.view.Display.INVALID_DISPLAY;
import android.annotation.NonNull;
import android.app.ClientTransactionHandler;
import android.content.res.Configuration;
import android.os.IBinder;
@@ -37,6 +38,8 @@ public class ActivityConfigurationChangeItem extends ClientTransactionItem {
@Override
public void preExecute(android.app.ClientTransactionHandler client, IBinder token) {
// 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(token, mConfiguration);
}
@@ -55,7 +58,11 @@ public class ActivityConfigurationChangeItem extends ClientTransactionItem {
private ActivityConfigurationChangeItem() {}
/** Obtain an instance initialized with provided params. */
public static ActivityConfigurationChangeItem obtain(Configuration config) {
public static ActivityConfigurationChangeItem obtain(@NonNull Configuration config) {
if (config == null) {
throw new IllegalArgumentException("Config must not be null.");
}
ActivityConfigurationChangeItem instance =
ObjectPool.obtain(ActivityConfigurationChangeItem.class);
if (instance == null) {
@@ -68,7 +75,7 @@ public class ActivityConfigurationChangeItem extends ClientTransactionItem {
@Override
public void recycle() {
mConfiguration = null;
mConfiguration = Configuration.EMPTY;
ObjectPool.recycle(this);
}

17
core/java/android/app/servertransaction/MoveToDisplayItem.java
View File

@@ -18,6 +18,7 @@ package android.app.servertransaction;
import static android.os.Trace.TRACE_TAG_ACTIVITY_MANAGER;
import android.annotation.NonNull;
import android.app.ClientTransactionHandler;
import android.content.res.Configuration;
import android.os.IBinder;
@@ -35,6 +36,13 @@ public class MoveToDisplayItem extends ClientTransactionItem {
private int mTargetDisplayId;
private Configuration mConfiguration;
@Override
public void preExecute(ClientTransactionHandler client, IBinder token) {
// 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(token, mConfiguration);
}
@Override
public void execute(ClientTransactionHandler client, IBinder token,
PendingTransactionActions pendingActions) {
@@ -49,7 +57,12 @@ public class MoveToDisplayItem extends ClientTransactionItem {
private MoveToDisplayItem() {}
/** Obtain an instance initialized with provided params. */
public static MoveToDisplayItem obtain(int targetDisplayId, Configuration configuration) {
public static MoveToDisplayItem obtain(int targetDisplayId,
@NonNull Configuration configuration) {
if (configuration == null) {
throw new IllegalArgumentException("Configuration must not be null");
}
MoveToDisplayItem instance = ObjectPool.obtain(MoveToDisplayItem.class);
if (instance == null) {
instance = new MoveToDisplayItem();
@@ -63,7 +76,7 @@ public class MoveToDisplayItem extends ClientTransactionItem {
@Override
public void recycle() {
mTargetDisplayId = 0;
mConfiguration = null;
mConfiguration = Configuration.EMPTY;
ObjectPool.recycle(this);
}

106
core/tests/coretests/src/android/app/activity/ActivityThreadTest.java
View File

@@ -20,6 +20,7 @@ import static android.content.Intent.ACTION_EDIT;
import static android.content.Intent.ACTION_VIEW;
import static android.content.res.Configuration.ORIENTATION_LANDSCAPE;
import static android.content.res.Configuration.ORIENTATION_PORTRAIT;
import static android.view.Display.INVALID_DISPLAY;
import static com.google.common.truth.Truth.assertThat;
@@ -29,6 +30,7 @@ import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.testng.Assert.assertFalse;
import android.annotation.Nullable;
import android.app.Activity;
import android.app.ActivityThread;
import android.app.IApplicationThread;
@@ -38,6 +40,7 @@ import android.app.servertransaction.ActivityConfigurationChangeItem;
import android.app.servertransaction.ActivityRelaunchItem;
import android.app.servertransaction.ClientTransaction;
import android.app.servertransaction.ClientTransactionItem;
import android.app.servertransaction.ConfigurationChangeItem;
import android.app.servertransaction.NewIntentItem;
import android.app.servertransaction.ResumeActivityItem;
import android.app.servertransaction.StopActivityItem;
@@ -225,7 +228,7 @@ public class ActivityThreadTest {
}
@Test
public void testHandleActivityConfigurationChanged_PickNewerPendingConfiguration() {
public void testHandleActivityConfigurationChanged_SkipWhenNewerConfigurationPending() {
final TestActivity activity = mActivityTestRule.launchActivity(new Intent());
InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> {
@@ -237,25 +240,90 @@ public class ActivityThreadTest {
final ActivityThread activityThread = activity.getActivityThread();
final Configuration pendingConfig = new Configuration();
pendingConfig.orientation = orientation == ORIENTATION_LANDSCAPE
? ORIENTATION_PORTRAIT
: ORIENTATION_LANDSCAPE;
pendingConfig.seq = seq + 2;
final Configuration newerConfig = new Configuration();
newerConfig.orientation = orientation == ORIENTATION_LANDSCAPE
? ORIENTATION_PORTRAIT : ORIENTATION_LANDSCAPE;
newerConfig.seq = seq + 2;
activityThread.updatePendingActivityConfiguration(activity.getActivityToken(),
pendingConfig);
newerConfig);
final Configuration newConfig = new Configuration();
newConfig.orientation = orientation;
newConfig.seq = seq + 1;
final Configuration olderConfig = new Configuration();
olderConfig.orientation = orientation;
olderConfig.seq = seq + 1;
activityThread.handleActivityConfigurationChanged(activity.getActivityToken(),
newConfig, Display.INVALID_DISPLAY);
olderConfig, INVALID_DISPLAY);
assertEquals(numOfConfig, activity.mNumOfConfigChanges);
assertEquals(olderConfig.orientation, activity.mConfig.orientation);
activityThread.handleActivityConfigurationChanged(activity.getActivityToken(),
newerConfig, INVALID_DISPLAY);
assertEquals(numOfConfig + 1, activity.mNumOfConfigChanges);
assertEquals(pendingConfig.orientation, activity.mConfig.orientation);
assertEquals(newerConfig.orientation, activity.mConfig.orientation);
});
}
@Test
public void testHandleActivityConfigurationChanged_EnsureUpdatesProcessedInOrder()
throws Exception {
final TestActivity activity = mActivityTestRule.launchActivity(new Intent());
final ActivityThread activityThread = activity.getActivityThread();
InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> {
final Configuration config = new Configuration();
config.seq = BASE_SEQ;
config.orientation = ORIENTATION_PORTRAIT;
activityThread.handleActivityConfigurationChanged(activity.getActivityToken(),
config, INVALID_DISPLAY);
});
final IApplicationThread appThread = activityThread.getApplicationThread();
final int numOfConfig = activity.mNumOfConfigChanges;
final Configuration processConfigLandscape = new Configuration();
processConfigLandscape.windowConfiguration.setBounds(new Rect(0, 0, 100, 60));
processConfigLandscape.seq = BASE_SEQ + 1;
final Configuration activityConfigLandscape = new Configuration();
activityConfigLandscape.windowConfiguration.setBounds(new Rect(0, 0, 100, 50));
activityConfigLandscape.seq = BASE_SEQ + 2;
final Configuration processConfigPortrait = new Configuration();
processConfigPortrait.windowConfiguration.setBounds(new Rect(0, 0, 60, 100));
processConfigPortrait.seq = BASE_SEQ + 3;
final Configuration activityConfigPortrait = new Configuration();
activityConfigPortrait.windowConfiguration.setBounds(new Rect(0, 0, 50, 100));
activityConfigPortrait.seq = BASE_SEQ + 4;
activity.mConfigLatch = new CountDownLatch(1);
activity.mTestLatch = new CountDownLatch(1);
ClientTransaction transaction = newTransaction(activityThread, null);
transaction.addCallback(ConfigurationChangeItem.obtain(processConfigLandscape));
appThread.scheduleTransaction(transaction);
transaction = newTransaction(activityThread, activity.getActivityToken());
transaction.addCallback(ActivityConfigurationChangeItem.obtain(activityConfigLandscape));
transaction.addCallback(ConfigurationChangeItem.obtain(processConfigPortrait));
transaction.addCallback(ActivityConfigurationChangeItem.obtain(activityConfigPortrait));
appThread.scheduleTransaction(transaction);
activity.mTestLatch.await();
activity.mConfigLatch.countDown();
activity.mConfigLatch = null;
activity.mTestLatch = null;
// Check display metrics, bounds should match the portrait activity bounds.
final Rect bounds = activity.getWindowManager().getCurrentWindowMetrics().getBounds();
assertEquals(activityConfigPortrait.windowConfiguration.getBounds(), bounds);
// Ensure that Activity#onConfigurationChanged() is only called once.
assertEquals(numOfConfig + 1, activity.mNumOfConfigChanges);
}
@Test
public void testHandleActivityConfigurationChanged_OnlyAppliesNewestConfiguration()
throws Exception {
@@ -268,7 +336,7 @@ public class ActivityThreadTest {
config.orientation = ORIENTATION_PORTRAIT;
activityThread.handleActivityConfigurationChanged(activity.getActivityToken(),
config, Display.INVALID_DISPLAY);
config, INVALID_DISPLAY);
});
final int numOfConfig = activity.mNumOfConfigChanges;
@@ -504,7 +572,7 @@ public class ActivityThreadTest {
config.orientation = ORIENTATION_PORTRAIT;
config.seq = seq;
activityThread.handleActivityConfigurationChanged(activity.getActivityToken(), config,
Display.INVALID_DISPLAY);
INVALID_DISPLAY);
if (activity.mNumOfConfigChanges > numOfConfig) {
return config.seq;
@@ -514,7 +582,7 @@ public class ActivityThreadTest {
config.orientation = ORIENTATION_LANDSCAPE;
config.seq = seq + 1;
activityThread.handleActivityConfigurationChanged(activity.getActivityToken(), config,
Display.INVALID_DISPLAY);
INVALID_DISPLAY);
return config.seq;
}
@@ -572,8 +640,12 @@ public class ActivityThreadTest {
}
private static ClientTransaction newTransaction(Activity activity) {
final IApplicationThread appThread = activity.getActivityThread().getApplicationThread();
return ClientTransaction.obtain(appThread, activity.getActivityToken());
return newTransaction(activity.getActivityThread(), activity.getActivityToken());
}
private static ClientTransaction newTransaction(ActivityThread activityThread,
@Nullable IBinder activityToken) {
return ClientTransaction.obtain(activityThread.getApplicationThread(), activityToken);
}
// Test activity

5
core/tests/coretests/src/android/app/servertransaction/ObjectPoolTests.java
View File

@@ -63,7 +63,8 @@ public class ObjectPoolTests {
@Test
public void testRecycleActivityConfigurationChangeItem() {
ActivityConfigurationChangeItem emptyItem = ActivityConfigurationChangeItem.obtain(null);
ActivityConfigurationChangeItem emptyItem =
ActivityConfigurationChangeItem.obtain(Configuration.EMPTY);
ActivityConfigurationChangeItem item = ActivityConfigurationChangeItem.obtain(config());
assertNotSame(item, emptyItem);
assertFalse(item.equals(emptyItem));
@@ -186,7 +187,7 @@ public class ObjectPoolTests {
@Test
public void testRecycleMoveToDisplayItem() {
MoveToDisplayItem emptyItem = MoveToDisplayItem.obtain(0, null);
MoveToDisplayItem emptyItem = MoveToDisplayItem.obtain(0, Configuration.EMPTY);
MoveToDisplayItem item = MoveToDisplayItem.obtain(4, config());
assertNotSame(item, emptyItem);
assertFalse(item.equals(emptyItem));