From 26a5f65afdc0d5dde1ec5e2b7eb06b9b9168b2e8 Mon Sep 17 00:00:00 2001 From: Anushree Ganjam Date: Fri, 13 Sep 2024 15:03:11 -0700 Subject: [PATCH] Make sandboxContext extend LauncherApplication (4/n) See https://docs.google.com/drawings/d/1JHFi_nhmQt2xPT1N3FB_1mnaRK5TVqKZ9-fSl3EA7sU/edit?usp=sharing and https://docs.google.com/drawings/d/1bx4WURP4uHZGzZ1bWQgpw701jkTkVqlNfA02Yt-ZtSI/edit?usp=sharing&resourcekey=0-oySjsnaCsOSrNIPqqEa0gw for design details. We need to make SandboxContext extend LauncherApplication because we want create MainThreadInitializedObjects in SandboxContext's AppComponent scope. Since MainThreadInitiliazedObjects are closed in SandboxContext's OnDestroy() , we need to replicate same thing using dagger as well. - DaggerSingletonObject is same as MainThreadInitializedObject but is used for fetching the dagger created singletons so that we can avoid major refactors for accessing singletons. This will be deleted soon. - DaggerSingletonTracker to track dagger created singletons and call close() on those singleton objects created in SandboxContext scope. - Annotate the singleton object SettingsChangeLogger constructor with @Inject and execute the statements in Main thread. - Added createSandboxContextForTest(only for Test) to avoid creation of dagger component in test. As follow up, I will delete this method and introduce fakeDaggerComponents in test. Bug: 361850561 Test: Manual Flag: NONE Dagger Integration Change-Id: I2d3762ea64e53baa4de190790568aec750b54201 --- quickstep/dagger/LauncherAppComponent.java | 3 +- .../quickstep/dagger/QuickStepModule.java | 4 +- .../dagger/QuickstepBaseAppComponent.java | 33 +++++++++++ .../logging/SettingsChangeLogger.java | 42 +++++++++----- .../logging/SettingsChangeLoggerTest.kt | 8 ++- .../launcher3/LauncherApplication.java | 15 ++++- .../dagger/LauncherBaseAppComponent.java | 3 + .../launcher3/util/DaggerSingletonObject.java | 44 ++++++++++++++ .../util/DaggerSingletonTracker.java | 57 +++++++++++++++++++ .../android/launcher3/util/ExecutorUtil.java | 27 +++++---- .../util/MainThreadInitializedObject.java | 8 ++- 11 files changed, 205 insertions(+), 39 deletions(-) create mode 100644 quickstep/src/com/android/quickstep/dagger/QuickstepBaseAppComponent.java create mode 100644 src/com/android/launcher3/util/DaggerSingletonObject.java create mode 100644 src/com/android/launcher3/util/DaggerSingletonTracker.java rename quickstep/src/com/android/quickstep/logging/LoggingModule.java => src/com/android/launcher3/util/ExecutorUtil.java (52%) diff --git a/quickstep/dagger/LauncherAppComponent.java b/quickstep/dagger/LauncherAppComponent.java index bd6008e70d..068f01cc5c 100644 --- a/quickstep/dagger/LauncherAppComponent.java +++ b/quickstep/dagger/LauncherAppComponent.java @@ -18,6 +18,7 @@ package com.android.launcher3.dagger; import com.android.quickstep.dagger.QuickStepModule; +import com.android.quickstep.dagger.QuickstepBaseAppComponent; import dagger.Component; @@ -26,7 +27,7 @@ import dagger.Component; */ @LauncherAppSingleton @Component(modules = QuickStepModule.class) -public interface LauncherAppComponent extends LauncherBaseAppComponent { +public interface LauncherAppComponent extends QuickstepBaseAppComponent { /** Builder for quickstep LauncherAppComponent. */ @Component.Builder interface Builder extends LauncherBaseAppComponent.Builder { diff --git a/quickstep/src/com/android/quickstep/dagger/QuickStepModule.java b/quickstep/src/com/android/quickstep/dagger/QuickStepModule.java index db29636c36..08345b85f6 100644 --- a/quickstep/src/com/android/quickstep/dagger/QuickStepModule.java +++ b/quickstep/src/com/android/quickstep/dagger/QuickStepModule.java @@ -15,10 +15,8 @@ */ package com.android.quickstep.dagger; -import com.android.quickstep.logging.LoggingModule; - import dagger.Module; -@Module(includes = {LoggingModule.class}) +@Module public class QuickStepModule { } diff --git a/quickstep/src/com/android/quickstep/dagger/QuickstepBaseAppComponent.java b/quickstep/src/com/android/quickstep/dagger/QuickstepBaseAppComponent.java new file mode 100644 index 0000000000..f2d571554d --- /dev/null +++ b/quickstep/src/com/android/quickstep/dagger/QuickstepBaseAppComponent.java @@ -0,0 +1,33 @@ +/* + * Copyright (C) 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.quickstep.dagger; + +import com.android.launcher3.dagger.LauncherAppComponent; +import com.android.launcher3.dagger.LauncherBaseAppComponent; +import com.android.quickstep.logging.SettingsChangeLogger; + +/** + * Launcher Quickstep base component for Dagger injection. + * + * This class is not actually annotated as a Dagger component, since it is not used directly as one. + * Doing so generates unnecessary code bloat. + * + * See {@link LauncherAppComponent} for the one actually used. + */ +public interface QuickstepBaseAppComponent extends LauncherBaseAppComponent { + SettingsChangeLogger getSettingsChangeLogger(); +} diff --git a/quickstep/src/com/android/quickstep/logging/SettingsChangeLogger.java b/quickstep/src/com/android/quickstep/logging/SettingsChangeLogger.java index 717f6c879b..c4ba5edf64 100644 --- a/quickstep/src/com/android/quickstep/logging/SettingsChangeLogger.java +++ b/quickstep/src/com/android/quickstep/logging/SettingsChangeLogger.java @@ -43,16 +43,20 @@ import androidx.annotation.VisibleForTesting; import com.android.launcher3.LauncherPrefs; import com.android.launcher3.R; +import com.android.launcher3.dagger.ApplicationContext; import com.android.launcher3.logging.InstanceId; import com.android.launcher3.logging.StatsLogManager; import com.android.launcher3.logging.StatsLogManager.StatsLogger; import com.android.launcher3.model.DeviceGridState; +import com.android.launcher3.util.DaggerSingletonObject; +import com.android.launcher3.util.DaggerSingletonTracker; import com.android.launcher3.util.DisplayController; import com.android.launcher3.util.DisplayController.Info; -import com.android.launcher3.util.MainThreadInitializedObject; +import com.android.launcher3.util.ExecutorUtil; import com.android.launcher3.util.NavigationMode; import com.android.launcher3.util.SafeCloseable; import com.android.launcher3.util.SettingsCache; +import com.android.quickstep.dagger.QuickstepBaseAppComponent; import org.xmlpull.v1.XmlPullParser; import org.xmlpull.v1.XmlPullParserException; @@ -60,6 +64,8 @@ import org.xmlpull.v1.XmlPullParserException; import java.io.IOException; import java.util.Optional; +import javax.inject.Inject; + /** * Utility class to log launcher settings changes */ @@ -70,8 +76,8 @@ public class SettingsChangeLogger implements /** * Singleton instance */ - public static MainThreadInitializedObject INSTANCE = - new MainThreadInitializedObject<>(SettingsChangeLogger::new); + public static DaggerSingletonObject INSTANCE = + new DaggerSingletonObject<>(QuickstepBaseAppComponent::getSettingsChangeLogger); private static final String TAG = "SettingsChangeLogger"; private static final String BOOLEAN_PREF = "SwitchPreference"; @@ -84,25 +90,31 @@ public class SettingsChangeLogger implements private StatsLogManager.LauncherEvent mNotificationDotsEvent; private StatsLogManager.LauncherEvent mHomeScreenSuggestionEvent; - private SettingsChangeLogger(Context context) { - this(context, StatsLogManager.newInstance(context)); + @Inject + SettingsChangeLogger(@ApplicationContext Context context, DaggerSingletonTracker tracker) { + this(context, StatsLogManager.newInstance(context), tracker); } @VisibleForTesting - SettingsChangeLogger(Context context, StatsLogManager statsLogManager) { + SettingsChangeLogger(Context context, StatsLogManager statsLogManager, + DaggerSingletonTracker tracker) { mContext = context; mStatsLogManager = statsLogManager; mLoggablePrefs = loadPrefKeys(context); - DisplayController.INSTANCE.get(context).addChangeListener(this); - mNavMode = DisplayController.getNavigationMode(context); - getPrefs(context).registerOnSharedPreferenceChangeListener(this); - getDevicePrefs(context).registerOnSharedPreferenceChangeListener(this); + ExecutorUtil.executeSyncOnMainOrFail(() -> { + DisplayController.INSTANCE.get(context).addChangeListener(this); + mNavMode = DisplayController.getNavigationMode(context); - SettingsCache mSettingsCache = SettingsCache.INSTANCE.get(context); - mSettingsCache.register(NOTIFICATION_BADGING_URI, - this::onNotificationDotsChanged); - onNotificationDotsChanged(mSettingsCache.getValue(NOTIFICATION_BADGING_URI)); + getPrefs(context).registerOnSharedPreferenceChangeListener(this); + getDevicePrefs(context).registerOnSharedPreferenceChangeListener(this); + + SettingsCache settingsCache = SettingsCache.INSTANCE.get(context); + settingsCache.register(NOTIFICATION_BADGING_URI, + this::onNotificationDotsChanged); + onNotificationDotsChanged(settingsCache.getValue(NOTIFICATION_BADGING_URI)); + tracker.addCloseable(this); + }); } private static ArrayMap loadPrefKeys(Context context) { @@ -209,6 +221,8 @@ public class SettingsChangeLogger implements public void close() { getPrefs(mContext).unregisterOnSharedPreferenceChangeListener(this); getDevicePrefs(mContext).unregisterOnSharedPreferenceChangeListener(this); + SettingsCache settingsCache = SettingsCache.INSTANCE.get(mContext); + settingsCache.unregister(NOTIFICATION_BADGING_URI, this::onNotificationDotsChanged); } @VisibleForTesting diff --git a/quickstep/tests/multivalentTests/src/com/android/quickstep/logging/SettingsChangeLoggerTest.kt b/quickstep/tests/multivalentTests/src/com/android/quickstep/logging/SettingsChangeLoggerTest.kt index 7c48ea489c..0a607744d0 100644 --- a/quickstep/tests/multivalentTests/src/com/android/quickstep/logging/SettingsChangeLoggerTest.kt +++ b/quickstep/tests/multivalentTests/src/com/android/quickstep/logging/SettingsChangeLoggerTest.kt @@ -34,6 +34,7 @@ import com.android.launcher3.logging.StatsLogManager.LauncherEvent.LAUNCHER_NAVI import com.android.launcher3.logging.StatsLogManager.LauncherEvent.LAUNCHER_NOTIFICATION_DOT_ENABLED import com.android.launcher3.logging.StatsLogManager.LauncherEvent.LAUNCHER_THEMED_ICON_DISABLED import com.android.launcher3.states.RotationHelper.ALLOW_ROTATION_PREFERENCE_KEY +import com.android.launcher3.util.DaggerSingletonTracker import com.google.common.truth.Truth.assertThat import org.junit.After import org.junit.Before @@ -62,6 +63,7 @@ class SettingsChangeLoggerTest { @Mock private lateinit var mMockLogger: StatsLogManager.StatsLogger @Captor private lateinit var mEventCaptor: ArgumentCaptor + @Mock private lateinit var mTracker: DaggerSingletonTracker private var mDefaultThemedIcons = false private var mDefaultAllowRotation = false @@ -79,7 +81,7 @@ class SettingsChangeLoggerTest { // To match the default value of ALLOW_ROTATION LauncherPrefs.get(mContext).put(item = ALLOW_ROTATION, value = false) - mSystemUnderTest = SettingsChangeLogger(mContext, mStatsLogManager) + mSystemUnderTest = SettingsChangeLogger(mContext, mStatsLogManager, mTracker) } @After @@ -90,7 +92,7 @@ class SettingsChangeLoggerTest { @Test fun loggingPrefs_correctDefaultValue() { - val systemUnderTest = SettingsChangeLogger(mContext, mStatsLogManager) + val systemUnderTest = SettingsChangeLogger(mContext, mStatsLogManager, mTracker) assertThat(systemUnderTest.loggingPrefs[ALLOW_ROTATION_PREFERENCE_KEY]!!.defaultValue) .isFalse() @@ -117,7 +119,7 @@ class SettingsChangeLoggerTest { LauncherPrefs.get(mContext).put(item = ALLOW_ROTATION, value = true) // This a new object so the values of mLoggablePrefs will be different - SettingsChangeLogger(mContext, mStatsLogManager).logSnapshot(mInstanceId) + SettingsChangeLogger(mContext, mStatsLogManager, mTracker).logSnapshot(mInstanceId) verify(mMockLogger, atLeastOnce()).log(mEventCaptor.capture()) val capturedEvents = mEventCaptor.allValues diff --git a/src/com/android/launcher3/LauncherApplication.java b/src/com/android/launcher3/LauncherApplication.java index 490186a524..4c82e5676f 100644 --- a/src/com/android/launcher3/LauncherApplication.java +++ b/src/com/android/launcher3/LauncherApplication.java @@ -18,6 +18,7 @@ package com.android.launcher3; import android.app.Application; import com.android.launcher3.dagger.DaggerLauncherAppComponent; +import com.android.launcher3.dagger.LauncherAppComponent; import com.android.launcher3.dagger.LauncherBaseAppComponent; /** @@ -30,10 +31,18 @@ public class LauncherApplication extends Application { public void onCreate() { super.onCreate(); MainProcessInitializer.initialize(this); - mAppComponent = DaggerLauncherAppComponent.builder().appContext(this).build(); + initDagger(); } - public LauncherBaseAppComponent getAppComponent() { - return mAppComponent; + public LauncherAppComponent getAppComponent() { + // Since supertype setters will return a supertype.builder and @Component.Builder types + // must not have any generic types. + // We need to cast mAppComponent to {@link LauncherAppComponent} since appContext() + // method is defined in the super class LauncherBaseComponent#Builder. + return (LauncherAppComponent) mAppComponent; + } + + protected void initDagger() { + mAppComponent = DaggerLauncherAppComponent.builder().appContext(this).build(); } } diff --git a/src/com/android/launcher3/dagger/LauncherBaseAppComponent.java b/src/com/android/launcher3/dagger/LauncherBaseAppComponent.java index 1a59d82266..0a50e8bc9e 100644 --- a/src/com/android/launcher3/dagger/LauncherBaseAppComponent.java +++ b/src/com/android/launcher3/dagger/LauncherBaseAppComponent.java @@ -18,6 +18,8 @@ package com.android.launcher3.dagger; import android.content.Context; +import com.android.launcher3.util.DaggerSingletonTracker; + import dagger.BindsInstance; /** @@ -29,6 +31,7 @@ import dagger.BindsInstance; * See {@link LauncherAppComponent} for the one actually used by AOSP. */ public interface LauncherBaseAppComponent { + DaggerSingletonTracker getDaggerSingletonTracker(); /** Builder for LauncherBaseAppComponent. */ interface Builder { @BindsInstance Builder appContext(@ApplicationContext Context context); diff --git a/src/com/android/launcher3/util/DaggerSingletonObject.java b/src/com/android/launcher3/util/DaggerSingletonObject.java new file mode 100644 index 0000000000..b8cf2ae77a --- /dev/null +++ b/src/com/android/launcher3/util/DaggerSingletonObject.java @@ -0,0 +1,44 @@ +/* + * Copyright (C) 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.launcher3.util; + +import android.content.Context; + +import com.android.launcher3.LauncherApplication; +import com.android.launcher3.dagger.LauncherAppComponent; + +import java.util.function.Function; + +/** + * A class to provide DaggerSingleton objects in a traditional way for + * {@link MainThreadInitializedObject}. + * We should delete this class at the end and use @Inject to get dagger provided singletons. + */ + +public class DaggerSingletonObject { + private final Function mFunction; + + public DaggerSingletonObject(Function function) { + mFunction = function; + } + + public T get(Context context) { + LauncherAppComponent component = + ((LauncherApplication) context.getApplicationContext()).getAppComponent(); + return mFunction.apply(component); + } +} diff --git a/src/com/android/launcher3/util/DaggerSingletonTracker.java b/src/com/android/launcher3/util/DaggerSingletonTracker.java new file mode 100644 index 0000000000..2946da1d0c --- /dev/null +++ b/src/com/android/launcher3/util/DaggerSingletonTracker.java @@ -0,0 +1,57 @@ +/* + * Copyright (C) 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.launcher3.util; + +import com.android.launcher3.dagger.LauncherAppSingleton; + +import java.util.ArrayList; + +import javax.inject.Inject; + +/** + * A tracker class for keeping track of Dagger created singletons. + * Dagger will take care of creating singletons. But we should take care of unregistering callbacks + * if at all registered during singleton construction. + * All singletons should be declared as SafeCloseable so that we can call close() method. + */ +@LauncherAppSingleton +public class DaggerSingletonTracker implements SafeCloseable { + + private final ArrayList mLauncherAppSingletons = new ArrayList<>(); + + @Inject + DaggerSingletonTracker() { + } + + /** + * Adds the SafeCloseable Singletons to the mLauncherAppSingletons list. + * This helps to track the singletons and close them appropriately. + * See {@link DaggerSingletonTracker#close()} and + * {@link MainThreadInitializedObject.SandboxContext#onDestroy()} + */ + public void addCloseable(SafeCloseable closeable) { + mLauncherAppSingletons.add(closeable); + } + + @Override + public void close() { + // Destroy in reverse order + for (int i = mLauncherAppSingletons.size() - 1; i >= 0; i--) { + mLauncherAppSingletons.get(i).close(); + } + } +} diff --git a/quickstep/src/com/android/quickstep/logging/LoggingModule.java b/src/com/android/launcher3/util/ExecutorUtil.java similarity index 52% rename from quickstep/src/com/android/quickstep/logging/LoggingModule.java rename to src/com/android/launcher3/util/ExecutorUtil.java index 8fdf3c7edf..efc0eec194 100644 --- a/quickstep/src/com/android/quickstep/logging/LoggingModule.java +++ b/src/com/android/launcher3/util/ExecutorUtil.java @@ -14,21 +14,24 @@ * limitations under the License. */ -package com.android.quickstep.logging; +package com.android.launcher3.util; -import android.content.Context; +import static com.android.launcher3.util.Executors.MAIN_EXECUTOR; -import com.android.launcher3.dagger.ApplicationContext; -import com.android.launcher3.dagger.LauncherAppSingleton; +import android.os.Looper; -import dagger.Module; -import dagger.Provides; +import java.util.concurrent.ExecutionException; -@Module -public class LoggingModule { - @Provides - @LauncherAppSingleton - SettingsChangeLogger provideSettingsChangeLogger(@ApplicationContext Context context) { - return SettingsChangeLogger.INSTANCE.get(context); +public final class ExecutorUtil { + + /** + * Executes runnable on {@link Looper#getMainLooper()}, otherwise fails with an exception. + */ + public static void executeSyncOnMainOrFail(Runnable runnable) { + try { + MAIN_EXECUTOR.submit(runnable).get(); + } catch (InterruptedException | ExecutionException e) { + throw new RuntimeException(e); + } } } diff --git a/src/com/android/launcher3/util/MainThreadInitializedObject.java b/src/com/android/launcher3/util/MainThreadInitializedObject.java index 63f14bd6ea..e12ccbcf70 100644 --- a/src/com/android/launcher3/util/MainThreadInitializedObject.java +++ b/src/com/android/launcher3/util/MainThreadInitializedObject.java @@ -18,13 +18,13 @@ package com.android.launcher3.util; import static com.android.launcher3.util.Executors.MAIN_EXECUTOR; import android.content.Context; -import android.content.ContextWrapper; import android.os.Looper; import android.util.Log; import androidx.annotation.UiThread; import androidx.annotation.VisibleForTesting; +import com.android.launcher3.LauncherApplication; import com.android.launcher3.util.ResourceBasedOverride.Overrides; import java.util.ArrayList; @@ -118,7 +118,7 @@ public class MainThreadInitializedObject { * Abstract Context which allows custom implementations for * {@link MainThreadInitializedObject} providers */ - public static class SandboxContext extends ContextWrapper implements SandboxApplication { + public static class SandboxContext extends LauncherApplication implements SandboxApplication { private static final String TAG = "SandboxContext"; @@ -129,7 +129,8 @@ public class MainThreadInitializedObject { private boolean mDestroyed = false; public SandboxContext(Context base) { - super(base); + attachBaseContext(base); + initDagger(); } @Override @@ -138,6 +139,7 @@ public class MainThreadInitializedObject { } public void onDestroy() { + getAppComponent().getDaggerSingletonTracker().close(); synchronized (mDestroyLock) { // Destroy in reverse order for (int i = mOrderedObjects.size() - 1; i >= 0; i--) {