Moving PluginManager to dagger
Multiple singletons depend on Plugin which can cause deadlock
if PluginManager is initialized on main thread
Test: presubmit
Bug: 361850561
Bug: 373557167
Flag: EXEMPT dagger
Change-Id: I79f17ac6b78a2ce60df2d27a6e794b9e4eba1b51
diff --git a/quickstep/res/values/config.xml b/quickstep/res/values/config.xml
index 41b2384..db5ff19 100644
--- a/quickstep/res/values/config.xml
+++ b/quickstep/res/values/config.xml
@@ -33,7 +33,6 @@
<string name="taskbar_model_callbacks_factory_class" translatable="false">com.android.launcher3.taskbar.TaskbarModelCallbacksFactory</string>
<string name="taskbar_view_callbacks_factory_class" translatable="false">com.android.launcher3.taskbar.TaskbarViewCallbacksFactory</string>
<string name="launcher_restore_event_logger_class" translatable="false">com.android.quickstep.LauncherRestoreEventLoggerImpl</string>
- <string name="plugin_manager_wrapper_class" translatable="false">com.android.launcher3.uioverrides.plugins.PluginManagerWrapperImpl</string>
<string name="taskbar_edu_tooltip_controller_class" translatable="false">com.android.launcher3.taskbar.TaskbarEduTooltipController</string>
<string name="contextual_edu_manager_class" translatable="false">com.android.quickstep.contextualeducation.SystemContextualEduStatsManager</string>
<string name="nav_handle_long_press_handler_class" translatable="false"></string>
diff --git a/quickstep/src/com/android/launcher3/uioverrides/plugins/PluginManagerWrapperImpl.java b/quickstep/src/com/android/launcher3/uioverrides/plugins/PluginManagerWrapperImpl.java
index 74572c4..3aa1963 100644
--- a/quickstep/src/com/android/launcher3/uioverrides/plugins/PluginManagerWrapperImpl.java
+++ b/quickstep/src/com/android/launcher3/uioverrides/plugins/PluginManagerWrapperImpl.java
@@ -27,6 +27,8 @@
import android.content.pm.ResolveInfo;
import com.android.launcher3.BuildConfig;
+import com.android.launcher3.dagger.ApplicationContext;
+import com.android.launcher3.dagger.LauncherAppSingleton;
import com.android.launcher3.util.PluginManagerWrapper;
import com.android.systemui.plugins.Plugin;
import com.android.systemui.plugins.PluginListener;
@@ -34,7 +36,6 @@
import com.android.systemui.shared.plugins.PluginInstance;
import com.android.systemui.shared.plugins.PluginManagerImpl;
import com.android.systemui.shared.plugins.PluginPrefs;
-import com.android.systemui.shared.system.UncaughtExceptionPreHandlerManager;
import java.io.PrintWriter;
import java.util.ArrayList;
@@ -42,16 +43,17 @@
import java.util.List;
import java.util.Set;
-public class PluginManagerWrapperImpl extends PluginManagerWrapper {
+import javax.inject.Inject;
- private static final UncaughtExceptionPreHandlerManager UNCAUGHT_EXCEPTION_PRE_HANDLER_MANAGER =
- new UncaughtExceptionPreHandlerManager();
+@LauncherAppSingleton
+public class PluginManagerWrapperImpl extends PluginManagerWrapper {
private final Context mContext;
private final PluginManagerImpl mPluginManager;
private final PluginEnablerImpl mPluginEnabler;
- public PluginManagerWrapperImpl(Context c) {
+ @Inject
+ public PluginManagerWrapperImpl(@ApplicationContext Context c) {
mContext = c;
mPluginEnabler = new PluginEnablerImpl(c);
List<String> privilegedPlugins = Collections.emptyList();
@@ -64,9 +66,11 @@
c.getSystemService(NotificationManager.class), mPluginEnabler,
privilegedPlugins, instanceFactory);
+ // Use null preHandlerManager, as the handler is never unregistered which can cause leaks
+ // when using multiple dagger graphs.
mPluginManager = new PluginManagerImpl(c, instanceManagerFactory,
BuildConfig.IS_DEBUG_DEVICE,
- UNCAUGHT_EXCEPTION_PRE_HANDLER_MANAGER, mPluginEnabler,
+ null /* preHandlerManager */, mPluginEnabler,
new PluginPrefs(c), privilegedPlugins);
}
diff --git a/quickstep/src/com/android/quickstep/dagger/QuickStepModule.java b/quickstep/src/com/android/quickstep/dagger/QuickStepModule.java
index 08345b8..ab77a7f 100644
--- a/quickstep/src/com/android/quickstep/dagger/QuickStepModule.java
+++ b/quickstep/src/com/android/quickstep/dagger/QuickStepModule.java
@@ -15,8 +15,14 @@
*/
package com.android.quickstep.dagger;
+import com.android.launcher3.uioverrides.plugins.PluginManagerWrapperImpl;
+import com.android.launcher3.util.PluginManagerWrapper;
+
+import dagger.Binds;
import dagger.Module;
@Module
-public class QuickStepModule {
+public abstract class QuickStepModule {
+
+ @Binds abstract PluginManagerWrapper bindPluginManagerWrapper(PluginManagerWrapperImpl impl);
}
diff --git a/res/values/config.xml b/res/values/config.xml
index 701e64a..504218b 100644
--- a/res/values/config.xml
+++ b/res/values/config.xml
@@ -85,7 +85,6 @@
<string name="wallpaper_picker_package" translatable="false"></string>
<string name="local_colors_extraction_class" translatable="false"></string>
<string name="search_session_manager_class" translatable="false"></string>
- <string name="plugin_manager_wrapper_class" translatable="false"></string>
<!-- Scalable Grid configuration -->
<!-- This is a float because it is converted to dp later in DeviceProfile -->
diff --git a/src/com/android/launcher3/dagger/LauncherBaseAppComponent.java b/src/com/android/launcher3/dagger/LauncherBaseAppComponent.java
index c3508b7..4da7c27 100644
--- a/src/com/android/launcher3/dagger/LauncherBaseAppComponent.java
+++ b/src/com/android/launcher3/dagger/LauncherBaseAppComponent.java
@@ -20,6 +20,7 @@
import com.android.launcher3.pm.InstallSessionHelper;
import com.android.launcher3.util.DaggerSingletonTracker;
+import com.android.launcher3.util.PluginManagerWrapper;
import com.android.launcher3.util.ScreenOnTracker;
import com.android.launcher3.util.SettingsCache;
import com.android.launcher3.widget.custom.CustomWidgetManager;
@@ -40,6 +41,7 @@
ScreenOnTracker getScreenOnTracker();
SettingsCache getSettingsCache();
CustomWidgetManager getCustomWidgetManager();
+ PluginManagerWrapper getPluginManagerWrapper();
/** Builder for LauncherBaseAppComponent. */
interface Builder {
diff --git a/src/com/android/launcher3/util/PluginManagerWrapper.java b/src/com/android/launcher3/util/PluginManagerWrapper.java
index b27aa12..5b28570 100644
--- a/src/com/android/launcher3/util/PluginManagerWrapper.java
+++ b/src/com/android/launcher3/util/PluginManagerWrapper.java
@@ -15,32 +15,39 @@
*/
package com.android.launcher3.util;
-import static com.android.launcher3.util.MainThreadInitializedObject.forOverride;
+import androidx.annotation.AnyThread;
-import com.android.launcher3.R;
+import com.android.launcher3.dagger.LauncherAppSingleton;
+import com.android.launcher3.dagger.LauncherBaseAppComponent;
import com.android.systemui.plugins.Plugin;
import com.android.systemui.plugins.PluginListener;
import java.io.PrintWriter;
-public class PluginManagerWrapper implements ResourceBasedOverride, SafeCloseable {
+import javax.inject.Inject;
- public static final MainThreadInitializedObject<PluginManagerWrapper> INSTANCE =
- forOverride(PluginManagerWrapper.class, R.string.plugin_manager_wrapper_class);
+@LauncherAppSingleton
+public class PluginManagerWrapper{
+ public static final DaggerSingletonObject<PluginManagerWrapper> INSTANCE =
+ new DaggerSingletonObject<>(LauncherBaseAppComponent::getPluginManagerWrapper);
+
+ @Inject
+ public PluginManagerWrapper() { }
+
+ @AnyThread
public <T extends Plugin> void addPluginListener(
PluginListener<T> listener, Class<T> pluginClass) {
addPluginListener(listener, pluginClass, false);
}
+ @AnyThread
public <T extends Plugin> void addPluginListener(
PluginListener<T> listener, Class<T> pluginClass, boolean allowMultiple) {
}
+ @AnyThread
public void removePluginListener(PluginListener<? extends Plugin> listener) { }
- @Override
- public void close() { }
-
public void dump(PrintWriter pw) { }
}
diff --git a/src/com/android/launcher3/widget/custom/CustomWidgetManager.java b/src/com/android/launcher3/widget/custom/CustomWidgetManager.java
index 0778172..4aeac76 100644
--- a/src/com/android/launcher3/widget/custom/CustomWidgetManager.java
+++ b/src/com/android/launcher3/widget/custom/CustomWidgetManager.java
@@ -41,7 +41,6 @@
import com.android.launcher3.util.ExecutorUtil;
import com.android.launcher3.util.PackageUserKey;
import com.android.launcher3.util.PluginManagerWrapper;
-import com.android.launcher3.util.SafeCloseable;
import com.android.launcher3.widget.LauncherAppWidgetHostView;
import com.android.launcher3.widget.LauncherAppWidgetProviderInfo;
import com.android.systemui.plugins.CustomWidgetPlugin;
@@ -61,7 +60,7 @@
* CustomWidgetManager handles custom widgets implemented as a plugin.
*/
@LauncherAppSingleton
-public class CustomWidgetManager implements PluginListener<CustomWidgetPlugin>, SafeCloseable {
+public class CustomWidgetManager implements PluginListener<CustomWidgetPlugin> {
public static final DaggerSingletonObject<CustomWidgetManager> INSTANCE =
new DaggerSingletonObject<>(LauncherBaseAppComponent::getCustomWidgetManager);
@@ -75,12 +74,14 @@
private final @NonNull AppWidgetManager mAppWidgetManager;
@Inject
- CustomWidgetManager(@ApplicationContext Context context, DaggerSingletonTracker tracker) {
- this(context, AppWidgetManager.getInstance(context), tracker);
+ CustomWidgetManager(@ApplicationContext Context context, PluginManagerWrapper pluginManager,
+ DaggerSingletonTracker tracker) {
+ this(context, pluginManager, AppWidgetManager.getInstance(context), tracker);
}
@VisibleForTesting
CustomWidgetManager(@ApplicationContext Context context,
+ PluginManagerWrapper pluginManager,
@NonNull AppWidgetManager widgetManager,
DaggerSingletonTracker tracker) {
mContext = context;
@@ -88,11 +89,9 @@
mPlugins = new HashMap<>();
mCustomWidgets = new ArrayList<>();
+ pluginManager.addPluginListener(this, CustomWidgetPlugin.class, true);
ExecutorUtil.executeSyncOnMainOrFail(() -> {
- PluginManagerWrapper.INSTANCE.get(context)
- .addPluginListener(this, CustomWidgetPlugin.class, true);
-
if (enableSmartspaceAsAWidget()) {
for (String s: context.getResources()
.getStringArray(R.array.custom_widget_providers)) {
@@ -110,16 +109,11 @@
}
}
- tracker.addCloseable(this);
+ tracker.addCloseable(() -> pluginManager.removePluginListener(this));
});
}
@Override
- public void close() {
- PluginManagerWrapper.INSTANCE.get(mContext).removePluginListener(this);
- }
-
- @Override
public void onPluginConnected(CustomWidgetPlugin plugin, Context context) {
List<AppWidgetProviderInfo> providers = mAppWidgetManager
.getInstalledProvidersForProfile(Process.myUserHandle());
diff --git a/tests/multivalentTests/src/com/android/launcher3/widget/custom/CustomWidgetManagerTest.kt b/tests/multivalentTests/src/com/android/launcher3/widget/custom/CustomWidgetManagerTest.kt
index 82f56b8..1c25db9 100644
--- a/tests/multivalentTests/src/com/android/launcher3/widget/custom/CustomWidgetManagerTest.kt
+++ b/tests/multivalentTests/src/com/android/launcher3/widget/custom/CustomWidgetManagerTest.kt
@@ -26,17 +26,19 @@
import com.android.launcher3.util.DaggerSingletonTracker
import com.android.launcher3.util.LauncherModelHelper.SandboxModelContext
import com.android.launcher3.util.PluginManagerWrapper
+import com.android.launcher3.util.SafeCloseable
import com.android.launcher3.util.WidgetUtils
import com.android.launcher3.widget.LauncherAppWidgetHostView
import com.android.launcher3.widget.LauncherAppWidgetProviderInfo
import com.android.systemui.plugins.CustomWidgetPlugin
-import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
+import org.mockito.ArgumentCaptor
+import org.mockito.Captor
import org.mockito.Mock
import org.mockito.Mockito.mock
import org.mockito.MockitoAnnotations
@@ -60,16 +62,12 @@
@Mock private lateinit var mockAppWidgetManager: AppWidgetManager
@Mock private lateinit var tracker: DaggerSingletonTracker
+ @Captor private lateinit var closableCaptor: ArgumentCaptor<SafeCloseable>
+
@Before
fun setUp() {
MockitoAnnotations.initMocks(this)
- context.putObject(PluginManagerWrapper.INSTANCE, pluginManager)
- underTest = CustomWidgetManager(context, mockAppWidgetManager, tracker)
- }
-
- @After
- fun tearDown() {
- underTest.close()
+ underTest = CustomWidgetManager(context, pluginManager, mockAppWidgetManager, tracker)
}
@Test
@@ -80,7 +78,8 @@
@Test
fun close_widget_manager_should_remove_plugin_listener() {
- underTest.close()
+ verify(tracker).addCloseable(closableCaptor.capture())
+ closableCaptor.allValues.forEach(SafeCloseable::close)
verify(pluginManager).removePluginListener(same(underTest))
}