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

Commit f33c3584 authored by Peter Kalauskas's avatar Peter Kalauskas
Browse files

Cache CoroutineScope for settings APIs

Instead of creating a new CoroutineScope each time a settings API is
called, create one scope and reuse it whenever possible.

Test: m SystemUIGoogle, atest SettingsProxyTest UserSettingsProxyTest
Bug: 330299944
Flag: NONE new async APIs and existing APIs kept unmodified.
Change-Id: I4b660cdcb49e1c618c94b1d6b2cbbf84c4b60dd4
parent 580a71bc
Loading
Loading
Loading
Loading
+5 −5
Original line number Diff line number Diff line
@@ -27,7 +27,7 @@ import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.SmallTest
import com.android.systemui.SysuiTestCase
import com.google.common.truth.Truth.assertThat
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.StandardTestDispatcher
import kotlinx.coroutines.test.TestScope
@@ -57,7 +57,7 @@ class SettingsProxyTest : SysuiTestCase() {
    @Before
    fun setUp() {
        testScope = TestScope(testDispatcher)
        mSettings = FakeSettingsProxy(testDispatcher)
        mSettings = FakeSettingsProxy(testScope)
        mContentObserver = object : ContentObserver(Handler(Looper.getMainLooper())) {}
    }

@@ -382,15 +382,15 @@ class SettingsProxyTest : SysuiTestCase() {
        assertThat(mSettings.getFloat(TEST_SETTING, 2.5F)).isEqualTo(2.5F)
    }

    private class FakeSettingsProxy(val testDispatcher: CoroutineDispatcher) : SettingsProxy {
    private class FakeSettingsProxy(val testScope: CoroutineScope) : SettingsProxy {

        private val mContentResolver = mock(ContentResolver::class.java)
        private val settingToValueMap: MutableMap<String, String?> = mutableMapOf()

        override fun getContentResolver() = mContentResolver

        override val backgroundDispatcher: CoroutineDispatcher
            get() = testDispatcher
        override val settingsScope: CoroutineScope
            get() = testScope

        override fun getUriFor(name: String) =
            Uri.parse(StringBuilder().append("content://settings/").append(name).toString())
+6 −12
Original line number Diff line number Diff line
@@ -28,7 +28,7 @@ import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.SmallTest
import com.android.systemui.SysuiTestCase
import com.google.common.truth.Truth.assertThat
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.launch
import kotlinx.coroutines.test.StandardTestDispatcher
@@ -36,7 +36,6 @@ import kotlinx.coroutines.test.TestScope
import kotlinx.coroutines.test.advanceUntilIdle
import kotlinx.coroutines.test.runTest
import org.junit.Assert.assertThrows
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.Mockito.mock
@@ -51,14 +50,9 @@ class UserSettingsProxyTest : SysuiTestCase() {

    private var userId = MAIN_USER_ID
    private val testDispatcher = StandardTestDispatcher()
    private var mSettings: UserSettingsProxy = FakeUserSettingsProxy({ userId }, testDispatcher)
    private val testScope = TestScope(testDispatcher)
    private var mSettings: UserSettingsProxy = FakeUserSettingsProxy({ userId }, testScope)
    private var mContentObserver = object : ContentObserver(Handler(Looper.getMainLooper())) {}
    private lateinit var testScope: TestScope

    @Before
    fun setUp() {
        testScope = TestScope(testDispatcher)
    }

    @Test
    fun registerContentObserverForUser_inputString_success() =
@@ -555,7 +549,7 @@ class UserSettingsProxyTest : SysuiTestCase() {
     */
    private class FakeUserSettingsProxy(
        override val currentUserProvider: SettingsProxy.CurrentUserIdProvider,
        val testDispatcher: CoroutineDispatcher,
        val testScope: CoroutineScope,
    ) : UserSettingsProxy {

        private val mContentResolver = mock(ContentResolver::class.java)
@@ -567,8 +561,8 @@ class UserSettingsProxyTest : SysuiTestCase() {
        override fun getUriFor(name: String) =
            Uri.parse(StringBuilder().append(URI_PREFIX).append(name).toString())

        override val backgroundDispatcher: CoroutineDispatcher
            get() = testDispatcher
        override val settingsScope: CoroutineScope
            get() = testScope

        override fun getStringForUser(name: String, userHandle: Int) =
            userIdToSettingsValueMap[userHandle]?.get(name) ?: ""
+6 −6
Original line number Diff line number Diff line
@@ -25,7 +25,7 @@ import android.provider.Settings;

import com.android.systemui.util.settings.SettingsSingleThreadBackground;

import kotlinx.coroutines.CoroutineDispatcher;
import kotlinx.coroutines.CoroutineScope;

import javax.inject.Inject;

@@ -33,13 +33,13 @@ import javax.inject.Inject;
@SuppressLint("StaticSettingsProvider")
class GlobalSettingsImpl implements GlobalSettings {
    private final ContentResolver mContentResolver;
    private final CoroutineDispatcher mBgDispatcher;
    private final CoroutineScope mSettingsScope;

    @Inject
    GlobalSettingsImpl(ContentResolver contentResolver,
            @SettingsSingleThreadBackground CoroutineDispatcher bgDispatcher) {
            @SettingsSingleThreadBackground CoroutineScope settingsScope) {
        mContentResolver = contentResolver;
        mBgDispatcher = bgDispatcher;
        mSettingsScope = settingsScope;
    }

    @NonNull
@@ -56,8 +56,8 @@ class GlobalSettingsImpl implements GlobalSettings {

    @NonNull
    @Override
    public CoroutineDispatcher getBackgroundDispatcher() {
        return mBgDispatcher;
    public CoroutineScope getSettingsScope() {
        return mSettingsScope;
    }

    @Override
+6 −6
Original line number Diff line number Diff line
@@ -23,23 +23,23 @@ import android.provider.Settings;

import com.android.systemui.util.settings.SettingsSingleThreadBackground;

import kotlinx.coroutines.CoroutineDispatcher;
import kotlinx.coroutines.CoroutineScope;

import javax.inject.Inject;

class SecureSettingsImpl implements SecureSettings {
    private final ContentResolver mContentResolver;
    private final CurrentUserIdProvider mCurrentUserProvider;
    private final CoroutineDispatcher mBgDispatcher;
    private final CoroutineScope mSettingsScope;

    @Inject
    SecureSettingsImpl(
            ContentResolver contentResolver,
            CurrentUserIdProvider currentUserProvider,
            @SettingsSingleThreadBackground CoroutineDispatcher bgDispatcher) {
            @SettingsSingleThreadBackground CoroutineScope settingsScope) {
        mContentResolver = contentResolver;
        mCurrentUserProvider = currentUserProvider;
        mBgDispatcher = bgDispatcher;
        mSettingsScope = settingsScope;
    }

    @NonNull
@@ -62,8 +62,8 @@ class SecureSettingsImpl implements SecureSettings {

    @NonNull
    @Override
    public CoroutineDispatcher getBackgroundDispatcher() {
        return mBgDispatcher;
    public CoroutineScope getSettingsScope() {
        return mSettingsScope;
    }

    @Override
+29 −20
Original line number Diff line number Diff line
@@ -24,7 +24,9 @@ import androidx.annotation.AnyThread
import androidx.annotation.WorkerThread
import com.android.app.tracing.TraceUtils.trace
import com.android.app.tracing.coroutines.launchTraced as launch
import com.android.systemui.coroutines.newTracingContext
import com.android.app.tracing.coroutines.nameCoroutine
import kotlin.coroutines.CoroutineContext
import kotlin.coroutines.EmptyCoroutineContext
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.withContext
@@ -47,11 +49,14 @@ interface SettingsProxy {
    /** Returns the [ContentResolver] this instance was constructed with. */
    fun getContentResolver(): ContentResolver

    /**
     * Returns the background [CoroutineDispatcher] that the async APIs will use for a specific
     * implementation.
     */
    val backgroundDispatcher: CoroutineDispatcher
    /** Returns the [CoroutineScope] that the async APIs will use. */
    val settingsScope: CoroutineScope

    @OptIn(ExperimentalStdlibApi::class)
    fun settingsDispatcherContext(name: String): CoroutineContext {
        return (settingsScope.coroutineContext[CoroutineDispatcher] ?: EmptyCoroutineContext) +
            nameCoroutine(name)
    }

    /**
     * Construct the content URI for a particular name/value pair, useful for monitoring changes
@@ -82,7 +87,7 @@ interface SettingsProxy {
     * wish to synchronize execution.
     */
    suspend fun registerContentObserver(name: String, settingsObserver: ContentObserver) {
        withContext(backgroundDispatcher) {
        withContext(settingsDispatcherContext("registerContentObserver-A")) {
            registerContentObserverSync(getUriFor(name), settingsObserver)
        }
    }
@@ -94,7 +99,7 @@ interface SettingsProxy {
     */
    @AnyThread
    fun registerContentObserverAsync(name: String, settingsObserver: ContentObserver) =
        CoroutineScope(backgroundDispatcher + newTracingContext("SettingsProxy-A")).launch {
        settingsScope.launch("registerContentObserverAsync-A") {
            registerContentObserverSync(getUriFor(name), settingsObserver)
        }

@@ -111,7 +116,7 @@ interface SettingsProxy {
        settingsObserver: ContentObserver,
        @WorkerThread registered: Runnable,
    ) =
        CoroutineScope(backgroundDispatcher + newTracingContext("SettingsProxy-B")).launch {
        settingsScope.launch("registerContentObserverAsync-B") {
            registerContentObserverSync(getUriFor(name), settingsObserver)
            registered.run()
        }
@@ -134,7 +139,9 @@ interface SettingsProxy {
     * wish to synchronize execution.
     */
    suspend fun registerContentObserver(uri: Uri, settingsObserver: ContentObserver) {
        withContext(backgroundDispatcher) { registerContentObserverSync(uri, settingsObserver) }
        withContext(settingsDispatcherContext("registerContentObserver-B")) {
            registerContentObserverSync(uri, settingsObserver)
        }
    }

    /**
@@ -144,7 +151,7 @@ interface SettingsProxy {
     */
    @AnyThread
    fun registerContentObserverAsync(uri: Uri, settingsObserver: ContentObserver) =
        CoroutineScope(backgroundDispatcher + newTracingContext("SettingsProxy-C")).launch {
        settingsScope.launch("registerContentObserverAsync-C") {
            registerContentObserverSync(uri, settingsObserver)
        }

@@ -161,7 +168,7 @@ interface SettingsProxy {
        settingsObserver: ContentObserver,
        @WorkerThread registered: Runnable,
    ) =
        CoroutineScope(backgroundDispatcher + newTracingContext("SettingsProxy-D")).launch {
        settingsScope.launch("registerContentObserverAsync-D") {
            registerContentObserverSync(uri, settingsObserver)
            registered.run()
        }
@@ -190,7 +197,7 @@ interface SettingsProxy {
        notifyForDescendants: Boolean,
        settingsObserver: ContentObserver,
    ) {
        withContext(backgroundDispatcher) {
        withContext(settingsDispatcherContext("registerContentObserver-C")) {
            registerContentObserverSync(getUriFor(name), notifyForDescendants, settingsObserver)
        }
    }
@@ -206,7 +213,7 @@ interface SettingsProxy {
        notifyForDescendants: Boolean,
        settingsObserver: ContentObserver,
    ) =
        CoroutineScope(backgroundDispatcher + newTracingContext("SettingsProxy-E")).launch {
        settingsScope.launch("registerContentObserverAsync-E") {
            registerContentObserverSync(getUriFor(name), notifyForDescendants, settingsObserver)
        }

@@ -224,7 +231,7 @@ interface SettingsProxy {
        settingsObserver: ContentObserver,
        @WorkerThread registered: Runnable,
    ) =
        CoroutineScope(backgroundDispatcher + newTracingContext("SettingsProxy-F")).launch {
        settingsScope.launch("registerContentObserverAsync-F") {
            registerContentObserverSync(getUriFor(name), notifyForDescendants, settingsObserver)
            registered.run()
        }
@@ -259,7 +266,7 @@ interface SettingsProxy {
        notifyForDescendants: Boolean,
        settingsObserver: ContentObserver,
    ) {
        withContext(backgroundDispatcher) {
        withContext(settingsDispatcherContext("registerContentObserver-D")) {
            registerContentObserverSync(uri, notifyForDescendants, settingsObserver)
        }
    }
@@ -275,7 +282,7 @@ interface SettingsProxy {
        notifyForDescendants: Boolean,
        settingsObserver: ContentObserver,
    ) =
        CoroutineScope(backgroundDispatcher + newTracingContext("SettingsProxy-G")).launch {
        settingsScope.launch("registerContentObserverAsync-G") {
            registerContentObserverSync(uri, notifyForDescendants, settingsObserver)
        }

@@ -293,7 +300,7 @@ interface SettingsProxy {
        settingsObserver: ContentObserver,
        @WorkerThread registered: Runnable,
    ) =
        CoroutineScope(backgroundDispatcher + newTracingContext("SettingsProxy-H")).launch {
        settingsScope.launch("registerContentObserverAsync-H") {
            registerContentObserverSync(uri, notifyForDescendants, settingsObserver)
            registered.run()
        }
@@ -319,7 +326,9 @@ interface SettingsProxy {
     * async block if they wish to synchronize execution.
     */
    suspend fun unregisterContentObserver(settingsObserver: ContentObserver) {
        withContext(backgroundDispatcher) { unregisterContentObserverSync(settingsObserver) }
        withContext(settingsDispatcherContext("unregisterContentObserver")) {
            unregisterContentObserverSync(settingsObserver)
        }
    }

    /**
@@ -330,7 +339,7 @@ interface SettingsProxy {
     */
    @AnyThread
    fun unregisterContentObserverAsync(settingsObserver: ContentObserver) =
        CoroutineScope(backgroundDispatcher + newTracingContext("SettingsProxy-I")).launch {
        settingsScope.launch("unregisterContentObserverAsync") {
            unregisterContentObserver(settingsObserver)
        }

Loading