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

Reland “Update the activity current config only if it is reported”

Browse Source 
The current configuration in the client activity could be
updated via the other flow (from ViewRootImpl). So, it is
possible that the client activity configuration is different
from the last reported configuration that cached in the system.

If the configuration changed again in that case, the
ActivityConfigurationChangeItem could end up without reporting
Activity#onConfigurationChanged if the differences between
the new configuration and the current client activity
configuration contains the configuration changes that the
activity cannot handle.

Update the activity current config only when reported, so
the configuration differences can be correctly evaluated from
the new configuration and the configuration last reported.

Bug: 231312158
Bug: 236415038
Test: repro steps on the bug
Test: atest ActivityThreadClientTest
Test: atest OverlayHostTest
Change-Id: I56a871fb30809f5f75f0007601a7629f9435e03a
This commit is contained in:
Louis Chang
2022-06-09 10:30:04 +08:00
parent 9e76e35728
commit 3f4bbc3a41
2 changed files with 22 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -5863,20 +5863,20 @@ public final class ActivityThread extends ClientTransactionHandler
final boolean movedToDifferentDisplay = isDifferentDisplay(activity.getDisplayId(),
displayId);
final Configuration currentConfig = activity.mCurrentConfig;
final int diff = currentConfig.diffPublicOnly(newConfig);
final boolean hasPublicConfigChange = diff != 0;
final Configuration currentResConfig = activity.getResources().getConfiguration();
final int diff = currentResConfig.diffPublicOnly(newConfig);
final boolean hasPublicResConfigChange = diff != 0;
final ActivityClientRecord r = getActivityClient(activityToken);
// TODO(b/173090263): Use diff instead after the improvement of AssetManager and
// ResourcesImpl constructions.
final boolean shouldUpdateResources = hasPublicConfigChange
|| shouldUpdateResources(activityToken, currentConfig, newConfig, amOverrideConfig,
movedToDifferentDisplay, hasPublicConfigChange);
final boolean shouldReportChange = shouldReportChange(diff, currentConfig, newConfig,
final boolean shouldUpdateResources = hasPublicResConfigChange
|| shouldUpdateResources(activityToken, currentResConfig, newConfig,
amOverrideConfig, movedToDifferentDisplay, hasPublicResConfigChange);
final boolean shouldReportChange = shouldReportChange(activity.mCurrentConfig, newConfig,
r != null ? r.mSizeConfigurations : null,
activity.mActivityInfo.getRealConfigChanged());
// Nothing significant, don't proceed with updating and reporting.
if (!shouldUpdateResources) {
if (!shouldUpdateResources && !shouldReportChange) {
return null;
}
@@ -5896,9 +5896,6 @@ public final class ActivityThread extends ClientTransactionHandler
amOverrideConfig, contextThemeWrapperOverrideConfig);
mResourcesManager.updateResourcesForActivity(activityToken, finalOverrideConfig, displayId);
activity.mConfigChangeFlags = 0;
activity.mCurrentConfig = new Configuration(newConfig);
// Apply the ContextThemeWrapper override if necessary.
// NOTE: Make sure the configurations are not modified, as they are treated as immutable
// in many places.
@@ -5909,8 +5906,10 @@ public final class ActivityThread extends ClientTransactionHandler
activity.dispatchMovedToDisplay(displayId, configToReport);
}
activity.mConfigChangeFlags = 0;
if (shouldReportChange) {
activity.mCalled = false;
activity.mCurrentConfig = new Configuration(newConfig);
activity.onConfigurationChanged(configToReport);
if (!activity.mCalled) {
throw new SuperNotCalledException("Activity " + activity.getLocalClassName() +
@@ -5925,8 +5924,6 @@ public final class ActivityThread extends ClientTransactionHandler
* Returns {@code true} if {@link Activity#onConfigurationChanged(Configuration)} should be
* dispatched.
*
* @param publicDiff Usually computed by {@link Configuration#diffPublicOnly(Configuration)}.
* This parameter is to prevent we compute it again.
* @param currentConfig The current configuration cached in {@link Activity#mCurrentConfig}.
* It is {@code null} before the first config update from the server side.
* @param newConfig The updated {@link Configuration}
@@ -5935,9 +5932,10 @@ public final class ActivityThread extends ClientTransactionHandler
* @return {@code true} if the config change should be reported to the Activity
*/
@VisibleForTesting
public static boolean shouldReportChange(int publicDiff, @Nullable Configuration currentConfig,
public static boolean shouldReportChange(@Nullable Configuration currentConfig,
@NonNull Configuration newConfig, @Nullable SizeConfigurationBuckets sizeBuckets,
int handledConfigChanges) {
final int publicDiff = currentConfig.diffPublicOnly(newConfig);
// Don't report the change if there's no public diff between current and new config.
if (publicDiff == 0) {
return false;

20
core/tests/mockingcoretests/src/android/app/activity/ActivityThreadClientTest.java
View File

@@ -207,8 +207,8 @@ public class ActivityThreadClientTest {
final Configuration currentConfig = new Configuration();
assertFalse("Must not report change if no public diff",
shouldReportChange(0 /* publicDiff */, currentConfig, newConfig,
null /* sizeBuckets */, 0 /* handledConfigChanges */));
shouldReportChange(currentConfig, newConfig, null /* sizeBuckets */,
0 /* handledConfigChanges */));
final int[] verticalThresholds = {100, 400};
final SizeConfigurationBuckets buckets = new SizeConfigurationBuckets(
@@ -221,25 +221,25 @@ public class ActivityThreadClientTest {
newConfig.screenHeightDp = 300;
assertFalse("Must not report changes if the diff is small and not handled",
shouldReportChange(CONFIG_SCREEN_SIZE /* publicDiff */, currentConfig,
newConfig, buckets, CONFIG_FONT_SCALE /* handledConfigChanges */));
shouldReportChange(currentConfig, newConfig, buckets,
CONFIG_FONT_SCALE /* handledConfigChanges */));
assertTrue("Must report changes if the small diff is handled",
shouldReportChange(CONFIG_SCREEN_SIZE /* publicDiff */, currentConfig, newConfig,
buckets, CONFIG_SCREEN_SIZE /* handledConfigChanges */));
shouldReportChange(currentConfig, newConfig, buckets,
CONFIG_SCREEN_SIZE /* handledConfigChanges */));
currentConfig.fontScale = 0.8f;
newConfig.fontScale = 1.2f;
assertTrue("Must report handled changes regardless of small unhandled change",
shouldReportChange(CONFIG_SCREEN_SIZE | CONFIG_FONT_SCALE /* publicDiff */,
currentConfig, newConfig, buckets, CONFIG_FONT_SCALE /* handledConfigChanges */));
shouldReportChange(currentConfig, newConfig, buckets,
CONFIG_FONT_SCALE /* handledConfigChanges */));
newConfig.screenHeightDp = 500;
assertFalse("Must not report changes if there's unhandled big changes",
shouldReportChange(CONFIG_SCREEN_SIZE | CONFIG_FONT_SCALE /* publicDiff */,
currentConfig, newConfig, buckets, CONFIG_FONT_SCALE /* handledConfigChanges */));
shouldReportChange(currentConfig, newConfig, buckets,
CONFIG_FONT_SCALE /* handledConfigChanges */));
}
private void recreateAndVerifyNoRelaunch(ActivityThread activityThread, TestActivity activity) {