Merge "Fix memory leak of MediaControlPanel" into udc-dev

4 files changed, 43 insertions, 24 deletions

diff --git a/packages/SystemUI/src/com/android/systemui/media/controls/ui/MediaCarouselController.kt b/packages/SystemUI/src/com/android/systemui/media/controls/ui/MediaCarouselController.kt
index 1e9a466ccdce..70b5e75e6048 100644
--- a/packages/SystemUI/src/com/android/systemui/media/controls/ui/MediaCarouselController.kt
+++ b/packages/SystemUI/src/com/android/systemui/media/controls/ui/MediaCarouselController.kt
@@ -21,6 +21,8 @@ import android.content.Context
 import android.content.Intent
 import android.content.res.ColorStateList
 import android.content.res.Configuration
+import android.database.ContentObserver
+import android.provider.Settings
 import android.provider.Settings.ACTION_MEDIA_CONTROLS_SETTINGS
 import android.util.Log
 import android.util.MathUtils
@@ -64,6 +66,7 @@ import com.android.systemui.util.Utils
 import com.android.systemui.util.animation.UniqueObjectHostView
 import com.android.systemui.util.animation.requiresRemeasuring
 import com.android.systemui.util.concurrency.DelayableExecutor
+import com.android.systemui.util.settings.GlobalSettings
 import com.android.systemui.util.time.SystemClock
 import com.android.systemui.util.traceSection
 import java.io.PrintWriter
@@ -105,6 +108,7 @@ constructor(
     private val mediaFlags: MediaFlags,
     private val keyguardUpdateMonitor: KeyguardUpdateMonitor,
     private val keyguardTransitionInteractor: KeyguardTransitionInteractor,
+    private val globalSettings: GlobalSettings,
 ) : Dumpable {
     /** The current width of the carousel */
     var currentCarouselWidth: Int = 0
@@ -169,6 +173,13 @@ constructor(
 
     private var carouselLocale: Locale? = null
 
+    private val animationScaleObserver: ContentObserver =
+        object : ContentObserver(null) {
+            override fun onChange(selfChange: Boolean) {
+                MediaPlayerData.players().forEach { it.updateAnimatorDurationScale() }
+            }
+        }
+
     /** Whether the media card currently has the "expanded" layout */
     @VisibleForTesting
     var currentlyExpanded = true
@@ -529,6 +540,12 @@ constructor(
                 listenForAnyStateToGoneKeyguardTransition(this)
             }
         }
+
+        // Notifies all active players about animation scale changes.
+        globalSettings.registerContentObserver(
+            Settings.Global.getUriFor(Settings.Global.ANIMATOR_DURATION_SCALE),
+            animationScaleObserver
+        )
     }
 
     private fun inflateSettingsButton() {
diff --git a/packages/SystemUI/src/com/android/systemui/media/controls/ui/MediaControlPanel.java b/packages/SystemUI/src/com/android/systemui/media/controls/ui/MediaControlPanel.java
index 0819d0d36e9a..35082fd5122a 100644
--- a/packages/SystemUI/src/com/android/systemui/media/controls/ui/MediaControlPanel.java
+++ b/packages/SystemUI/src/com/android/systemui/media/controls/ui/MediaControlPanel.java
@@ -34,7 +34,6 @@ import android.content.pm.PackageManager;
 import android.content.res.ColorStateList;
 import android.content.res.Configuration;
 import android.content.res.Resources;
-import android.database.ContentObserver;
 import android.graphics.Bitmap;
 import android.graphics.BlendMode;
 import android.graphics.Color;
@@ -252,13 +251,6 @@ public class MediaControlPanel {
     private boolean mWasPlaying = false;
     private boolean mButtonClicked = false;
 
-    private ContentObserver mAnimationScaleObserver = new ContentObserver(null) {
-        @Override
-        public void onChange(boolean selfChange) {
-            updateAnimatorDurationScale();
-        }
-    };
-
     /**
      * Initialize a new control panel
      *
@@ -318,10 +310,6 @@ public class MediaControlPanel {
         mFeatureFlags = featureFlags;
 
         mGlobalSettings = globalSettings;
-        mGlobalSettings.registerContentObserver(
-                Settings.Global.getUriFor(Settings.Global.ANIMATOR_DURATION_SCALE),
-                mAnimationScaleObserver
-        );
         updateAnimatorDurationScale();
     }
 
@@ -405,7 +393,10 @@ public class MediaControlPanel {
         updateSeekBarVisibility();
     }
 
-    private void updateAnimatorDurationScale() {
+    /**
+     * Reloads animator duration scale.
+     */
+    void updateAnimatorDurationScale() {
         if (mSeekBarObserver != null) {
             mSeekBarObserver.setAnimationEnabled(
                     mGlobalSettings.getFloat(Settings.Global.ANIMATOR_DURATION_SCALE, 1f) > 0f);
diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/controls/ui/MediaCarouselControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/controls/ui/MediaCarouselControllerTest.kt
index 2aff90c47189..5b8272b04bfb 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/media/controls/ui/MediaCarouselControllerTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/media/controls/ui/MediaCarouselControllerTest.kt
@@ -19,7 +19,9 @@ package com.android.systemui.media.controls.ui
 import android.app.PendingIntent
 import android.content.res.ColorStateList
 import android.content.res.Configuration
+import android.database.ContentObserver
 import android.os.LocaleList
+import android.provider.Settings
 import android.testing.AndroidTestingRunner
 import android.testing.TestableLooper
 import android.util.MathUtils.abs
@@ -56,6 +58,7 @@ import com.android.systemui.util.concurrency.DelayableExecutor
 import com.android.systemui.util.mockito.any
 import com.android.systemui.util.mockito.capture
 import com.android.systemui.util.mockito.eq
+import com.android.systemui.util.settings.GlobalSettings
 import com.android.systemui.util.time.FakeSystemClock
 import java.util.Locale
 import javax.inject.Provider
@@ -113,6 +116,7 @@ class MediaCarouselControllerTest : SysuiTestCase() {
     @Mock lateinit var mediaFlags: MediaFlags
     @Mock lateinit var keyguardUpdateMonitor: KeyguardUpdateMonitor
     @Mock lateinit var keyguardTransitionInteractor: KeyguardTransitionInteractor
+    @Mock lateinit var globalSettings: GlobalSettings
     private lateinit var transitionRepository: FakeKeyguardTransitionRepository
     @Captor lateinit var listener: ArgumentCaptor<MediaDataManager.Listener>
     @Captor
@@ -120,6 +124,7 @@ class MediaCarouselControllerTest : SysuiTestCase() {
     @Captor lateinit var visualStabilityCallback: ArgumentCaptor<OnReorderingAllowedListener>
     @Captor lateinit var keyguardCallback: ArgumentCaptor<KeyguardUpdateMonitorCallback>
     @Captor lateinit var hostStateCallback: ArgumentCaptor<MediaHostStatesManager.Callback>
+    @Captor lateinit var settingsObserverCaptor: ArgumentCaptor<ContentObserver>
 
     private val clock = FakeSystemClock()
     private lateinit var mediaCarouselController: MediaCarouselController
@@ -148,6 +153,7 @@ class MediaCarouselControllerTest : SysuiTestCase() {
                 mediaFlags,
                 keyguardUpdateMonitor,
                 KeyguardTransitionInteractor(transitionRepository, TestScope().backgroundScope),
+                globalSettings
             )
         verify(configurationController).addCallback(capture(configListener))
         verify(mediaDataManager).addListener(capture(listener))
@@ -160,6 +166,11 @@ class MediaCarouselControllerTest : SysuiTestCase() {
         whenever(mediaDataManager.smartspaceMediaData).thenReturn(smartspaceMediaData)
         whenever(mediaFlags.isPersistentSsCardEnabled()).thenReturn(false)
         MediaPlayerData.clear()
+        verify(globalSettings)
+            .registerContentObserver(
+                eq(Settings.Global.getUriFor(Settings.Global.ANIMATOR_DURATION_SCALE)),
+                settingsObserverCaptor.capture()
+            )
     }
 
     @Test
@@ -873,6 +884,15 @@ class MediaCarouselControllerTest : SysuiTestCase() {
         assertTrue(stateUpdated)
     }
 
+    @Test
+    fun testAnimationScaleChanged_mediaControlPanelsNotified() {
+        MediaPlayerData.addMediaPlayer("key", DATA, panel, clock, isSsReactivated = false)
+
+        globalSettings.putFloat(Settings.Global.ANIMATOR_DURATION_SCALE, 0f)
+        settingsObserverCaptor.value!!.onChange(false)
+        verify(panel).updateAnimatorDurationScale()
+    }
+
     /**
      * Helper method when a configuration change occurs.
      *
diff --git a/packages/SystemUI/tests/src/com/android/systemui/media/controls/ui/MediaControlPanelTest.kt b/packages/SystemUI/tests/src/com/android/systemui/media/controls/ui/MediaControlPanelTest.kt
index f6075add6afe..f902be3c1a29 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/media/controls/ui/MediaControlPanelTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/media/controls/ui/MediaControlPanelTest.kt
@@ -25,7 +25,6 @@ import android.content.Intent
 import android.content.pm.ApplicationInfo
 import android.content.pm.PackageManager
 import android.content.res.Configuration
-import android.database.ContentObserver
 import android.graphics.Bitmap
 import android.graphics.Canvas
 import android.graphics.Color
@@ -113,7 +112,6 @@ import org.junit.runner.RunWith
 import org.mockito.ArgumentCaptor
 import org.mockito.ArgumentMatchers.anyInt
 import org.mockito.ArgumentMatchers.anyLong
-import org.mockito.Captor
 import org.mockito.Mock
 import org.mockito.Mockito.anyString
 import org.mockito.Mockito.mock
@@ -239,7 +237,6 @@ public class MediaControlPanelTest : SysuiTestCase() {
             this.set(Flags.MEDIA_RECOMMENDATION_CARD_UPDATE, false)
         }
     @Mock private lateinit var globalSettings: GlobalSettings
-    @Captor private lateinit var settingsObserverCaptor: ArgumentCaptor<ContentObserver>
 
     @JvmField @Rule val mockito = MockitoJUnit.rule()
 
@@ -281,7 +278,7 @@ public class MediaControlPanelTest : SysuiTestCase() {
                     lockscreenUserManager,
                     broadcastDialogController,
                     fakeFeatureFlag,
-                    globalSettings,
+                    globalSettings
                 ) {
                 override fun loadAnimator(
                     animId: Int,
@@ -292,12 +289,6 @@ public class MediaControlPanelTest : SysuiTestCase() {
                 }
             }
 
-        verify(globalSettings)
-            .registerContentObserver(
-                eq(Settings.Global.getUriFor(Settings.Global.ANIMATOR_DURATION_SCALE)),
-                settingsObserverCaptor.capture()
-            )
-
         initGutsViewHolderMocks()
         initMediaViewHolderMocks()
 
@@ -986,7 +977,7 @@ public class MediaControlPanelTest : SysuiTestCase() {
 
         // When the setting changes,
         globalSettings.putFloat(Settings.Global.ANIMATOR_DURATION_SCALE, 0f)
-        settingsObserverCaptor.value!!.onChange(false)
+        player.updateAnimatorDurationScale()
 
         // Then the seekbar is set to not animate
         assertThat(seekBarObserver.animationEnabled).isFalse()