Merge "Always use placeholder for blank media titles" into udc-dev

core/java/android/app/StatusBarManager.java

@@ -24,9 +24,11 @@ import android.annotation.RequiresPermission;
 import android.annotation.SystemApi;
 import android.annotation.SystemService;
 import android.annotation.TestApi;
+import android.annotation.UserIdInt;
 import android.app.compat.CompatChanges;
 import android.compat.annotation.ChangeId;
 import android.compat.annotation.EnabledSince;
+import android.compat.annotation.LoggingOnly;
 import android.compat.annotation.UnsupportedAppUsage;
 import android.content.ComponentName;
 import android.content.Context;
@@ -49,6 +51,7 @@ import android.util.Slog;
 import android.view.KeyEvent;
 import android.view.View;
 
+import com.android.internal.compat.IPlatformCompat;
 import com.android.internal.statusbar.AppClipsServiceConnector;
 import com.android.internal.statusbar.IAddTileResultCallback;
 import com.android.internal.statusbar.IStatusBarService;
@@ -170,6 +173,8 @@ public class StatusBarManager {
     public @interface Disable2Flags {}
     // LINT.ThenChange(frameworks/base/packages/SystemUI/src/com/android/systemui/statusbar/disableflags/DisableFlagsLogger.kt)
 
+    private static final String TAG = "StatusBarManager";
+
     /**
      * Default disable flags for setup
      *
@@ -572,13 +577,13 @@ public class StatusBarManager {
     private static final long MEDIA_CONTROL_SESSION_ACTIONS = 203800354L;
 
     /**
-     * Media controls based on {@link android.app.Notification.MediaStyle} notifications will be
+     * Media controls based on {@link android.app.Notification.MediaStyle} notifications should
-     * required to include a non-empty title, either in the {@link android.media.MediaMetadata} or
+     * include a non-empty title, either in the {@link android.media.MediaMetadata} or
      * notification title.
      */
     @ChangeId
-    @EnabledSince(targetSdkVersion = Build.VERSION_CODES.UPSIDE_DOWN_CAKE)
+    @LoggingOnly
-    private static final long MEDIA_CONTROL_REQUIRES_TITLE = 274775190L;
+    private static final long MEDIA_CONTROL_BLANK_TITLE = 274775190L;
 
     @UnsupportedAppUsage
     private Context mContext;
@@ -586,6 +591,9 @@ public class StatusBarManager {
     @UnsupportedAppUsage
     private IBinder mToken = new Binder();
 
+    private final IPlatformCompat mPlatformCompat = IPlatformCompat.Stub.asInterface(
+            ServiceManager.getService(Context.PLATFORM_COMPAT_SERVICE));
+
     @UnsupportedAppUsage
     StatusBarManager(Context context) {
         mContext = context;
@@ -597,7 +605,7 @@ public class StatusBarManager {
             mService = IStatusBarService.Stub.asInterface(
                     ServiceManager.getService(Context.STATUS_BAR_SERVICE));
             if (mService == null) {
-                Slog.w("StatusBarManager", "warning: no STATUS_BAR_SERVICE");
+                Slog.w(TAG, "warning: no STATUS_BAR_SERVICE");
             }
         }
         return mService;
@@ -1226,18 +1234,22 @@ public class StatusBarManager {
     }
 
     /**
-     * Checks whether the given package must include a non-empty title for its media controls.
+     * Log that the given package has posted media controls with a blank title
      *
      * @param packageName App posting media controls
-     * @param user Current user handle
+     * @param userId Current user ID
-     * @return true if the app is required to provide a non-empty title
+     * @throws RuntimeException if there is an error reporting the change
      *
      * @hide
      */
-    @RequiresPermission(allOf = {android.Manifest.permission.READ_COMPAT_CHANGE_CONFIG,
+    public void logBlankMediaTitle(String packageName, @UserIdInt int userId)
-            android.Manifest.permission.LOG_COMPAT_CHANGE})
+            throws RuntimeException {
-    public static boolean isMediaTitleRequiredForApp(String packageName, UserHandle user) {
+        try {
-        return CompatChanges.isChangeEnabled(MEDIA_CONTROL_REQUIRES_TITLE, packageName, user);
+            mPlatformCompat.reportChangeByPackageName(MEDIA_CONTROL_BLANK_TITLE, packageName,
+                        userId);
+        } catch (RemoteException e) {
+            throw e.rethrowFromSystemServer();
+        }
     }
 
     /**

packages/SystemUI/src/com/android/systemui/media/controls/pipeline/MediaDataManager.kt

@@ -16,10 +16,12 @@
 
 package com.android.systemui.media.controls.pipeline
 
+import android.annotation.SuppressLint
 import android.app.BroadcastOptions
 import android.app.Notification
 import android.app.Notification.EXTRA_SUBSTITUTE_APP_NAME
 import android.app.PendingIntent
+import android.app.StatusBarManager
 import android.app.smartspace.SmartspaceConfig
 import android.app.smartspace.SmartspaceManager
 import android.app.smartspace.SmartspaceSession
@@ -43,7 +45,6 @@ import android.media.session.PlaybackState
 import android.net.Uri
 import android.os.Parcelable
 import android.os.Process
-import android.os.RemoteException
 import android.os.UserHandle
 import android.provider.Settings
 import android.service.notification.StatusBarNotification
@@ -53,7 +54,6 @@ import android.util.Log
 import android.util.Pair as APair
 import androidx.media.utils.MediaConstants
 import com.android.internal.logging.InstanceId
-import com.android.internal.statusbar.IStatusBarService
 import com.android.keyguard.KeyguardUpdateMonitor
 import com.android.systemui.Dumpable
 import com.android.systemui.R
@@ -185,7 +185,6 @@ class MediaDataManager(
     private val logger: MediaUiEventLogger,
     private val smartspaceManager: SmartspaceManager,
     private val keyguardUpdateMonitor: KeyguardUpdateMonitor,
-    private val statusBarService: IStatusBarService,
 ) : Dumpable, BcSmartspaceDataPlugin.SmartspaceTargetListener {
 
     companion object {
@@ -230,6 +229,10 @@ class MediaDataManager(
     private val artworkHeight =
         context.resources.getDimensionPixelSize(R.dimen.qs_media_session_height_expanded)
 
+    @SuppressLint("WrongConstant") // sysui allowed to call STATUS_BAR_SERVICE
+    private val statusBarManager =
+        context.getSystemService(Context.STATUS_BAR_SERVICE) as StatusBarManager
+
     /** Check whether this notification is an RCN */
     private fun isRemoteCastNotification(sbn: StatusBarNotification): Boolean {
         return sbn.notification.extras.containsKey(Notification.EXTRA_MEDIA_REMOTE_DEVICE)
@@ -257,7 +260,6 @@ class MediaDataManager(
         mediaFlags: MediaFlags,
         logger: MediaUiEventLogger,
         smartspaceManager: SmartspaceManager,
-        statusBarService: IStatusBarService,
         keyguardUpdateMonitor: KeyguardUpdateMonitor,
     ) : this(
         context,
@@ -283,7 +285,6 @@ class MediaDataManager(
         logger,
         smartspaceManager,
         keyguardUpdateMonitor,
-        statusBarService,
     )
 
     private val appChangeReceiver =
@@ -793,27 +794,12 @@ class MediaDataManager(
             song = HybridGroupManager.resolveTitle(notif)
         }
         if (song.isNullOrBlank()) {
-            if (mediaFlags.isMediaTitleRequired(sbn.packageName, sbn.user)) {
+            // For apps that don't include a title, log and add a placeholder
-                // App is required to provide a title: cancel the underlying notification
+            song = context.getString(R.string.controls_media_empty_title, appName)
-                try {
+            try {
-                    statusBarService.onNotificationError(
+                statusBarManager.logBlankMediaTitle(sbn.packageName, sbn.user.identifier)
-                        sbn.packageName,
+            } catch (e: RuntimeException) {
-                        sbn.tag,
+                Log.e(TAG, "Error reporting blank media title for package ${sbn.packageName}")
-                        sbn.id,
-                        sbn.uid,
-                        sbn.initialPid,
-                        MEDIA_TITLE_ERROR_MESSAGE,
-                        sbn.user.identifier
-                    )
-                } catch (e: RemoteException) {
-                    Log.e(TAG, "cancelNotification failed: $e")
-                }
-                // Only add log for media removed if active media is updated with invalid title.
-                foregroundExecutor.execute { removeEntry(key, !isNewlyActiveEntry) }
-                return
-            } else {
-                // For apps that don't have the title requirement yet, add a placeholder
-                song = context.getString(R.string.controls_media_empty_title, appName)
             }
         }
 

packages/SystemUI/src/com/android/systemui/media/controls/util/MediaFlags.kt

@@ -64,9 +64,4 @@ class MediaFlags @Inject constructor(private val featureFlags: FeatureFlags) {
 
     /** Check whether we allow remote media to generate resume controls */
     fun isRemoteResumeAllowed() = featureFlags.isEnabled(Flags.MEDIA_REMOTE_RESUME)
-
-    /** Check whether app is required to provide a non-empty media title */
-    fun isMediaTitleRequired(packageName: String, user: UserHandle): Boolean {
-        return StatusBarManager.isMediaTitleRequiredForApp(packageName, user)
-    }
 }

packages/SystemUI/tests/src/com/android/systemui/media/controls/pipeline/MediaDataManagerTest.kt

@@ -41,7 +41,6 @@ import android.testing.TestableLooper.RunWithLooper
 import androidx.media.utils.MediaConstants
 import androidx.test.filters.SmallTest
 import com.android.internal.logging.InstanceId
-import com.android.internal.statusbar.IStatusBarService
 import com.android.keyguard.KeyguardUpdateMonitor
 import com.android.systemui.InstanceIdSequenceFake
 import com.android.systemui.R
@@ -133,7 +132,6 @@ class MediaDataManagerTest : SysuiTestCase() {
     @Mock lateinit var activityStarter: ActivityStarter
     @Mock lateinit var smartspaceManager: SmartspaceManager
     @Mock lateinit var keyguardUpdateMonitor: KeyguardUpdateMonitor
-    @Mock lateinit var statusBarService: IStatusBarService
     lateinit var smartspaceMediaDataProvider: SmartspaceMediaDataProvider
     @Mock lateinit var mediaSmartspaceTarget: SmartspaceTarget
     @Mock private lateinit var mediaRecommendationItem: SmartspaceAction
@@ -197,7 +195,6 @@ class MediaDataManagerTest : SysuiTestCase() {
                 logger = logger,
                 smartspaceManager = smartspaceManager,
                 keyguardUpdateMonitor = keyguardUpdateMonitor,
-                statusBarService = statusBarService,
             )
         verify(tunerService)
             .addTunable(capture(tunableCaptor), eq(Settings.Secure.MEDIA_CONTROLS_RECOMMENDATION))
@@ -522,143 +519,12 @@ class MediaDataManagerTest : SysuiTestCase() {
     }
 
     @Test
-    fun testOnNotificationAdded_emptyTitle_isRequired_notLoaded() {
+    fun testOnNotificationAdded_emptyTitle_hasPlaceholder() {
-        // When the manager has a notification with an empty title, and the app is required
-        // to include a non-empty title
-        whenever(mediaFlags.isMediaTitleRequired(any(), any())).thenReturn(true)
-        whenever(controller.metadata)
-            .thenReturn(
-                metadataBuilder
-                    .putString(MediaMetadata.METADATA_KEY_TITLE, SESSION_EMPTY_TITLE)
-                    .build()
-            )
-        mediaDataManager.onNotificationAdded(KEY, mediaNotification)
-
-        // Then the media control is not added and we report a notification error
-        assertThat(backgroundExecutor.runAllReady()).isEqualTo(1)
-        assertThat(foregroundExecutor.runAllReady()).isEqualTo(1)
-        verify(statusBarService)
-            .onNotificationError(
-                eq(PACKAGE_NAME),
-                eq(mediaNotification.tag),
-                eq(mediaNotification.id),
-                eq(mediaNotification.uid),
-                eq(mediaNotification.initialPid),
-                eq(MEDIA_TITLE_ERROR_MESSAGE),
-                eq(mediaNotification.user.identifier)
-            )
-        verify(listener, never())
-            .onMediaDataLoaded(
-                eq(KEY),
-                eq(null),
-                capture(mediaDataCaptor),
-                eq(true),
-                eq(0),
-                eq(false)
-            )
-        verify(logger, never()).logResumeMediaAdded(anyInt(), eq(PACKAGE_NAME), any())
-        verify(logger, never()).logMediaRemoved(anyInt(), eq(PACKAGE_NAME), any())
-    }
-
-    @Test
-    fun testOnNotificationAdded_blankTitle_isRequired_notLoaded() {
-        // When the manager has a notification with a blank title, and the app is required
-        // to include a non-empty title
-        whenever(mediaFlags.isMediaTitleRequired(any(), any())).thenReturn(true)
-        whenever(controller.metadata)
-            .thenReturn(
-                metadataBuilder
-                    .putString(MediaMetadata.METADATA_KEY_TITLE, SESSION_BLANK_TITLE)
-                    .build()
-            )
-        mediaDataManager.onNotificationAdded(KEY, mediaNotification)
-
-        // Then the media control is not added and we report a notification error
-        assertThat(backgroundExecutor.runAllReady()).isEqualTo(1)
-        assertThat(foregroundExecutor.runAllReady()).isEqualTo(1)
-        verify(statusBarService)
-            .onNotificationError(
-                eq(PACKAGE_NAME),
-                eq(mediaNotification.tag),
-                eq(mediaNotification.id),
-                eq(mediaNotification.uid),
-                eq(mediaNotification.initialPid),
-                eq(MEDIA_TITLE_ERROR_MESSAGE),
-                eq(mediaNotification.user.identifier)
-            )
-        verify(listener, never())
-            .onMediaDataLoaded(
-                eq(KEY),
-                eq(null),
-                capture(mediaDataCaptor),
-                eq(true),
-                eq(0),
-                eq(false)
-            )
-        verify(logger, never()).logResumeMediaAdded(anyInt(), eq(PACKAGE_NAME), any())
-        verify(logger, never()).logMediaRemoved(anyInt(), eq(PACKAGE_NAME), any())
-    }
-
-    @Test
-    fun testOnNotificationUpdated_invalidTitle_isRequired_logMediaRemoved() {
-        // When the app is required to provide a non-blank title, and updates a previously valid
-        // title to an empty one
-        whenever(mediaFlags.isMediaTitleRequired(any(), any())).thenReturn(true)
-        addNotificationAndLoad()
-        val data = mediaDataCaptor.value
-
-        verify(listener)
-            .onMediaDataLoaded(
-                eq(KEY),
-                eq(null),
-                capture(mediaDataCaptor),
-                eq(true),
-                eq(0),
-                eq(false)
-            )
-
-        reset(listener)
-        whenever(controller.metadata)
-            .thenReturn(
-                metadataBuilder
-                    .putString(MediaMetadata.METADATA_KEY_TITLE, SESSION_BLANK_TITLE)
-                    .build()
-            )
-        mediaDataManager.onNotificationAdded(KEY, mediaNotification)
-
-        // Then the media control is removed
-        assertThat(backgroundExecutor.runAllReady()).isEqualTo(1)
-        assertThat(foregroundExecutor.runAllReady()).isEqualTo(1)
-        verify(statusBarService)
-            .onNotificationError(
-                eq(PACKAGE_NAME),
-                eq(mediaNotification.tag),
-                eq(mediaNotification.id),
-                eq(mediaNotification.uid),
-                eq(mediaNotification.initialPid),
-                eq(MEDIA_TITLE_ERROR_MESSAGE),
-                eq(mediaNotification.user.identifier)
-            )
-        verify(listener, never())
-            .onMediaDataLoaded(
-                eq(KEY),
-                eq(null),
-                capture(mediaDataCaptor),
-                eq(true),
-                eq(0),
-                eq(false)
-            )
-        verify(logger).logMediaRemoved(anyInt(), eq(PACKAGE_NAME), eq(data.instanceId))
-    }
-
-    @Test
-    fun testOnNotificationAdded_emptyTitle_notRequired_hasPlaceholder() {
         // When the manager has a notification with an empty title, and the app is not
         // required to include a non-empty title
         val mockPackageManager = mock(PackageManager::class.java)
         context.setMockPackageManager(mockPackageManager)
         whenever(mockPackageManager.getApplicationLabel(any())).thenReturn(APP_NAME)
-        whenever(mediaFlags.isMediaTitleRequired(any(), any())).thenReturn(false)
         whenever(controller.metadata)
             .thenReturn(
                 metadataBuilder
@@ -684,13 +550,12 @@ class MediaDataManagerTest : SysuiTestCase() {
     }
 
     @Test
-    fun testOnNotificationAdded_blankTitle_notRequired_hasPlaceholder() {
+    fun testOnNotificationAdded_blankTitle_hasPlaceholder() {
         // GIVEN that the manager has a notification with a blank title, and the app is not
         // required to include a non-empty title
         val mockPackageManager = mock(PackageManager::class.java)
         context.setMockPackageManager(mockPackageManager)
         whenever(mockPackageManager.getApplicationLabel(any())).thenReturn(APP_NAME)
-        whenever(mediaFlags.isMediaTitleRequired(any(), any())).thenReturn(false)
         whenever(controller.metadata)
             .thenReturn(
                 metadataBuilder
@@ -722,7 +587,6 @@ class MediaDataManagerTest : SysuiTestCase() {
         val mockPackageManager = mock(PackageManager::class.java)
         context.setMockPackageManager(mockPackageManager)
         whenever(mockPackageManager.getApplicationLabel(any())).thenReturn(APP_NAME)
-        whenever(mediaFlags.isMediaTitleRequired(any(), any())).thenReturn(true)
         whenever(controller.metadata)
             .thenReturn(
                 metadataBuilder