From 03262298bd5939f24439b29915a0d738099310bc Mon Sep 17 00:00:00 2001 From: Dave Mankoff Date: Thu, 5 Aug 2021 20:25:21 -0400 Subject: [PATCH] Daggerize Plugin code. Restructure the dependencies within all the plugin code and make it suitable for injection. There is still a lot of gnarly code in the plugin architecture, and this is the first step towards cleaning it up. This is also the first step towards being able to load plugins (like the FlagReaderPlugin) _before_ the dagger graph is setup. With this change, it is theoretically possible to directly construct the plugin infrastructure, rather than relying on calls to Dependency#get and other SystemUI idiosyncracies. Bug: 194781951 Test: manual && atest SystemUITests Change-Id: I04da3d12211d9f9d1a5c2c5cd27b6a2d81c3907e --- .../shared/plugins/PluginInitializer.java | 17 +-- .../shared/plugins/PluginInstanceManager.java | 41 ++++-- .../shared/plugins/PluginManager.java | 5 +- .../shared/plugins/PluginManagerImpl.java | 122 ++++++++---------- .../systemui/dagger/DependencyProvider.java | 16 +-- .../android/systemui/dagger/GlobalModule.java | 5 +- .../systemui/dagger/GlobalRootComponent.java | 7 - .../plugins/PluginDependencyProvider.java | 14 +- .../systemui/plugins/PluginEnablerImpl.java | 8 +- .../plugins/PluginInitializerImpl.java | 41 ++---- .../systemui/plugins/PluginsModule.java | 108 ++++++++++++++++ .../systemui/tuner/PluginFragment.java | 4 +- .../concurrency/GlobalConcurrencyModule.java | 10 ++ .../plugins/PluginInstanceManagerTest.java | 10 +- .../shared/plugins/PluginManagerTest.java | 61 ++++----- .../utils/leaks/FakePluginManager.java | 6 +- 16 files changed, 274 insertions(+), 201 deletions(-) create mode 100644 packages/SystemUI/src/com/android/systemui/plugins/PluginsModule.java diff --git a/packages/SystemUI/shared/src/com/android/systemui/shared/plugins/PluginInitializer.java b/packages/SystemUI/shared/src/com/android/systemui/shared/plugins/PluginInitializer.java index 42bc1d0ea0ff7..895b6cd96d6fe 100644 --- a/packages/SystemUI/shared/src/com/android/systemui/shared/plugins/PluginInitializer.java +++ b/packages/SystemUI/shared/src/com/android/systemui/shared/plugins/PluginInitializer.java @@ -15,31 +15,20 @@ package com.android.systemui.shared.plugins; import android.content.Context; -import android.os.Looper; /** * Provides necessary components for initializing {@link PluginManagerImpl}. */ public interface PluginInitializer { - Looper getBgLooper(); - /** - * Called from the bg looper during initialization of {@link PluginManagerImpl}. + * Return a list of plugins that don't get disabled when an exception occurs. */ - void onPluginManagerInit(); + String[] getPrivilegedPlugins(Context context); - String[] getWhitelistedPlugins(Context context); - - PluginEnabler getPluginEnabler(Context context); /** - * Called from {@link PluginManagerImpl#handleWtfs()}. + * Called from {@link PluginInstanceManager}. */ void handleWtfs(); - - /** - * Returns if pluging manager should run in debug mode. - */ - boolean isDebuggable(); } diff --git a/packages/SystemUI/shared/src/com/android/systemui/shared/plugins/PluginInstanceManager.java b/packages/SystemUI/shared/src/com/android/systemui/shared/plugins/PluginInstanceManager.java index 2b35bcd9a3ea9..d361661114027 100644 --- a/packages/SystemUI/shared/src/com/android/systemui/shared/plugins/PluginInstanceManager.java +++ b/packages/SystemUI/shared/src/com/android/systemui/shared/plugins/PluginInstanceManager.java @@ -67,17 +67,13 @@ public class PluginInstanceManager { private final PackageManager mPm; private final PluginManagerImpl mManager; private final ArraySet mWhitelistedPlugins = new ArraySet<>(); + private final PluginInitializer mInitializer; - PluginInstanceManager(Context context, String action, PluginListener listener, - boolean allowMultiple, Looper looper, VersionInfo version, PluginManagerImpl manager) { - this(context, context.getPackageManager(), action, listener, allowMultiple, looper, version, - manager, manager.isDebuggable(), manager.getWhitelistedPlugins()); - } - - @VisibleForTesting PluginInstanceManager(Context context, PackageManager pm, String action, PluginListener listener, boolean allowMultiple, Looper looper, VersionInfo version, - PluginManagerImpl manager, boolean debuggable, String[] pluginWhitelist) { + PluginManagerImpl manager, boolean debuggable, String[] pluginWhitelist, + PluginInitializer initializer) { + mInitializer = initializer; mMainHandler = new MainHandler(Looper.getMainLooper()); mPluginHandler = new PluginHandler(looper); mManager = manager; @@ -214,7 +210,7 @@ public class PluginInstanceManager { if (DEBUG) Log.d(TAG, "onPluginConnected"); PluginPrefs.setHasPlugins(mContext); PluginInfo info = (PluginInfo) msg.obj; - mManager.handleWtfs(); + mInitializer.handleWtfs(); if (!(msg.obj instanceof PluginFragment)) { // Only call onDestroy for plugins that aren't fragments, as fragments // will get the onCreate as part of the fragment lifecycle. @@ -417,6 +413,33 @@ public class PluginInstanceManager { } } + /** + * Construct a {@link PluginInstanceManager} + */ + public static class Factory { + private final Context mContext; + private final PackageManager mPackageManager; + private final Looper mLooper; + private final PluginInitializer mInitializer; + + public Factory(Context context, PackageManager packageManager, Looper looper, + PluginInitializer initializer) { + mContext = context; + mPackageManager = packageManager; + mLooper = looper; + mInitializer = initializer; + } + + PluginInstanceManager create( + String action, + PluginListener listener, boolean allowMultiple, VersionInfo version, + PluginManagerImpl manager, boolean debuggable, String[] pluginWhitelist) { + return new PluginInstanceManager<>(mContext, mPackageManager, action, listener, + allowMultiple, mLooper, version, manager, debuggable, pluginWhitelist, + mInitializer); + } + } + public static class PluginContextWrapper extends ContextWrapper { private final ClassLoader mClassLoader; private LayoutInflater mInflater; diff --git a/packages/SystemUI/shared/src/com/android/systemui/shared/plugins/PluginManager.java b/packages/SystemUI/shared/src/com/android/systemui/shared/plugins/PluginManager.java index 3f907a8aa3480..de2a398b36197 100644 --- a/packages/SystemUI/shared/src/com/android/systemui/shared/plugins/PluginManager.java +++ b/packages/SystemUI/shared/src/com/android/systemui/shared/plugins/PluginManager.java @@ -27,7 +27,8 @@ public interface PluginManager { // must be one of the channels created in NotificationChannels.java String NOTIFICATION_CHANNEL_ID = "ALR"; - String[] getWhitelistedPlugins(); + /** Returns plugins that don't get disabled when an exceptoin occurs. */ + String[] getPrivilegedPlugins(); T getOneShotPlugin(Class cls); T getOneShotPlugin(String action, Class cls); @@ -38,7 +39,7 @@ public interface PluginManager { void addPluginListener(String action, PluginListener listener, Class cls); void addPluginListener(String action, PluginListener listener, - Class cls, boolean allowMultiple); + Class cls, boolean allowMultiple); void removePluginListener(PluginListener listener); diff --git a/packages/SystemUI/shared/src/com/android/systemui/shared/plugins/PluginManagerImpl.java b/packages/SystemUI/shared/src/com/android/systemui/shared/plugins/PluginManagerImpl.java index 2b4cdd6cf575d..e4183740ce7a6 100644 --- a/packages/SystemUI/shared/src/com/android/systemui/shared/plugins/PluginManagerImpl.java +++ b/packages/SystemUI/shared/src/com/android/systemui/shared/plugins/PluginManagerImpl.java @@ -30,7 +30,6 @@ import android.content.pm.PackageManager.NameNotFoundException; import android.content.res.Resources; import android.net.Uri; import android.os.Build; -import android.os.Handler; import android.os.Looper; import android.os.SystemProperties; import android.text.TextUtils; @@ -39,7 +38,6 @@ import android.util.ArraySet; import android.util.Log; import android.widget.Toast; -import com.android.internal.annotations.VisibleForTesting; import com.android.internal.messages.nano.SystemMessageProto.SystemMessage; import com.android.systemui.plugins.Plugin; import com.android.systemui.plugins.PluginListener; @@ -56,6 +54,8 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Map; +import java.util.Optional; + /** * @see Plugin */ @@ -64,57 +64,45 @@ public class PluginManagerImpl extends BroadcastReceiver implements PluginManage private static final String TAG = PluginManagerImpl.class.getSimpleName(); static final String DISABLE_PLUGIN = "com.android.systemui.action.DISABLE_PLUGIN"; - private final ArrayMap, PluginInstanceManager> mPluginMap + private final ArrayMap, PluginInstanceManager> mPluginMap = new ArrayMap<>(); private final Map mClassLoaders = new ArrayMap<>(); private final ArraySet mOneShotPackages = new ArraySet<>(); - private final ArraySet mWhitelistedPlugins = new ArraySet<>(); + private final ArraySet mPrivilegedPlugins = new ArraySet<>(); private final Context mContext; - private final PluginInstanceManagerFactory mFactory; + private final PluginInstanceManager.Factory mInstanceManagerFactory; private final boolean mIsDebuggable; private final PluginPrefs mPluginPrefs; private final PluginEnabler mPluginEnabler; - private final PluginInitializer mPluginInitializer; private ClassLoaderFilter mParentClassLoader; private boolean mListening; private boolean mHasOneShot; - private Looper mLooper; - public PluginManagerImpl(Context context, PluginInitializer initializer) { - this(context, new PluginInstanceManagerFactory(), initializer.isDebuggable(), - Thread.getUncaughtExceptionPreHandler(), initializer); - } - - @VisibleForTesting - PluginManagerImpl(Context context, PluginInstanceManagerFactory factory, boolean debuggable, - UncaughtExceptionHandler defaultHandler, final PluginInitializer initializer) { + public PluginManagerImpl(Context context, + PluginInstanceManager.Factory instanceManagerFactory, + boolean debuggable, + Optional defaultHandlerOptional, + PluginEnabler pluginEnabler, + PluginPrefs pluginPrefs, + String[] privilegedPlugins) { mContext = context; - mFactory = factory; - mLooper = initializer.getBgLooper(); + mInstanceManagerFactory = instanceManagerFactory; mIsDebuggable = debuggable; - mWhitelistedPlugins.addAll(Arrays.asList(initializer.getWhitelistedPlugins(mContext))); - mPluginPrefs = new PluginPrefs(mContext); - mPluginEnabler = initializer.getPluginEnabler(mContext); - mPluginInitializer = initializer; + mPrivilegedPlugins.addAll(Arrays.asList(privilegedPlugins)); + mPluginPrefs = pluginPrefs; + mPluginEnabler = pluginEnabler; PluginExceptionHandler uncaughtExceptionHandler = new PluginExceptionHandler( - defaultHandler); + defaultHandlerOptional); Thread.setUncaughtExceptionPreHandler(uncaughtExceptionHandler); - - new Handler(mLooper).post(new Runnable() { - @Override - public void run() { - initializer.onPluginManagerInit(); - } - }); } public boolean isDebuggable() { return mIsDebuggable; } - public String[] getWhitelistedPlugins() { - return mWhitelistedPlugins.toArray(new String[0]); + public String[] getPrivilegedPlugins() { + return mPrivilegedPlugins.toArray(new String[0]); } public PluginEnabler getPluginEnabler() { @@ -138,9 +126,10 @@ public class PluginManagerImpl extends BroadcastReceiver implements PluginManage throw new RuntimeException("Must be called from UI thread"); } // Passing null causes compiler to complain about incompatible (generic) types. - PluginListener dummy = null; - PluginInstanceManager p = mFactory.createPluginInstanceManager(mContext, action, dummy, - false, mLooper, cls, this); + PluginListener dummy = null; + PluginInstanceManager p = mInstanceManagerFactory.create( + action, dummy, false, new VersionInfo().addClass(cls), this, + isDebuggable(), getPrivilegedPlugins()); mPluginPrefs.addAction(action); PluginInfo info = p.getPlugin(); if (info != null) { @@ -167,10 +156,11 @@ public class PluginManagerImpl extends BroadcastReceiver implements PluginManage } public void addPluginListener(String action, PluginListener listener, - Class cls, boolean allowMultiple) { + Class cls, boolean allowMultiple) { mPluginPrefs.addAction(action); - PluginInstanceManager p = mFactory.createPluginInstanceManager(mContext, action, listener, - allowMultiple, mLooper, cls, this); + PluginInstanceManager p = mInstanceManagerFactory.create(action, listener, allowMultiple, + new VersionInfo().addClass(cls), this, isDebuggable(), + getPrivilegedPlugins()); p.loadAll(); synchronized (this) { mPluginMap.put(listener, p); @@ -218,7 +208,7 @@ public class PluginManagerImpl extends BroadcastReceiver implements PluginManage public void onReceive(Context context, Intent intent) { if (Intent.ACTION_USER_UNLOCKED.equals(intent.getAction())) { synchronized (this) { - for (PluginInstanceManager manager : mPluginMap.values()) { + for (PluginInstanceManager manager : mPluginMap.values()) { manager.loadAll(); } } @@ -226,8 +216,8 @@ public class PluginManagerImpl extends BroadcastReceiver implements PluginManage Uri uri = intent.getData(); ComponentName component = ComponentName.unflattenFromString( uri.toString().substring(10)); - if (isPluginWhitelisted(component)) { - // Don't disable whitelisted plugins as they are a part of the OS. + if (isPluginPrivileged(component)) { + // Don't disable privileged plugins as they are a part of the OS. return; } getPluginEnabler().setDisabled(component, PluginEnabler.DISABLED_INVALID_VERSION); @@ -287,11 +277,11 @@ public class PluginManagerImpl extends BroadcastReceiver implements PluginManage } synchronized (this) { if (!Intent.ACTION_PACKAGE_REMOVED.equals(intent.getAction())) { - for (PluginInstanceManager manager : mPluginMap.values()) { + for (PluginInstanceManager manager : mPluginMap.values()) { manager.onPackageChange(pkg); } } else { - for (PluginInstanceManager manager : mPluginMap.values()) { + for (PluginInstanceManager manager : mPluginMap.values()) { manager.onPackageRemoved(pkg); } } @@ -301,8 +291,8 @@ public class PluginManagerImpl extends BroadcastReceiver implements PluginManage /** Returns class loader specific for the given plugin. */ public ClassLoader getClassLoader(ApplicationInfo appInfo) { - if (!mIsDebuggable && !isPluginPackageWhitelisted(appInfo.packageName)) { - Log.w(TAG, "Cannot get class loader for non-whitelisted plugin. Src:" + if (!mIsDebuggable && !isPluginPackagePrivileged(appInfo.packageName)) { + Log.w(TAG, "Cannot get class loader for non-privileged plugin. Src:" + appInfo.sourceDir + ", pkg: " + appInfo.packageName); return null; } @@ -345,32 +335,18 @@ public class PluginManagerImpl extends BroadcastReceiver implements PluginManage return false; } - public void handleWtfs() { - mPluginInitializer.handleWtfs(); - } - public void dump(FileDescriptor fd, PrintWriter pw, String[] args) { synchronized (this) { pw.println(String.format(" plugin map (%d):", mPluginMap.size())); - for (PluginListener listener : mPluginMap.keySet()) { + for (PluginListener listener : mPluginMap.keySet()) { pw.println(String.format(" %s -> %s", listener, mPluginMap.get(listener))); } } } - @VisibleForTesting - public static class PluginInstanceManagerFactory { - public PluginInstanceManager createPluginInstanceManager(Context context, - String action, PluginListener listener, boolean allowMultiple, Looper looper, - Class cls, PluginManagerImpl manager) { - return new PluginInstanceManager(context, action, listener, allowMultiple, looper, - new VersionInfo().addClass(cls), manager); - } - } - - private boolean isPluginPackageWhitelisted(String packageName) { - for (String componentNameOrPackage : mWhitelistedPlugins) { + private boolean isPluginPackagePrivileged(String packageName) { + for (String componentNameOrPackage : mPrivilegedPlugins) { ComponentName componentName = ComponentName.unflattenFromString(componentNameOrPackage); if (componentName != null) { if (componentName.getPackageName().equals(packageName)) { @@ -383,8 +359,8 @@ public class PluginManagerImpl extends BroadcastReceiver implements PluginManage return false; } - private boolean isPluginWhitelisted(ComponentName pluginName) { - for (String componentNameOrPackage : mWhitelistedPlugins) { + private boolean isPluginPrivileged(ComponentName pluginName) { + for (String componentNameOrPackage : mPrivilegedPlugins) { ComponentName componentName = ComponentName.unflattenFromString(componentNameOrPackage); if (componentName != null) { if (componentName.equals(pluginName)) { @@ -417,16 +393,20 @@ public class PluginManagerImpl extends BroadcastReceiver implements PluginManage } private class PluginExceptionHandler implements UncaughtExceptionHandler { - private final UncaughtExceptionHandler mHandler; + private final Optional mExceptionHandlerOptional; - private PluginExceptionHandler(UncaughtExceptionHandler handler) { - mHandler = handler; + private PluginExceptionHandler( + Optional exceptionHandlerOptional) { + mExceptionHandlerOptional = exceptionHandlerOptional; } @Override public void uncaughtException(Thread thread, Throwable throwable) { if (SystemProperties.getBoolean("plugin.debugging", false)) { - mHandler.uncaughtException(thread, throwable); + Throwable finalThrowable = throwable; + mExceptionHandlerOptional.ifPresent( + handler -> handler.uncaughtException(thread, finalThrowable)); + return; } // Search for and disable plugins that may have been involved in this crash. @@ -436,7 +416,7 @@ public class PluginManagerImpl extends BroadcastReceiver implements PluginManage // disable all the plugins, so we can be sure that SysUI is running as // best as possible. synchronized (this) { - for (PluginInstanceManager manager : mPluginMap.values()) { + for (PluginInstanceManager manager : mPluginMap.values()) { disabledAny |= manager.disableAll(); } } @@ -446,7 +426,9 @@ public class PluginManagerImpl extends BroadcastReceiver implements PluginManage } // Run the normal exception handler so we can crash and cleanup our state. - mHandler.uncaughtException(thread, throwable); + Throwable finalThrowable = throwable; + mExceptionHandlerOptional.ifPresent( + handler -> handler.uncaughtException(thread, finalThrowable)); } private boolean checkStack(Throwable throwable) { @@ -454,7 +436,7 @@ public class PluginManagerImpl extends BroadcastReceiver implements PluginManage boolean disabledAny = false; synchronized (this) { for (StackTraceElement element : throwable.getStackTrace()) { - for (PluginInstanceManager manager : mPluginMap.values()) { + for (PluginInstanceManager manager : mPluginMap.values()) { disabledAny |= manager.checkAndDisable(element.getClassName()); } } diff --git a/packages/SystemUI/src/com/android/systemui/dagger/DependencyProvider.java b/packages/SystemUI/src/com/android/systemui/dagger/DependencyProvider.java index 2d200e3f5456e..54a1b5505df8d 100644 --- a/packages/SystemUI/src/com/android/systemui/dagger/DependencyProvider.java +++ b/packages/SystemUI/src/com/android/systemui/dagger/DependencyProvider.java @@ -68,21 +68,15 @@ import com.android.systemui.navigationbar.NavigationBarController; import com.android.systemui.navigationbar.NavigationBarOverlayController; import com.android.systemui.navigationbar.NavigationModeController; import com.android.systemui.navigationbar.TaskbarDelegate; -import com.android.systemui.plugins.PluginInitializerImpl; import com.android.systemui.plugins.statusbar.StatusBarStateController; import com.android.systemui.qs.ReduceBrightColorsController; import com.android.systemui.recents.OverviewProxyService; import com.android.systemui.recents.Recents; import com.android.systemui.settings.UserTracker; -import com.android.systemui.shared.plugins.PluginManager; -import com.android.systemui.shared.plugins.PluginManagerImpl; import com.android.systemui.shared.system.ActivityManagerWrapper; import com.android.systemui.shared.system.DevicePolicyManagerWrapper; import com.android.systemui.shared.system.TaskStackChangeListeners; import com.android.systemui.shared.system.WindowManagerWrapper; -import com.android.unfold.UnfoldTransitionFactory; -import com.android.unfold.UnfoldTransitionProgressProvider; -import com.android.unfold.config.UnfoldTransitionConfig; import com.android.systemui.statusbar.CommandQueue; import com.android.systemui.statusbar.NotificationRemoteInputManager; import com.android.systemui.statusbar.NotificationShadeDepthController; @@ -98,6 +92,9 @@ import com.android.systemui.statusbar.policy.NetworkController; import com.android.systemui.theme.ThemeOverlayApplier; import com.android.systemui.util.leak.LeakDetector; import com.android.systemui.util.settings.SecureSettings; +import com.android.unfold.UnfoldTransitionFactory; +import com.android.unfold.UnfoldTransitionProgressProvider; +import com.android.unfold.config.UnfoldTransitionConfig; import com.android.wm.shell.legacysplitscreen.LegacySplitScreen; import com.android.wm.shell.pip.Pip; @@ -195,13 +192,6 @@ public class DependencyProvider { return new MetricsLogger(); } - /** */ - @Provides - @SysUISingleton - public PluginManager providePluginManager(Context context) { - return new PluginManagerImpl(context, new PluginInitializerImpl()); - } - /** */ @SysUISingleton @Provides diff --git a/packages/SystemUI/src/com/android/systemui/dagger/GlobalModule.java b/packages/SystemUI/src/com/android/systemui/dagger/GlobalModule.java index a89c7acea9846..648f345205db0 100644 --- a/packages/SystemUI/src/com/android/systemui/dagger/GlobalModule.java +++ b/packages/SystemUI/src/com/android/systemui/dagger/GlobalModule.java @@ -23,6 +23,7 @@ import android.util.DisplayMetrics; import com.android.internal.logging.UiEventLogger; import com.android.internal.logging.UiEventLoggerImpl; import com.android.systemui.dagger.qualifiers.TestHarness; +import com.android.systemui.plugins.PluginsModule; import com.android.systemui.util.concurrency.GlobalConcurrencyModule; import javax.inject.Singleton; @@ -47,7 +48,9 @@ import dagger.Provides; */ @Module(includes = { FrameworkServicesModule.class, - GlobalConcurrencyModule.class}) + GlobalConcurrencyModule.class, + PluginsModule.class, +}) public class GlobalModule { /** */ diff --git a/packages/SystemUI/src/com/android/systemui/dagger/GlobalRootComponent.java b/packages/SystemUI/src/com/android/systemui/dagger/GlobalRootComponent.java index d648c949ffc57..a3a45fe7ae40f 100644 --- a/packages/SystemUI/src/com/android/systemui/dagger/GlobalRootComponent.java +++ b/packages/SystemUI/src/com/android/systemui/dagger/GlobalRootComponent.java @@ -18,8 +18,6 @@ package com.android.systemui.dagger; import android.content.Context; -import com.android.systemui.util.concurrency.ThreadFactory; - import javax.inject.Singleton; import dagger.BindsInstance; @@ -55,9 +53,4 @@ public interface GlobalRootComponent { * Builder for a SysUIComponent. */ SysUIComponent.Builder getSysUIComponent(); - - /** - * Build a {@link ThreadFactory}. - */ - ThreadFactory createThreadFactory(); } diff --git a/packages/SystemUI/src/com/android/systemui/plugins/PluginDependencyProvider.java b/packages/SystemUI/src/com/android/systemui/plugins/PluginDependencyProvider.java index ad1e21d7cc451..0b565ea25911b 100644 --- a/packages/SystemUI/src/com/android/systemui/plugins/PluginDependencyProvider.java +++ b/packages/SystemUI/src/com/android/systemui/plugins/PluginDependencyProvider.java @@ -17,25 +17,27 @@ package com.android.systemui.plugins; import android.util.ArrayMap; import com.android.systemui.Dependency; -import com.android.systemui.dagger.SysUISingleton; import com.android.systemui.plugins.PluginDependency.DependencyProvider; import com.android.systemui.shared.plugins.PluginManager; import javax.inject.Inject; +import javax.inject.Singleton; + +import dagger.Lazy; /** */ -@SysUISingleton +@Singleton public class PluginDependencyProvider extends DependencyProvider { private final ArrayMap, Object> mDependencies = new ArrayMap<>(); - private final PluginManager mManager; + private final Lazy mManagerLazy; /** */ @Inject - public PluginDependencyProvider(PluginManager manager) { - mManager = manager; + public PluginDependencyProvider(Lazy managerLazy) { + mManagerLazy = managerLazy; PluginDependency.sProvider = this; } @@ -51,7 +53,7 @@ public class PluginDependencyProvider extends DependencyProvider { @Override T get(Plugin p, Class cls) { - if (!mManager.dependsOn(p, cls)) { + if (!mManagerLazy.get().dependsOn(p, cls)) { throw new IllegalArgumentException(p.getClass() + " does not depend on " + cls); } synchronized (mDependencies) { diff --git a/packages/SystemUI/src/com/android/systemui/plugins/PluginEnablerImpl.java b/packages/SystemUI/src/com/android/systemui/plugins/PluginEnablerImpl.java index 63374150adaad..40f59744e0384 100644 --- a/packages/SystemUI/src/com/android/systemui/plugins/PluginEnablerImpl.java +++ b/packages/SystemUI/src/com/android/systemui/plugins/PluginEnablerImpl.java @@ -22,16 +22,22 @@ import android.content.pm.PackageManager; import com.android.internal.annotations.VisibleForTesting; import com.android.systemui.shared.plugins.PluginEnabler; +import javax.inject.Inject; +import javax.inject.Singleton; + +/** */ +@Singleton public class PluginEnablerImpl implements PluginEnabler { private static final String CRASH_DISABLED_PLUGINS_PREF_FILE = "auto_disabled_plugins_prefs"; - private PackageManager mPm; + private final PackageManager mPm; private final SharedPreferences mAutoDisabledPrefs; public PluginEnablerImpl(Context context) { this(context, context.getPackageManager()); } + @Inject @VisibleForTesting public PluginEnablerImpl(Context context, PackageManager pm) { mAutoDisabledPrefs = context.getSharedPreferences( CRASH_DISABLED_PLUGINS_PREF_FILE, Context.MODE_PRIVATE); diff --git a/packages/SystemUI/src/com/android/systemui/plugins/PluginInitializerImpl.java b/packages/SystemUI/src/com/android/systemui/plugins/PluginInitializerImpl.java index 7f01d6f1ffa3c..654d000ca09e9 100644 --- a/packages/SystemUI/src/com/android/systemui/plugins/PluginInitializerImpl.java +++ b/packages/SystemUI/src/com/android/systemui/plugins/PluginInitializerImpl.java @@ -15,16 +15,17 @@ package com.android.systemui.plugins; import android.content.Context; -import android.os.Build; -import android.os.Looper; import android.util.Log; -import com.android.systemui.Dependency; import com.android.systemui.R; -import com.android.systemui.shared.plugins.PluginEnabler; import com.android.systemui.shared.plugins.PluginInitializer; import com.android.systemui.shared.plugins.PluginManagerImpl; +import javax.inject.Inject; +import javax.inject.Singleton; + +/** */ +@Singleton public class PluginInitializerImpl implements PluginInitializer { /** @@ -33,44 +34,24 @@ public class PluginInitializerImpl implements PluginInitializer { private static final boolean WTFS_SHOULD_CRASH = false; private boolean mWtfsSet; - @Override - public Looper getBgLooper() { - return Dependency.get(Dependency.BG_LOOPER); + @Inject + public PluginInitializerImpl(PluginDependencyProvider dependencyProvider) { + dependencyProvider.allowPluginDependency(ActivityStarter.class); } @Override - public void onPluginManagerInit() { - // Plugin dependencies that don't have another good home can go here, but - // dependencies that have better places to init can happen elsewhere. - Dependency.get(PluginDependencyProvider.class) - .allowPluginDependency(ActivityStarter.class); - } - - @Override - public String[] getWhitelistedPlugins(Context context) { + public String[] getPrivilegedPlugins(Context context) { return context.getResources().getStringArray(R.array.config_pluginWhitelist); } - public PluginEnabler getPluginEnabler(Context context) { - return new PluginEnablerImpl(context); - } @Override public void handleWtfs() { if (WTFS_SHOULD_CRASH && !mWtfsSet) { mWtfsSet = true; - Log.setWtfHandler(new Log.TerribleFailureHandler() { - @Override - public void onTerribleFailure(String tag, Log.TerribleFailure what, - boolean system) { - throw new PluginManagerImpl.CrashWhilePluginActiveException(what); - } + Log.setWtfHandler((tag, what, system) -> { + throw new PluginManagerImpl.CrashWhilePluginActiveException(what); }); } } - - @Override - public boolean isDebuggable() { - return Build.IS_DEBUGGABLE; - } } diff --git a/packages/SystemUI/src/com/android/systemui/plugins/PluginsModule.java b/packages/SystemUI/src/com/android/systemui/plugins/PluginsModule.java new file mode 100644 index 0000000000000..79751075b71fb --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/plugins/PluginsModule.java @@ -0,0 +1,108 @@ +/* + * Copyright (C) 2021 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.systemui.plugins; + +import static com.android.systemui.util.concurrency.GlobalConcurrencyModule.PRE_HANDLER; + +import android.content.Context; +import android.content.pm.PackageManager; +import android.os.Build; +import android.os.Looper; + +import com.android.systemui.shared.plugins.PluginEnabler; +import com.android.systemui.shared.plugins.PluginInitializer; +import com.android.systemui.shared.plugins.PluginInstanceManager; +import com.android.systemui.shared.plugins.PluginManager; +import com.android.systemui.shared.plugins.PluginManagerImpl; +import com.android.systemui.shared.plugins.PluginPrefs; +import com.android.systemui.util.concurrency.ThreadFactory; + +import java.util.Optional; + +import javax.inject.Named; +import javax.inject.Singleton; + +import dagger.Binds; +import dagger.Module; +import dagger.Provides; + +/** + * Dagger Module for code related to plugins. + * + * Covers code both in com.android.systemui.plugins and code in + * com.android.systemui.shared.plugins. + */ +@Module +public abstract class PluginsModule { + public static final String PLUGIN_THREAD = "plugin_thread"; + public static final String PLUGIN_DEBUG = "plugin_debug"; + public static final String PLUGIN_PRIVILEGED = "plugin_privileged"; + + @Provides + @Named(PLUGIN_DEBUG) + static boolean providesPluginDebug() { + return Build.IS_DEBUGGABLE; + } + + @Binds + abstract PluginEnabler bindsPluginEnablerImpl(PluginEnablerImpl impl); + + @Binds + abstract PluginInitializer bindsPluginInitializerImpl(PluginInitializerImpl impl); + + @Provides + @Singleton + static PluginInstanceManager.Factory providePluginInstanceManagerFactory(Context context, + PackageManager packageManager, @Named(PLUGIN_THREAD) Looper pluginLooper, + PluginInitializer initializer) { + return new PluginInstanceManager.Factory( + context, packageManager, pluginLooper, initializer); + } + + @Provides + @Singleton + @Named(PLUGIN_THREAD) + static Looper providesPluginLooper(ThreadFactory threadFactory) { + return threadFactory.buildLooperOnNewThread("plugin"); + } + + @Provides + static PluginManager providesPluginManager( + Context context, + PluginInstanceManager.Factory instanceManagerFactory, + @Named(PLUGIN_DEBUG) boolean debug, + @Named(PRE_HANDLER) + Optional uncaughtExceptionHandlerOptional, + PluginEnabler pluginEnabler, + PluginPrefs pluginPrefs, + @Named(PLUGIN_PRIVILEGED) String[] privilegedPlugins) { + return new PluginManagerImpl(context, instanceManagerFactory, debug, + uncaughtExceptionHandlerOptional, pluginEnabler, pluginPrefs, + privilegedPlugins); + } + + @Provides + static PluginPrefs providesPluginPrefs(Context context) { + return new PluginPrefs(context); + } + + @Provides + @Named(PLUGIN_PRIVILEGED) + static String[] providesPrivilegedPlugins(PluginInitializer initializer, Context context) { + return initializer.getPrivilegedPlugins(context); + } +} diff --git a/packages/SystemUI/src/com/android/systemui/tuner/PluginFragment.java b/packages/SystemUI/src/com/android/systemui/tuner/PluginFragment.java index 0a29e04ce20f5..20857eaba7d48 100644 --- a/packages/SystemUI/src/com/android/systemui/tuner/PluginFragment.java +++ b/packages/SystemUI/src/com/android/systemui/tuner/PluginFragment.java @@ -106,8 +106,8 @@ public class PluginFragment extends PreferenceFragment { PackageManager.MATCH_DISABLED_COMPONENTS | PackageManager.GET_SERVICES); apps.forEach(app -> { if (!plugins.containsKey(app.packageName)) return; - if (ArrayUtils.contains(manager.getWhitelistedPlugins(), app.packageName)) { - // Don't manage whitelisted plugins, they are part of the OS. + if (ArrayUtils.contains(manager.getPrivilegedPlugins(), app.packageName)) { + // Don't manage privileged plugins, they are part of the OS. return; } SwitchPreference pref = new PluginPreference(prefContext, app, mPluginEnabler); diff --git a/packages/SystemUI/src/com/android/systemui/util/concurrency/GlobalConcurrencyModule.java b/packages/SystemUI/src/com/android/systemui/util/concurrency/GlobalConcurrencyModule.java index 1c504961e7155..23641f990d650 100644 --- a/packages/SystemUI/src/com/android/systemui/util/concurrency/GlobalConcurrencyModule.java +++ b/packages/SystemUI/src/com/android/systemui/util/concurrency/GlobalConcurrencyModule.java @@ -22,8 +22,10 @@ import android.os.Looper; import com.android.systemui.dagger.qualifiers.Main; +import java.util.Optional; import java.util.concurrent.Executor; +import javax.inject.Named; import javax.inject.Singleton; import dagger.Binds; @@ -35,6 +37,7 @@ import dagger.Provides; */ @Module public abstract class GlobalConcurrencyModule { + public static final String PRE_HANDLER = "pre_handler"; /** * Binds {@link ThreadFactoryImpl} to {@link ThreadFactory}. @@ -73,4 +76,11 @@ public abstract class GlobalConcurrencyModule { @Binds @Singleton public abstract Execution provideExecution(ExecutionImpl execution); + + /** */ + @Provides + @Named(PRE_HANDLER) + public static Optional providesUncaughtExceptionHandler() { + return Optional.ofNullable(Thread.getUncaughtExceptionPreHandler()); + } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/shared/plugins/PluginInstanceManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/shared/plugins/PluginInstanceManagerTest.java index 325d540ad7417..9a6af58c08dd8 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/shared/plugins/PluginInstanceManagerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/shared/plugins/PluginInstanceManagerTest.java @@ -82,6 +82,7 @@ public class PluginInstanceManagerTest extends SysuiTestCase { private PluginEnabler mMockEnabler; ComponentName mTestPluginComponentName = new ComponentName(WHITELISTED_PACKAGE, TestPlugin.class.getName()); + private PluginInitializer mInitializer; @Before public void setup() throws Exception { @@ -95,9 +96,10 @@ public class PluginInstanceManagerTest extends SysuiTestCase { mMockEnabler = mock(PluginEnabler.class); when(mMockManager.getPluginEnabler()).thenReturn(mMockEnabler); mMockVersionInfo = mock(VersionInfo.class); + mInitializer = mock(PluginInitializer.class); mPluginInstanceManager = new PluginInstanceManager(mContextWrapper, mMockPm, "myAction", mMockListener, true, mHandlerThread.getLooper(), mMockVersionInfo, - mMockManager, true, new String[0]); + mMockManager, true, new String[0], mInitializer); sMockPlugin = mock(Plugin.class); when(sMockPlugin.getVersion()).thenReturn(1); } @@ -194,7 +196,7 @@ public class PluginInstanceManagerTest extends SysuiTestCase { // Create a version that thinks the build is not debuggable. mPluginInstanceManager = new PluginInstanceManager(mContextWrapper, mMockPm, "myAction", mMockListener, true, mHandlerThread.getLooper(), mMockVersionInfo, - mMockManager, false, new String[0]); + mMockManager, false, new String[0], mInitializer); setupFakePmQuery(); mPluginInstanceManager.loadAll(); @@ -211,7 +213,7 @@ public class PluginInstanceManagerTest extends SysuiTestCase { // Create a version that thinks the build is not debuggable. mPluginInstanceManager = new PluginInstanceManager(mContextWrapper, mMockPm, "myAction", mMockListener, true, mHandlerThread.getLooper(), mMockVersionInfo, - mMockManager, false, new String[] {WHITELISTED_PACKAGE}); + mMockManager, false, new String[] {WHITELISTED_PACKAGE}, mInitializer); setupFakePmQuery(); mPluginInstanceManager.loadAll(); @@ -255,7 +257,7 @@ public class PluginInstanceManagerTest extends SysuiTestCase { public void testDisableWhitelisted() throws Exception { mPluginInstanceManager = new PluginInstanceManager(mContextWrapper, mMockPm, "myAction", mMockListener, true, mHandlerThread.getLooper(), mMockVersionInfo, - mMockManager, false, new String[] {WHITELISTED_PACKAGE}); + mMockManager, false, new String[] {WHITELISTED_PACKAGE}, mInitializer); createPlugin(); // Get into valid created state. mPluginInstanceManager.disableAll(); diff --git a/packages/SystemUI/tests/src/com/android/systemui/shared/plugins/PluginManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/shared/plugins/PluginManagerTest.java index 536c043bd7aec..5dee8e51f16e1 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/shared/plugins/PluginManagerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/shared/plugins/PluginManagerTest.java @@ -30,19 +30,14 @@ import android.content.pm.PackageManager; import android.net.Uri; import android.test.suitebuilder.annotation.SmallTest; import android.testing.AndroidTestingRunner; -import android.testing.TestableLooper; import android.testing.TestableLooper.RunWithLooper; import com.android.internal.messages.nano.SystemMessageProto.SystemMessage; -import com.android.systemui.Dependency; import com.android.systemui.SysuiTestCase; import com.android.systemui.plugins.Plugin; -import com.android.systemui.plugins.PluginEnablerImpl; -import com.android.systemui.plugins.PluginInitializerImpl; import com.android.systemui.plugins.PluginListener; import com.android.systemui.plugins.annotations.ProvidesInterface; import com.android.systemui.shared.plugins.PluginInstanceManager.PluginInfo; -import com.android.systemui.shared.plugins.PluginManagerImpl.PluginInstanceManagerFactory; import org.junit.Before; import org.junit.Test; @@ -51,6 +46,7 @@ import org.mockito.ArgumentCaptor; import org.mockito.Mockito; import java.lang.Thread.UncaughtExceptionHandler; +import java.util.Optional; @SmallTest @RunWith(AndroidTestingRunner.class) @@ -59,11 +55,13 @@ public class PluginManagerTest extends SysuiTestCase { private static final String WHITELISTED_PACKAGE = "com.android.systemui"; - private PluginInstanceManagerFactory mMockFactory; + private PluginInstanceManager.Factory mMockFactory; private PluginInstanceManager mMockPluginInstance; private PluginManagerImpl mPluginManager; - private PluginListener mMockListener; + private PluginListener mMockListener; private PackageManager mMockPackageManager; + private PluginEnabler mPluginEnabler; + private PluginPrefs mPluginPrefs; private UncaughtExceptionHandler mRealExceptionHandler; private UncaughtExceptionHandler mMockExceptionHandler; @@ -71,30 +69,23 @@ public class PluginManagerTest extends SysuiTestCase { @Before public void setup() throws Exception { - mDependency.injectTestDependency(Dependency.BG_LOOPER, - TestableLooper.get(this).getLooper()); mRealExceptionHandler = Thread.getUncaughtExceptionPreHandler(); mMockExceptionHandler = mock(UncaughtExceptionHandler.class); - mMockFactory = mock(PluginInstanceManagerFactory.class); + mMockFactory = mock(PluginInstanceManager.Factory.class); mMockPluginInstance = mock(PluginInstanceManager.class); - when(mMockFactory.createPluginInstanceManager(Mockito.any(), Mockito.any(), Mockito.any(), - Mockito.anyBoolean(), Mockito.any(), Mockito.any(), Mockito.any())) + mPluginEnabler = mock(PluginEnabler.class); + mPluginPrefs = mock(PluginPrefs.class); + when(mMockFactory.create(Mockito.any(), Mockito.any(), + Mockito.anyBoolean(), Mockito.any(), Mockito.any(), Mockito.anyBoolean(), + Mockito.any())) .thenReturn(mMockPluginInstance); mMockPackageManager = mock(PackageManager.class); mPluginManager = new PluginManagerImpl( getContext(), mMockFactory, true, - mMockExceptionHandler, new PluginInitializerImpl() { - @Override - public String[] getWhitelistedPlugins(Context context) { - return new String[0]; - } + Optional.of(mMockExceptionHandler), mPluginEnabler, + mPluginPrefs, new String[0]); - @Override - public PluginEnabler getPluginEnabler(Context context) { - return new PluginEnablerImpl(context, mMockPackageManager); - } - }); resetExceptionHandler(); mMockListener = mock(PluginListener.class); } @@ -127,13 +118,10 @@ public class PluginManagerTest extends SysuiTestCase { @Test @RunWithLooper(setAsMainLooper = true) public void testNonDebuggable_noWhitelist() { - mPluginManager = new PluginManagerImpl(getContext(), mMockFactory, false, - mMockExceptionHandler, new PluginInitializerImpl() { - @Override - public String[] getWhitelistedPlugins(Context context) { - return new String[0]; - } - }); + mPluginManager = new PluginManagerImpl( + getContext(), mMockFactory, false, + Optional.of(mMockExceptionHandler), mPluginEnabler, + mPluginPrefs, new String[0]); resetExceptionHandler(); String sourceDir = "myPlugin"; @@ -148,13 +136,10 @@ public class PluginManagerTest extends SysuiTestCase { @Test @RunWithLooper(setAsMainLooper = true) public void testNonDebuggable_whitelistedPkg() { - mPluginManager = new PluginManagerImpl(getContext(), mMockFactory, false, - mMockExceptionHandler, new PluginInitializerImpl() { - @Override - public String[] getWhitelistedPlugins(Context context) { - return new String[] {WHITELISTED_PACKAGE}; - } - }); + mPluginManager = new PluginManagerImpl( + getContext(), mMockFactory, false, + Optional.of(mMockExceptionHandler), mPluginEnabler, + mPluginPrefs, new String[] {WHITELISTED_PACKAGE}); resetExceptionHandler(); String sourceDir = "myPlugin"; @@ -211,9 +196,7 @@ public class PluginManagerTest extends SysuiTestCase { intent.setData(Uri.parse("package://" + testComponent.flattenToString())); mPluginManager.onReceive(mContext, intent); verify(nm).cancel(eq(testComponent.getClassName()), eq(SystemMessage.NOTE_PLUGIN)); - verify(mMockPackageManager).setComponentEnabledSetting(eq(testComponent), - eq(PackageManager.COMPONENT_ENABLED_STATE_DISABLED), - eq(PackageManager.DONT_KILL_APP)); + verify(mPluginEnabler).setDisabled(testComponent, PluginEnabler.DISABLED_INVALID_VERSION); } private void resetExceptionHandler() { diff --git a/packages/SystemUI/tests/src/com/android/systemui/utils/leaks/FakePluginManager.java b/packages/SystemUI/tests/src/com/android/systemui/utils/leaks/FakePluginManager.java index 6d1e6ce9ea336..a7e4382000817 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/utils/leaks/FakePluginManager.java +++ b/packages/SystemUI/tests/src/com/android/systemui/utils/leaks/FakePluginManager.java @@ -22,7 +22,7 @@ import com.android.systemui.shared.plugins.PluginManager; public class FakePluginManager implements PluginManager { - private final BaseLeakChecker mLeakChecker; + private final BaseLeakChecker> mLeakChecker; public FakePluginManager(LeakCheck test) { mLeakChecker = new BaseLeakChecker<>(test, "Plugin"); @@ -30,7 +30,7 @@ public class FakePluginManager implements PluginManager { @Override public void addPluginListener(String action, PluginListener listener, - Class cls, boolean allowMultiple) { + Class cls, boolean allowMultiple) { mLeakChecker.addCallback(listener); } @@ -62,7 +62,7 @@ public class FakePluginManager implements PluginManager { } @Override - public String[] getWhitelistedPlugins() { + public String[] getPrivilegedPlugins() { return new String[0]; }