From b898e57082e5950d47ff753c54d0819827dcdccc Mon Sep 17 00:00:00 2001
From: Hawkwood Glazier <jglazier@google.com>
Date: Tue, 25 Apr 2023 21:03:35 +0000
Subject: [PATCH] Unload the duplicate unused managers when they conflict

Duplicate versions of several clocks are being included in the
build and loaded by the plugin manager. This prevents those
duplicates, which are already ignored, from continuing to consume
the majority of their memory. I'll need to follow up to understand
why they're being included and loaded in the first place.

Bug: 270860591
Bug: 278073250
Test: Checked device profile
Change-Id: Ia642f640b6bec382bf672d4437f56b5424e9347d
---
 .../systemui/shared/clocks/ClockRegistry.kt   |  1 +
 .../shared/plugins/PluginActionManager.java   | 23 ++++++++++---------
 .../shared/clocks/ClockRegistryTest.kt        |  6 ++++-
 3 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/packages/SystemUI/customization/src/com/android/systemui/shared/clocks/ClockRegistry.kt b/packages/SystemUI/customization/src/com/android/systemui/shared/clocks/ClockRegistry.kt
index f57432c073c19..b0c02407873c3 100644
--- a/packages/SystemUI/customization/src/com/android/systemui/shared/clocks/ClockRegistry.kt
+++ b/packages/SystemUI/customization/src/com/android/systemui/shared/clocks/ClockRegistry.kt
@@ -151,6 +151,7 @@ open class ClockRegistry(
                             { str1 = id },
                             { "Clock Id conflict on load: $str1 is double registered" }
                         )
+                        manager.unloadPlugin()
                         continue
                     }
 
diff --git a/packages/SystemUI/shared/src/com/android/systemui/shared/plugins/PluginActionManager.java b/packages/SystemUI/shared/src/com/android/systemui/shared/plugins/PluginActionManager.java
index 3d05542116e04..4f73fc426c3bb 100644
--- a/packages/SystemUI/shared/src/com/android/systemui/shared/plugins/PluginActionManager.java
+++ b/packages/SystemUI/shared/src/com/android/systemui/shared/plugins/PluginActionManager.java
@@ -109,7 +109,7 @@ public class PluginActionManager<T extends Plugin> {
     /** Load all plugins matching this instance's action. */
     public void loadAll() {
         if (DEBUG) Log.d(TAG, "startListening");
-        mBgExecutor.execute(this::queryAll);
+        mBgExecutor.execute(() -> queryAll());
     }
 
     /** Unload all plugins managed by this instance. */
@@ -255,17 +255,18 @@ public class PluginActionManager<T extends Plugin> {
             intent.setPackage(pkgName);
         }
         List<ResolveInfo> result = mPm.queryIntentServices(intent, 0);
-        if (DEBUG) Log.d(TAG, "Found " + result.size() + " plugins");
+        if (DEBUG) {
+            Log.d(TAG, "Found " + result.size() + " plugins");
+            for (ResolveInfo info : result) {
+                ComponentName name = new ComponentName(info.serviceInfo.packageName,
+                        info.serviceInfo.name);
+                Log.d(TAG, "  " + name);
+            }
+        }
+
         if (result.size() > 1 && !mAllowMultiple) {
             // TODO: Show warning.
             Log.w(TAG, "Multiple plugins found for " + mAction);
-            if (DEBUG) {
-                for (ResolveInfo info : result) {
-                    ComponentName name = new ComponentName(info.serviceInfo.packageName,
-                            info.serviceInfo.name);
-                    Log.w(TAG, "  " + name);
-                }
-            }
             return;
         }
         for (ResolveInfo info : result) {
@@ -307,7 +308,7 @@ public class PluginActionManager<T extends Plugin> {
             // TODO: Only create the plugin before version check if we need it for
             // legacy version check.
             if (DEBUG) {
-                Log.d(TAG, "createPlugin");
+                Log.d(TAG, "createPlugin: " + component);
             }
             try {
                 return mPluginInstanceFactory.create(
@@ -317,7 +318,7 @@ public class PluginActionManager<T extends Plugin> {
                 reportInvalidVersion(component, component.getClassName(), e);
             }
         } catch (Throwable e) {
-            Log.w(TAG, "Couldn't load plugin: " + packageName, e);
+            Log.w(TAG, "Couldn't load plugin: " + component, e);
             return null;
         }
 
diff --git a/packages/SystemUI/tests/src/com/android/systemui/shared/clocks/ClockRegistryTest.kt b/packages/SystemUI/tests/src/com/android/systemui/shared/clocks/ClockRegistryTest.kt
index 78f5bf20e6f98..eef4470c48cd7 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/shared/clocks/ClockRegistryTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/shared/clocks/ClockRegistryTest.kt
@@ -43,6 +43,8 @@ import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.Mock
+import org.mockito.Mockito.never
+import org.mockito.Mockito.times
 import org.mockito.Mockito.verify
 import org.mockito.Mockito.`when` as whenever
 import org.mockito.junit.MockitoJUnit
@@ -172,7 +174,7 @@ class ClockRegistryTest : SysuiTestCase() {
     }
 
     @Test
-    fun clockIdConflict_ErrorWithoutCrash() {
+    fun clockIdConflict_ErrorWithoutCrash_unloadDuplicate() {
         val mockPluginLifecycle1 = mock<PluginLifecycleManager<ClockProviderPlugin>>()
         val plugin1 = FakeClockPlugin()
             .addClock("clock_1", "clock 1", { mockClock }, { mockThumbnail })
@@ -199,6 +201,8 @@ class ClockRegistryTest : SysuiTestCase() {
         assertEquals(registry.createExampleClock("clock_2"), mockClock)
         assertEquals(registry.getClockThumbnail("clock_1"), mockThumbnail)
         assertEquals(registry.getClockThumbnail("clock_2"), mockThumbnail)
+        verify(mockPluginLifecycle1, never()).unloadPlugin()
+        verify(mockPluginLifecycle2, times(2)).unloadPlugin()
     }
 
     @Test