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

Commit d41d9c9b authored by Satoshi Niwa's avatar Satoshi Niwa
Browse files

Refactor CoreSettingsObserver to improve locking and clarity 

This is a preparatory refactoring that extracts clean-ups from
eae16b2f6a98f84bb56a7d4a57ea40e7a1affe18

Key changes include:
- CoreSettingsObserver now manages its own internal lock instead of relying on the ActivityManagerService lock.
- Refactored sendCoreSettings() to build settings in a temporary bundle, improving readability and reducing lock duration.
- Migrated tests in CoreSettingsObserverTest to the Truth assertion library for consistency and updated them for the refactor.

Test: atest CoreSettingsObserverTest
Bug: 413694508
Flag: EXEMPT refactor
Change-Id: I6330bc9d697939f2486ab659f1317714adb20e7f
parent 1b827075

services/core/java/com/android/server/am/ActivityManagerService.java

+1 −1
Original line number Diff line number Diff line
@@ -4814,7 +4814,7 @@ public class ActivityManagerService extends IActivityManager.Stub 
                        preBindInfo.configuration,

                        app.getCompat(),

                        getCommonServicesLocked(app.isolated),

                        mCoreSettingsObserver.getCoreSettingsLocked(),

                        mCoreSettingsObserver.getCoreSettings(),

                        buildSerial,

                        autofillOptions,

                        contentCaptureOptions,

services/core/java/com/android/server/am/CoreSettingsObserver.java

+115 −55
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ 
package com.android.server.am;



import android.annotation.NonNull;

import android.annotation.Nullable;

import android.app.ActivityThread;

import android.companion.virtual.VirtualDevice;

import android.companion.virtual.VirtualDeviceManager;
@@ -27,11 +28,13 @@ import android.net.Uri; 
import android.os.Bundle;

import android.provider.DeviceConfig;

import android.provider.Settings;

import android.util.Slog;

import android.util.IntArray;

import android.widget.WidgetFlags;



import com.android.internal.R;

import com.android.internal.annotations.GuardedBy;

import com.android.internal.annotations.VisibleForTesting;

import com.android.server.utils.Slogf;



import java.util.ArrayList;

import java.util.HashMap;
@@ -49,6 +52,9 @@ import java.util.Objects; 
final class CoreSettingsObserver extends ContentObserver {



    private static final String TAG = "CoreSettingsObserver";

    private static final boolean DEBUG = false;



    private final Object mLock = new Object();



    private static class DeviceConfigEntry<T> {

        String namespace;
@@ -170,9 +176,12 @@ final class CoreSettingsObserver extends ContentObserver { 
    }

    private static volatile boolean sDeviceConfigContextEntriesLoaded = false;



    @GuardedBy("mLock")

    private final Bundle mCoreSettings = new Bundle();



    private final ActivityManagerService mActivityManagerService;



    @Nullable

    private VirtualDeviceManager mVirtualDeviceManager;



    public CoreSettingsObserver(ActivityManagerService activityManagerService) {
@@ -200,87 +209,127 @@ final class CoreSettingsObserver extends ContentObserver { 
                        .getInteger(R.integer.config_defaultAnalogClockSecondsHandFps)));

    }



    public Bundle getCoreSettingsLocked() {

    /**

     * Gets a deep copy of the core settings.

     */

    public Bundle getCoreSettings() {

        synchronized (mLock) {

            return mCoreSettings.deepCopy();

        }

    }



    @Override

    public void onChange(boolean selfChange) {

        synchronized (mActivityManagerService) {

            sendCoreSettings();

        if (DEBUG) {

            Slogf.d(TAG, "Core settings changed, selfChange: %b", selfChange);

        }

        sendCoreSettings();

    }



    private void sendCoreSettings() {

        final Context context = mActivityManagerService.mContext;

        if (android.companion.virtualdevice.flags.Flags.deviceAwareSettingsOverride()) {

            final List<Integer> deviceIds = new ArrayList<>();

    private IntArray getVirtualDeviceIds() {

        if (mVirtualDeviceManager == null) {

            mVirtualDeviceManager = mActivityManagerService.mContext.getSystemService(

                    VirtualDeviceManager.class);

            if (mVirtualDeviceManager == null) {

                return new IntArray(0);

            }

        }

            if (mVirtualDeviceManager != null) {

                final List<VirtualDevice> virtualDevices =

                        mVirtualDeviceManager.getVirtualDevices();

                for (VirtualDevice virtualDevice : virtualDevices) {

                    deviceIds.add(virtualDevice.getDeviceId());



        List<VirtualDevice> virtualDevices = mVirtualDeviceManager.getVirtualDevices();

        IntArray deviceIds = new IntArray(virtualDevices.size());

        for (int i = 0; i < virtualDevices.size(); i++) {

            deviceIds.add(virtualDevices.get(i).getDeviceId());

        }

        return deviceIds;

    }



            Bundle globalSettingsBundle = new Bundle();

    /**

     * Populates the core settings bundle with the latest values and sends them to app processes

     * via {@link ActivityThread}.

     */

    private void sendCoreSettings() {

        Context context = mActivityManagerService.mContext;



        // Create a temporary bundle to store the settings that will be sent.

        Bundle settingsToSend;



        if (android.companion.virtualdevice.flags.Flags.deviceAwareSettingsOverride()) {

            IntArray deviceIds = getVirtualDeviceIds();

            deviceIds.add(Context.DEVICE_ID_DEFAULT);

            settingsToSend = new Bundle(deviceIds.size());



            // Global settings and device config values do not vary across devices, so we can

            // populate them once.

            Bundle globalSettingsBundle = new Bundle(sGlobalSettingToTypeMap.size());

            populateSettings(context, globalSettingsBundle, sGlobalSettingToTypeMap);

            Bundle deviceConfigBundle = new Bundle();

            Bundle deviceConfigBundle = new Bundle(sDeviceConfigEntries.size());

            populateSettingsFromDeviceConfig(deviceConfigBundle);



            deviceIds.add(Context.DEVICE_ID_DEFAULT);

            for (int deviceId : deviceIds) {

                final Bundle deviceBundle = new Bundle();

            for (int i = 0; i < deviceIds.size(); i++) {

                int deviceId = deviceIds.get(i);

                Context deviceContext = null;

                if (deviceId == Context.DEVICE_ID_DEFAULT) {

                    deviceContext = context;

                } else {

                    try {

                        deviceContext = context.createDeviceContext(deviceId);

                    } catch (IllegalArgumentException e) {

                    Slog.w(TAG, "Exception during Context#createDeviceContext", e);

                        Slogf.e(TAG, e, "Exception during Context#createDeviceContext "

                                + "for deviceId: %d", deviceId);

                        continue;

                    }

                }



                if (deviceContext != null) {

                if (DEBUG) {

                    Slogf.d(TAG, "Populating settings for deviceId: %d", deviceId);

                }

                Bundle deviceBundle = new Bundle();

                populateSettings(deviceContext, deviceBundle, sSecureSettingToTypeMap);

                populateSettings(deviceContext, deviceBundle, sSystemSettingToTypeMap);



                    // Copy global settings and device config values, as they don't vary across

                    // devices.

                // Copy global settings and device config values.

                deviceBundle.putAll(globalSettingsBundle);

                deviceBundle.putAll(deviceConfigBundle);



                    mCoreSettings.putBundle(String.valueOf(deviceId), deviceBundle);

                }

                settingsToSend.putBundle(String.valueOf(deviceId), deviceBundle);

            }

        } else {

            populateSettings(context, mCoreSettings, sSecureSettingToTypeMap);

            populateSettings(context, mCoreSettings, sSystemSettingToTypeMap);

            populateSettings(context, mCoreSettings, sGlobalSettingToTypeMap);

            populateSettingsFromDeviceConfig(mCoreSettings);

            if (DEBUG) {

                Slogf.d(TAG, "Populating settings for default device");

            }



            // For non-device-aware case, populate all settings into the single bundle.

            settingsToSend = new Bundle();

            populateSettings(context, settingsToSend, sSecureSettingToTypeMap);

            populateSettings(context, settingsToSend, sSystemSettingToTypeMap);

            populateSettings(context, settingsToSend, sGlobalSettingToTypeMap);

            populateSettingsFromDeviceConfig(settingsToSend);

        }



        synchronized (mLock) {

            mCoreSettings.clear();

            mCoreSettings.putAll(settingsToSend);

        }



        mActivityManagerService.onCoreSettingsChange(mCoreSettings);

        mActivityManagerService.onCoreSettingsChange(settingsToSend);

    }



    private void beginObserveCoreSettings() {

        ContentResolver cr = mActivityManagerService.mContext.getContentResolver();



        for (String setting : sSecureSettingToTypeMap.keySet()) {

            Uri uri = Settings.Secure.getUriFor(setting);

            mActivityManagerService.mContext.getContentResolver().registerContentObserver(

                    uri, false, this);

            cr.registerContentObserver(uri, false, this);

        }



        for (String setting : sSystemSettingToTypeMap.keySet()) {

            Uri uri = Settings.System.getUriFor(setting);

            mActivityManagerService.mContext.getContentResolver().registerContentObserver(

                    uri, false, this);

            cr.registerContentObserver(uri, false, this);

        }



        for (String setting : sGlobalSettingToTypeMap.keySet()) {

            Uri uri = Settings.Global.getUriFor(setting);

            mActivityManagerService.mContext.getContentResolver().registerContentObserver(

                    uri, false, this);

            cr.registerContentObserver(uri, false, this);

        }



        HashSet<String> deviceConfigNamespaces = new HashSet<>();
@@ -294,6 +343,13 @@ final class CoreSettingsObserver extends ContentObserver { 
        }

    }



    /**

     * Populates the given bundle with settings from the given map.

     *

     * @param context The context to use for retrieving the settings.

     * @param snapshot The bundle to populate.

     * @param map The map of settings to retrieve.

     */

    @VisibleForTesting

    void populateSettings(Context context, Bundle snapshot, Map<String, Class<?>> map) {

        final ContentResolver cr = context.getContentResolver();
@@ -312,6 +368,7 @@ final class CoreSettingsObserver extends ContentObserver { 
                continue;

            }

            Class<?> type = entry.getValue();

            try {

                if (type == String.class) {

                    snapshot.putString(setting, value);

                } else if (type == int.class) {
@@ -321,6 +378,9 @@ final class CoreSettingsObserver extends ContentObserver { 
                } else if (type == long.class) {

                    snapshot.putLong(setting, Long.parseLong(value));

                }

            } catch (NumberFormatException e) {

                Slogf.w(TAG, e, "Couldn't parse %s for %s", value, setting);

            }

        }

    }

services/tests/servicestests/src/com/android/server/am/CoreSettingsObserverTest.java

+27 −22
Original line number Diff line number Diff line
@@ -20,9 +20,9 @@ import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentat 


import static com.android.server.am.ActivityManagerService.Injector;



import static com.google.common.truth.Truth.assertThat;

import static com.google.common.truth.Truth.assertWithMessage;



import static org.junit.Assert.assertEquals;

import static org.mockito.Mockito.anyInt;

import static org.mockito.Mockito.mock;

import static org.mockito.Mockito.when;
@@ -67,6 +67,7 @@ public class CoreSettingsObserverTest { 
    private static final int TEST_INT = 111;

    private static final float TEST_FLOAT = 3.14f;

    private static final String TEST_STRING = "testString";

    private static final float TOLERANCE = 0.001f;



    @Rule

    public ServiceThreadRule mServiceThreadRule = new ServiceThreadRule();
@@ -99,6 +100,7 @@ public class CoreSettingsObserverTest { 
    public void setUp() {

        MockitoAnnotations.initMocks(this);



        // Mock context and content resolver.

        final Context originalContext = getInstrumentation().getTargetContext();

        when(mContext.getApplicationInfo()).thenReturn(originalContext.getApplicationInfo());

        mContentResolver = new MockContentResolver(mContext);
@@ -114,6 +116,7 @@ public class CoreSettingsObserverTest { 
        when(mockTypedArray.length()).thenReturn(1);

        when(mResources.obtainTypedArray(anyInt())).thenReturn(mockTypedArray);



        // Initialize ActivityManagerService and CoreSettingsObserver.

        mAms = new ActivityManagerService(new TestInjector(mContext),

                mServiceThreadRule.getThread());

        mCoreSettingsObserver = new CoreSettingsObserver(mAms);
@@ -127,23 +130,23 @@ public class CoreSettingsObserverTest { 


        final Bundle settingsBundle = getPopulatedBundle();



        assertEquals("Unexpected value of " + TEST_SETTING_SECURE_INT,

                TEST_INT, settingsBundle.getInt(TEST_SETTING_SECURE_INT));

        assertEquals("Unexpected value of " + TEST_SETTING_GLOBAL_FLOAT,

                TEST_FLOAT, settingsBundle.getFloat(TEST_SETTING_GLOBAL_FLOAT), 0);

        assertEquals("Unexpected value of " + TEST_SETTING_SYSTEM_STRING,

                TEST_STRING, settingsBundle.getString(TEST_SETTING_SYSTEM_STRING));

        // Assert that the bundle contains the correct values.

        assertThat(settingsBundle.getInt(TEST_SETTING_SECURE_INT)).isEqualTo(TEST_INT);

        assertThat(settingsBundle.getFloat(TEST_SETTING_GLOBAL_FLOAT)).isWithin(TOLERANCE).of(

                TEST_FLOAT);

        assertThat(settingsBundle.getString(TEST_SETTING_SYSTEM_STRING)).isEqualTo(TEST_STRING);

    }



    @Test

    public void testPopulateSettings_settingNotSet() {

        final Bundle settingsBundle = getPopulatedBundle();



        assertWithMessage("Bundle should not contain " + TEST_SETTING_SECURE_INT)

        // Assert that the bundle does not contain any of the test settings.

        assertWithMessage("Bundle should not contain %s", TEST_SETTING_SECURE_INT)

                .that(settingsBundle.keySet()).doesNotContain(TEST_SETTING_SECURE_INT);

        assertWithMessage("Bundle should not contain " + TEST_SETTING_GLOBAL_FLOAT)

        assertWithMessage("Bundle should not contain %s", TEST_SETTING_GLOBAL_FLOAT)

                .that(settingsBundle.keySet()).doesNotContain(TEST_SETTING_GLOBAL_FLOAT);

        assertWithMessage("Bundle should not contain " + TEST_SETTING_SYSTEM_STRING)

        assertWithMessage("Bundle should not contain %s", TEST_SETTING_SYSTEM_STRING)

                .that(settingsBundle.keySet()).doesNotContain(TEST_SETTING_SYSTEM_STRING);

    }
@@ -153,24 +156,26 @@ public class CoreSettingsObserverTest { 
        Settings.Global.putFloat(mContentResolver, TEST_SETTING_GLOBAL_FLOAT, TEST_FLOAT);

        Settings.System.putString(mContentResolver, TEST_SETTING_SYSTEM_STRING, TEST_STRING);



        // Trigger the observer and get the populated bundle.

        Bundle settingsBundle = getPopulatedBundle();



        assertEquals("Unexpected value of " + TEST_SETTING_SECURE_INT,

                TEST_INT, settingsBundle.getInt(TEST_SETTING_SECURE_INT));

        assertEquals("Unexpected value of " + TEST_SETTING_GLOBAL_FLOAT,

                TEST_FLOAT, settingsBundle.getFloat(TEST_SETTING_GLOBAL_FLOAT), 0);

        assertEquals("Unexpected value of " + TEST_SETTING_SYSTEM_STRING,

                TEST_STRING, settingsBundle.getString(TEST_SETTING_SYSTEM_STRING));

        // Assert that the bundle contains the correct values initially.

        assertThat(settingsBundle.getInt(TEST_SETTING_SECURE_INT)).isEqualTo(TEST_INT);

        assertThat(settingsBundle.getFloat(TEST_SETTING_GLOBAL_FLOAT)).isWithin(TOLERANCE).of(

                TEST_FLOAT);

        assertThat(settingsBundle.getString(TEST_SETTING_SYSTEM_STRING)).isEqualTo(TEST_STRING);



        // Delete one of the settings.

        Settings.Global.putString(mContentResolver, TEST_SETTING_GLOBAL_FLOAT, null);

        // Trigger the observer again.

        settingsBundle = getPopulatedBundle();



        assertWithMessage("Bundle should not contain " + TEST_SETTING_GLOBAL_FLOAT)

        // Assert that the deleted setting is no longer in the bundle.

        assertWithMessage("Bundle should not contain %s", TEST_SETTING_GLOBAL_FLOAT)

                .that(settingsBundle.keySet()).doesNotContain(TEST_SETTING_GLOBAL_FLOAT);

        assertEquals("Unexpected value of " + TEST_SETTING_SECURE_INT,

                TEST_INT, settingsBundle.getInt(TEST_SETTING_SECURE_INT));

        assertEquals("Unexpected value of " + TEST_SETTING_SYSTEM_STRING,

                TEST_STRING, settingsBundle.getString(TEST_SETTING_SYSTEM_STRING));

        // Assert that the other settings remain.

        assertThat(settingsBundle.getInt(TEST_SETTING_SECURE_INT)).isEqualTo(TEST_INT);

        assertThat(settingsBundle.getString(TEST_SETTING_SYSTEM_STRING)).isEqualTo(TEST_STRING);

    }



    @Test
@@ -182,7 +187,7 @@ public class CoreSettingsObserverTest { 


    private Bundle getPopulatedBundle() {

        mCoreSettingsObserver.onChange(false);

        return mCoreSettingsObserver.getCoreSettingsLocked().getBundle(

        return mCoreSettingsObserver.getCoreSettings().getBundle(

                String.valueOf(Context.DEVICE_ID_DEFAULT));

    }