Merge "Remove custom PluginHandler from PluginInstanceManager" into sc-v2-dev am: 67cf37f39b

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/15478380

Change-Id: If0c6b135e9567eb989332a0d283677febb17047f

packages/SystemUI/shared/src/com/android/systemui/shared/plugins/PluginInstanceManager.java

@@ -28,9 +28,6 @@ import android.content.pm.PackageManager.NameNotFoundException;
 import android.content.pm.ResolveInfo;
 import android.content.res.Resources;
 import android.net.Uri;
-import android.os.Handler;
-import android.os.Looper;
-import android.os.Message;
 import android.util.ArraySet;
 import android.util.Log;
 import android.view.LayoutInflater;
@@ -61,21 +58,22 @@ public class PluginInstanceManager<T extends Plugin> {
     private final VersionInfo mVersion;
 
     @VisibleForTesting
-    final PluginHandler mPluginHandler;
+    private final ArrayList<PluginInfo<T>> mPlugins = new ArrayList<>();
     private final boolean isDebuggable;
     private final PackageManager mPm;
     private final PluginManagerImpl mManager;
     private final ArraySet<String> mWhitelistedPlugins = new ArraySet<>();
     private final PluginInitializer mInitializer;
     private final Executor mMainExecutor;
+    private final Executor mBgExecutor;
 
     PluginInstanceManager(Context context, PackageManager pm, String action,
             PluginListener<T> listener, boolean allowMultiple, Executor mainExecutor,
-            Looper looper, VersionInfo version, PluginManagerImpl manager, boolean debuggable,
+            Executor bgExecutor, VersionInfo version, PluginManagerImpl manager, boolean debuggable,
             String[] pluginWhitelist, PluginInitializer initializer) {
         mInitializer = initializer;
         mMainExecutor = mainExecutor;
-        mPluginHandler = new PluginHandler(looper);
+        mBgExecutor = bgExecutor;
         mManager = manager;
         mContext = context;
         mPm = pm;
@@ -89,29 +87,29 @@ public class PluginInstanceManager<T extends Plugin> {
 
     public void loadAll() {
         if (DEBUG) Log.d(TAG, "startListening");
-        mPluginHandler.sendEmptyMessage(PluginHandler.QUERY_ALL);
+        mBgExecutor.execute(this::queryAll);
     }
 
     public void destroy() {
         if (DEBUG) Log.d(TAG, "stopListening");
-        ArrayList<PluginInfo<T>> plugins = new ArrayList<>(mPluginHandler.mPlugins);
+        ArrayList<PluginInfo<T>> plugins = new ArrayList<>(mPlugins);
         for (PluginInfo<T> pluginInfo : plugins) {
             mMainExecutor.execute(() -> onPluginDisconnected(pluginInfo.mPlugin));
         }
     }
 
     public void onPackageRemoved(String pkg) {
-        mPluginHandler.obtainMessage(PluginHandler.REMOVE_PKG, pkg).sendToTarget();
+        mBgExecutor.execute(() -> removePkg(pkg));
     }
 
     public void onPackageChange(String pkg) {
-        mPluginHandler.obtainMessage(PluginHandler.REMOVE_PKG, pkg).sendToTarget();
+        mBgExecutor.execute(() -> removePkg(pkg));
-        mPluginHandler.obtainMessage(PluginHandler.QUERY_PKG, pkg).sendToTarget();
+        mBgExecutor.execute(() -> queryPkg(pkg));
     }
 
     public boolean checkAndDisable(String className) {
         boolean disableAny = false;
-        ArrayList<PluginInfo<T>> plugins = new ArrayList<>(mPluginHandler.mPlugins);
+        ArrayList<PluginInfo<T>> plugins = new ArrayList<>(mPlugins);
         for (PluginInfo<T> info : plugins) {
             if (className.startsWith(info.mPackage)) {
                 disableAny |= disable(info, PluginEnabler.DISABLED_FROM_EXPLICIT_CRASH);
@@ -121,7 +119,7 @@ public class PluginInstanceManager<T extends Plugin> {
     }
 
     public boolean disableAll() {
-        ArrayList<PluginInfo<T>> plugins = new ArrayList<>(mPluginHandler.mPlugins);
+        ArrayList<PluginInfo<T>> plugins = new ArrayList<>(mPlugins);
         boolean disabledAny = false;
         for (int i = 0; i < plugins.size(); i++) {
             disabledAny |= disable(plugins.get(i), PluginEnabler.DISABLED_FROM_SYSTEM_CRASH);
@@ -164,7 +162,7 @@ public class PluginInstanceManager<T extends Plugin> {
     }
 
     <C> boolean dependsOn(Plugin p, Class<C> cls) {
-        ArrayList<PluginInfo<T>> plugins = new ArrayList<>(mPluginHandler.mPlugins);
+        ArrayList<PluginInfo<T>> plugins = new ArrayList<>(mPlugins);
         for (PluginInfo<T> info : plugins) {
             if (info.mPlugin.getClass().getName().equals(p.getClass().getName())) {
                 return info.mVersion != null && info.mVersion.hasClass(cls);
@@ -201,21 +199,7 @@ public class PluginInstanceManager<T extends Plugin> {
         }
     }
 
-    private class PluginHandler extends Handler {
+    private void queryAll() {
-        private static final int QUERY_ALL = 1;
-        private static final int QUERY_PKG = 2;
-        private static final int REMOVE_PKG = 3;
-
-        private final ArrayList<PluginInfo<T>> mPlugins = new ArrayList<>();
-
-        public PluginHandler(Looper looper) {
-            super(looper);
-        }
-
-        @Override
-        public void handleMessage(Message msg) {
-            switch (msg.what) {
-                case QUERY_ALL:
         if (DEBUG) Log.d(TAG, "queryAll " + mAction);
         for (int i = mPlugins.size() - 1; i >= 0; i--) {
             PluginInfo<T> pluginInfo = mPlugins.get(i);
@@ -223,9 +207,9 @@ public class PluginInstanceManager<T extends Plugin> {
         }
         mPlugins.clear();
         handleQueryPlugins(null);
-                    break;
+    }
-                case REMOVE_PKG:
+
-                    String pkg = (String) msg.obj;
+    private void removePkg(String pkg) {
         for (int i = mPlugins.size() - 1; i >= 0; i--) {
             final PluginInfo<T> pluginInfo = mPlugins.get(i);
             if (pluginInfo.mPackage.equals(pkg)) {
@@ -233,19 +217,15 @@ public class PluginInstanceManager<T extends Plugin> {
                 mPlugins.remove(i);
             }
         }
-                    break;
+    }
-                case QUERY_PKG:
+
-                    String p = (String) msg.obj;
+    private void queryPkg(String pkg) {
-                    if (DEBUG) Log.d(TAG, "queryPkg " + p + " " + mAction);
+        if (DEBUG) Log.d(TAG, "queryPkg " + pkg + " " + mAction);
         if (mAllowMultiple || (mPlugins.size() == 0)) {
-                        handleQueryPlugins(p);
+            handleQueryPlugins(pkg);
         } else {
             if (DEBUG) Log.d(TAG, "Too many of " + mAction);
         }
-                    break;
-                default:
-                    super.handleMessage(msg);
-            }
     }
 
         private void handleQueryPlugins(String pkgName) {
@@ -376,7 +356,6 @@ public class PluginInstanceManager<T extends Plugin> {
             }
             return pv;
         }
-    }
 
     /**
      * Construct a {@link PluginInstanceManager}
@@ -385,15 +364,15 @@ public class PluginInstanceManager<T extends Plugin> {
         private final Context mContext;
         private final PackageManager mPackageManager;
         private final Executor mMainExecutor;
-        private final Looper mLooper;
+        private final Executor mBgExecutor;
         private final PluginInitializer mInitializer;
 
         public Factory(Context context, PackageManager packageManager,
-                Executor mainExecutor, Looper looper, PluginInitializer initializer) {
+                Executor mainExecutor, Executor bgExecutor, PluginInitializer initializer) {
             mContext = context;
             mPackageManager = packageManager;
             mMainExecutor = mainExecutor;
-            mLooper = looper;
+            mBgExecutor = bgExecutor;
             mInitializer = initializer;
         }
 
@@ -402,7 +381,7 @@ public class PluginInstanceManager<T extends Plugin> {
                 PluginListener<T> listener, boolean allowMultiple, VersionInfo version,
                 PluginManagerImpl manager, boolean debuggable, String[] pluginWhitelist) {
             return new PluginInstanceManager<>(mContext, mPackageManager, action, listener,
-                    allowMultiple, mMainExecutor, mLooper, version, manager, debuggable,
+                    allowMultiple, mMainExecutor, mBgExecutor, version, manager, debuggable,
                     pluginWhitelist, mInitializer);
         }
     }

packages/SystemUI/src/com/android/systemui/plugins/PluginsModule.java

@@ -21,7 +21,6 @@ import static com.android.systemui.util.concurrency.GlobalConcurrencyModule.PRE_
 import android.content.Context;
 import android.content.pm.PackageManager;
 import android.os.Build;
-import android.os.Looper;
 
 import com.android.systemui.dagger.qualifiers.Main;
 import com.android.systemui.shared.plugins.PluginEnabler;
@@ -71,16 +70,16 @@ public abstract class PluginsModule {
     @Singleton
     static PluginInstanceManager.Factory providePluginInstanceManagerFactory(Context context,
             PackageManager packageManager, @Main Executor mainExecutor,
-            @Named(PLUGIN_THREAD) Looper pluginLooper, PluginInitializer initializer) {
+            @Named(PLUGIN_THREAD) Executor pluginExecutor, PluginInitializer initializer) {
         return new PluginInstanceManager.Factory(
-                context, packageManager, mainExecutor, pluginLooper, initializer);
+                context, packageManager, mainExecutor, pluginExecutor, initializer);
     }
 
     @Provides
     @Singleton
     @Named(PLUGIN_THREAD)
-    static Looper providesPluginLooper(ThreadFactory threadFactory) {
+    static Executor providesPluginExecutor(ThreadFactory threadFactory) {
-        return threadFactory.buildLooperOnNewThread("plugin");
+        return threadFactory.buildExecutorOnNewThread("plugin");
     }
 
     @Provides

packages/SystemUI/tests/src/com/android/systemui/shared/plugins/PluginInstanceManagerTest.java

@@ -38,7 +38,6 @@ import android.content.pm.ApplicationInfo;
 import android.content.pm.PackageManager;
 import android.content.pm.ResolveInfo;
 import android.content.pm.ServiceInfo;
-import android.os.HandlerThread;
 import android.test.suitebuilder.annotation.SmallTest;
 
 import androidx.test.runner.AndroidJUnit4;
@@ -71,7 +70,6 @@ public class PluginInstanceManagerTest extends SysuiTestCase {
     // Static since the plugin needs to be generated by the PluginInstanceManager using newInstance.
     private static Plugin sMockPlugin;
 
-    private HandlerThread mHandlerThread;
     private Context mContextWrapper;
     private PackageManager mMockPm;
     private PluginListener mMockListener;
@@ -86,8 +84,6 @@ public class PluginInstanceManagerTest extends SysuiTestCase {
 
     @Before
     public void setup() throws Exception {
-        mHandlerThread = new HandlerThread("test_thread");
-        mHandlerThread.start();
         mContextWrapper = new MyContextWrapper(getContext());
         mMockPm = mock(PackageManager.class);
         mMockListener = mock(PluginListener.class);
@@ -98,15 +94,14 @@ public class PluginInstanceManagerTest extends SysuiTestCase {
         mMockVersionInfo = mock(VersionInfo.class);
         mInitializer = mock(PluginInitializer.class);
         mPluginInstanceManager = new PluginInstanceManager(mContextWrapper, mMockPm, "myAction",
-                mMockListener, true, mFakeExecutor, mHandlerThread.getLooper(),
+                mMockListener, true, mFakeExecutor, mFakeExecutor,
                 mMockVersionInfo, mMockManager, true, new String[0], mInitializer);
         sMockPlugin = mock(Plugin.class);
         when(sMockPlugin.getVersion()).thenReturn(1);
     }
 
     @After
-    public void tearDown() throws Exception {
+    public void tearDown() {
-        mHandlerThread.quit();
         sMockPlugin = null;
     }
 
@@ -116,7 +111,6 @@ public class PluginInstanceManagerTest extends SysuiTestCase {
                 Collections.emptyList());
         mPluginInstanceManager.loadAll();
 
-        waitForIdleSync(mPluginInstanceManager.mPluginHandler);
         mFakeExecutor.runAllReady();
 
         verify(mMockListener, never()).onPluginConnected(any(), any());
@@ -138,7 +132,6 @@ public class PluginInstanceManagerTest extends SysuiTestCase {
 
         mPluginInstanceManager.destroy();
 
-        waitForIdleSync(mPluginInstanceManager.mPluginHandler);
         mFakeExecutor.runAllReady();
 
 
@@ -156,7 +149,6 @@ public class PluginInstanceManagerTest extends SysuiTestCase {
 
         mPluginInstanceManager.loadAll();
 
-        waitForIdleSync(mPluginInstanceManager.mPluginHandler);
         mFakeExecutor.runAllReady();
 
 
@@ -171,7 +163,6 @@ public class PluginInstanceManagerTest extends SysuiTestCase {
 
         mPluginInstanceManager.onPackageChange("com.android.systemui");
 
-        waitForIdleSync(mPluginInstanceManager.mPluginHandler);
         mFakeExecutor.runAllReady();
 
         // Verify the old one was destroyed.
@@ -188,13 +179,12 @@ public class PluginInstanceManagerTest extends SysuiTestCase {
     public void testNonDebuggable() throws Exception {
         // Create a version that thinks the build is not debuggable.
         mPluginInstanceManager = new PluginInstanceManager(mContextWrapper, mMockPm, "myAction",
-                mMockListener, true, mFakeExecutor, mHandlerThread.getLooper(),
+                mMockListener, true, mFakeExecutor, mFakeExecutor,
                 mMockVersionInfo, mMockManager, false, new String[0], mInitializer);
         setupFakePmQuery();
 
         mPluginInstanceManager.loadAll();
 
-        waitForIdleSync(mPluginInstanceManager.mPluginHandler);
         mFakeExecutor.runAllReady();
 
         // Non-debuggable build should receive no plugins.
@@ -205,14 +195,13 @@ public class PluginInstanceManagerTest extends SysuiTestCase {
     public void testNonDebuggable_whitelist() throws Exception {
         // Create a version that thinks the build is not debuggable.
         mPluginInstanceManager = new PluginInstanceManager(mContextWrapper, mMockPm, "myAction",
-                mMockListener, true, mFakeExecutor, mHandlerThread.getLooper(),
+                mMockListener, true, mFakeExecutor, mFakeExecutor,
                 mMockVersionInfo, mMockManager, false,
                 new String[] {WHITELISTED_PACKAGE}, mInitializer);
         setupFakePmQuery();
 
         mPluginInstanceManager.loadAll();
 
-        waitForIdleSync(mPluginInstanceManager.mPluginHandler);
         mFakeExecutor.runAllReady();
 
         // Verify startup lifecycle
@@ -250,7 +239,7 @@ public class PluginInstanceManagerTest extends SysuiTestCase {
     @Test
     public void testDisableWhitelisted() throws Exception {
         mPluginInstanceManager = new PluginInstanceManager(mContextWrapper, mMockPm, "myAction",
-                mMockListener, true, mFakeExecutor, mHandlerThread.getLooper(),
+                mMockListener, true, mFakeExecutor, mFakeExecutor,
                 mMockVersionInfo, mMockManager, false, new String[] {WHITELISTED_PACKAGE},
                 mInitializer);
         createPlugin(); // Get into valid created state.
@@ -288,7 +277,6 @@ public class PluginInstanceManagerTest extends SysuiTestCase {
 
         mPluginInstanceManager.loadAll();
 
-        waitForIdleSync(mPluginInstanceManager.mPluginHandler);
         mFakeExecutor.runAllReady();
     }