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

Commit b9db2311 authored by Charles Chen's avatar Charles Chen
Browse files

Fix #testChangeFontScaleNoRelaunch flaky 

The test failed because we used public config diff to determine if
 Activity#onConfigurationChanged should be dispatched.
However, the public config diff contained the change which wouldn't
make Activity relaunch and also receive the callback.

This CL uses the public diff, which filters the change without
relaunching Activity, to determine if the callback should be
dispatched. This CL also covers the case that the callback is
still dispatched if small changes are handled.

Test: atest ActivityThreadClientTest ConfigChangeTests
Bug: 222067515
Bug: 222223467
Change-Id: I8aa80e9fd70fddc9f88a36e7dc2ffb2163b9ffa6
parent 87d60042

core/java/android/app/Activity.java

+1 −1
Original line number Original line Diff line number Diff line
@@ -872,7 +872,7 @@ public class Activity extends ContextThemeWrapper 
    @UnsupportedAppUsage

    @UnsupportedAppUsage

    /*package*/ int mConfigChangeFlags;

    /*package*/ int mConfigChangeFlags;

    @UnsupportedAppUsage

    @UnsupportedAppUsage

    /*package*/ Configuration mCurrentConfig;

    /*package*/ Configuration mCurrentConfig = Configuration.EMPTY;

    private SearchManager mSearchManager;

    private SearchManager mSearchManager;

    private MenuInflater mMenuInflater;

    private MenuInflater mMenuInflater;

core/java/android/app/ActivityThread.java

+39 −7
Original line number Original line Diff line number Diff line
@@ -5883,19 +5883,17 @@ public final class ActivityThread extends ClientTransactionHandler 
        final boolean movedToDifferentDisplay = isDifferentDisplay(activity.getDisplayId(),

        final boolean movedToDifferentDisplay = isDifferentDisplay(activity.getDisplayId(),

                displayId);

                displayId);

        final Configuration currentConfig = activity.mCurrentConfig;

        final Configuration currentConfig = activity.mCurrentConfig;

        final int diff = (currentConfig == null) ? 0xffffffff

        final int diff = currentConfig.diffPublicOnly(newConfig);

                : currentConfig.diffPublicOnly(newConfig);

        final boolean hasPublicConfigChange = diff != 0;

        final boolean hasPublicConfigChange = diff != 0;

        final ActivityClientRecord r = getActivityClient(activityToken);

        // TODO(b/173090263): Use diff instead after the improvement of AssetManager and

        // TODO(b/173090263): Use diff instead after the improvement of AssetManager and

        // ResourcesImpl constructions.

        // ResourcesImpl constructions.

        final boolean shouldUpdateResources = hasPublicConfigChange

        final boolean shouldUpdateResources = hasPublicConfigChange

                || shouldUpdateResources(activityToken, currentConfig, newConfig, amOverrideConfig,

                || shouldUpdateResources(activityToken, currentConfig, newConfig, amOverrideConfig,

                movedToDifferentDisplay, hasPublicConfigChange);

                movedToDifferentDisplay, hasPublicConfigChange);

        final boolean shouldReportChange = hasPublicConfigChange

        final boolean shouldReportChange = shouldReportChange(diff, currentConfig, newConfig,

                // If this activity doesn't handle any of the config changes, then don't bother

                r != null ? r.mSizeConfigurations : null,

                // calling onConfigurationChanged. Otherwise, report to the activity for the

                activity.mActivityInfo.getRealConfigChanged());

                // changes.

                && (~activity.mActivityInfo.getRealConfigChanged() & diff) == 0;

        // Nothing significant, don't proceed with updating and reporting.

        // Nothing significant, don't proceed with updating and reporting.

        if (!shouldUpdateResources) {

        if (!shouldUpdateResources) {

            return null;

            return null;
@@ -5942,6 +5940,40 @@ public final class ActivityThread extends ClientTransactionHandler 
        return configToReport;

        return configToReport;

    }

    }





    /**

     * 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}

     * @param sizeBuckets The Activity's {@link SizeConfigurationBuckets} if not {@code null}

     * @param handledConfigChanges Bit mask of configuration changes that the activity can handle

     * @return {@code true} if the config change should be reported to the Activity

     */

    @VisibleForTesting

    public static boolean shouldReportChange(int publicDiff, @Nullable Configuration currentConfig,

            @NonNull Configuration newConfig, @Nullable SizeConfigurationBuckets sizeBuckets,

            int handledConfigChanges) {

        // Don't report the change if there's no public diff between current and new config.

        if (publicDiff == 0) {

            return false;

        }

        final int diffWithBucket = SizeConfigurationBuckets.filterDiff(publicDiff, currentConfig,

                newConfig, sizeBuckets);

        // Compare to the diff which filter the change without crossing size buckets with

        // {@code handledConfigChanges}. The small changes should not block Activity to receive

        // its handled config updates. Also, if Activity handles all small changes, we should

        // dispatch the updated config to it.

        final int diff = diffWithBucket != 0 ? diffWithBucket : publicDiff;

        // If this activity doesn't handle any of the config changes, then don't bother

        // calling onConfigurationChanged. Otherwise, report to the activity for the

        // changes.

        return (~handledConfigChanges & diff) == 0;

    }



    public final void applyConfigurationToResources(Configuration config) {

    public final void applyConfigurationToResources(Configuration config) {

        synchronized (mResourcesManager) {

        synchronized (mResourcesManager) {

            mResourcesManager.applyConfigurationToResources(config, null);

            mResourcesManager.applyConfigurationToResources(config, null);

core/tests/mockingcoretests/src/android/app/activity/ActivityThreadClientTest.java

+47 −0
Original line number Original line Diff line number Diff line
@@ -16,12 +16,15 @@ 




package android.app.activity;

package android.app.activity;





import static android.app.ActivityThread.shouldReportChange;

import static android.app.servertransaction.ActivityLifecycleItem.ON_CREATE;

import static android.app.servertransaction.ActivityLifecycleItem.ON_CREATE;

import static android.app.servertransaction.ActivityLifecycleItem.ON_DESTROY;

import static android.app.servertransaction.ActivityLifecycleItem.ON_DESTROY;

import static android.app.servertransaction.ActivityLifecycleItem.ON_PAUSE;

import static android.app.servertransaction.ActivityLifecycleItem.ON_PAUSE;

import static android.app.servertransaction.ActivityLifecycleItem.ON_RESUME;

import static android.app.servertransaction.ActivityLifecycleItem.ON_RESUME;

import static android.app.servertransaction.ActivityLifecycleItem.ON_START;

import static android.app.servertransaction.ActivityLifecycleItem.ON_START;

import static android.app.servertransaction.ActivityLifecycleItem.ON_STOP;

import static android.app.servertransaction.ActivityLifecycleItem.ON_STOP;

import static android.content.pm.ActivityInfo.CONFIG_FONT_SCALE;

import static android.content.pm.ActivityInfo.CONFIG_SCREEN_SIZE;





import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation;

import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation;
@@ -31,6 +34,8 @@ import static com.android.dx.mockito.inline.extended.ExtendedMockito.mockitoSess 
import static com.android.dx.mockito.inline.extended.ExtendedMockito.spyOn;

import static com.android.dx.mockito.inline.extended.ExtendedMockito.spyOn;





import static org.junit.Assert.assertEquals;

import static org.junit.Assert.assertEquals;

import static org.junit.Assert.assertFalse;

import static org.junit.Assert.assertTrue;

import static org.mockito.ArgumentMatchers.any;

import static org.mockito.ArgumentMatchers.any;

import static org.mockito.ArgumentMatchers.anyInt;

import static org.mockito.ArgumentMatchers.anyInt;

import static org.mockito.Mockito.clearInvocations;

import static org.mockito.Mockito.clearInvocations;
@@ -56,6 +61,7 @@ import android.os.UserHandle; 
import android.platform.test.annotations.Presubmit;

import android.platform.test.annotations.Presubmit;

import android.testing.PollingCheck;

import android.testing.PollingCheck;

import android.view.WindowManagerGlobal;

import android.view.WindowManagerGlobal;

import android.window.SizeConfigurationBuckets;





import androidx.test.annotation.UiThreadTest;

import androidx.test.annotation.UiThreadTest;

import androidx.test.ext.junit.runners.AndroidJUnit4;

import androidx.test.ext.junit.runners.AndroidJUnit4;
@@ -195,6 +201,47 @@ public class ActivityThreadClientTest { 
        }

        }

    }

    }





    @Test

    public void testShouldReportChange() {

        final Configuration newConfig = new Configuration();

        final Configuration currentConfig = new Configuration();



        assertFalse("Must not report change if no public diff",

                shouldReportChange(0 /* publicDiff */, currentConfig, newConfig,

                null /* sizeBuckets */, 0 /* handledConfigChanges */));



        final int[] verticalThresholds = {100, 400};

        final SizeConfigurationBuckets buckets = new SizeConfigurationBuckets(

                null /* horizontal */,

                verticalThresholds,

                null /* smallest */,

                null /* screenLayoutSize */,

                false /* screenLayoutLongSet */);

        currentConfig.screenHeightDp = 200;

        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 */));



        assertTrue("Must report changes if the small diff is handled",

                shouldReportChange(CONFIG_SCREEN_SIZE /* publicDiff */, 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 */));



        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 */));

    }



    private void recreateAndVerifyNoRelaunch(ActivityThread activityThread, TestActivity activity) {

    private void recreateAndVerifyNoRelaunch(ActivityThread activityThread, TestActivity activity) {

        clearInvocations(activityThread);

        clearInvocations(activityThread);

        getInstrumentation().runOnMainSync(() -> activity.recreate());

        getInstrumentation().runOnMainSync(() -> activity.recreate());