Merge "Improve user handling when querying for resumable media" into rvc-dev

packages/SystemUI/src/com/android/systemui/media/MediaBrowserFactory.java

@@ -0,0 +1,49 @@
+/*
+ * Copyright (C) 2020 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.media;
+
+import android.content.ComponentName;
+import android.content.Context;
+import android.media.browse.MediaBrowser;
+import android.os.Bundle;
+
+import javax.inject.Inject;
+
+/**
+ * Testable wrapper around {@link MediaBrowser} constructor
+ */
+public class MediaBrowserFactory {
+    private final Context mContext;
+
+    @Inject
+    public MediaBrowserFactory(Context context) {
+        mContext = context;
+    }
+
+    /**
+     * Creates a new MediaBrowser
+     *
+     * @param serviceComponent
+     * @param callback
+     * @param rootHints
+     * @return
+     */
+    public MediaBrowser create(ComponentName serviceComponent,
+            MediaBrowser.ConnectionCallback callback, Bundle rootHints) {
+        return new MediaBrowser(mContext, serviceComponent, callback, rootHints);
+    }
+}

packages/SystemUI/src/com/android/systemui/media/MediaResumeListener.kt

@@ -28,6 +28,7 @@ import android.os.UserHandle
 import android.provider.Settings
 import android.service.media.MediaBrowserService
 import android.util.Log
+import com.android.internal.annotations.VisibleForTesting
 import com.android.systemui.broadcast.BroadcastDispatcher
 import com.android.systemui.dagger.qualifiers.Background
 import com.android.systemui.tuner.TunerService
@@ -47,7 +48,8 @@ class MediaResumeListener @Inject constructor(
     private val context: Context,
     private val broadcastDispatcher: BroadcastDispatcher,
     @Background private val backgroundExecutor: Executor,
-    private val tunerService: TunerService
+    private val tunerService: TunerService,
+    private val mediaBrowserFactory: ResumeMediaBrowserFactory
 ) : MediaDataManager.Listener {
 
     private var useMediaResumption: Boolean = Utils.useMediaResumption(context)
@@ -58,7 +60,8 @@ class MediaResumeListener @Inject constructor(
     private var mediaBrowser: ResumeMediaBrowser? = null
     private var currentUserId: Int = context.userId
 
-    private val userChangeReceiver = object : BroadcastReceiver() {
+    @VisibleForTesting
+    val userChangeReceiver = object : BroadcastReceiver() {
         override fun onReceive(context: Context, intent: Intent) {
             if (Intent.ACTION_USER_UNLOCKED == intent.action) {
                 loadMediaResumptionControls()
@@ -87,9 +90,16 @@ class MediaResumeListener @Inject constructor(
                 Log.e(TAG, "Error getting package information", e)
             }
 
-            Log.d(TAG, "Adding resume controls $desc")
+            Log.d(TAG, "Adding resume controls for ${browser.userId}: $desc")
-            mediaDataManager.addResumptionControls(currentUserId, desc, resumeAction, token,
+            mediaDataManager.addResumptionControls(
-                appName.toString(), appIntent, component.packageName)
+                browser.userId,
+                desc,
+                resumeAction,
+                token,
+                appName.toString(),
+                appIntent,
+                component.packageName
+            )
         }
     }
 
@@ -132,7 +142,11 @@ class MediaResumeListener @Inject constructor(
             val component = ComponentName(packageName, className)
             resumeComponents.add(component)
         }
-        Log.d(TAG, "loaded resume components ${resumeComponents.toArray().contentToString()}")
+        Log.d(
+            TAG,
+            "loaded resume components for $currentUserId: " +
+                "${resumeComponents.toArray().contentToString()}"
+        )
     }
 
     /**
@@ -143,9 +157,19 @@ class MediaResumeListener @Inject constructor(
             return
         }
 
+        val pm = context.packageManager
         resumeComponents.forEach {
-            val browser = ResumeMediaBrowser(context, mediaBrowserCallback, it)
+            // Verify that the service exists for this user
-            browser.findRecentMedia()
+            val intent = Intent(MediaBrowserService.SERVICE_INTERFACE)
+            intent.component = it
+            val inf = pm.resolveServiceAsUser(intent, 0, currentUserId)
+            if (inf != null) {
+                val browser =
+                        mediaBrowserFactory.create(mediaBrowserCallback, it, currentUserId)
+                browser.findRecentMedia()
+            } else {
+                Log.d(TAG, "User $currentUserId does not have component $it")
+            }
         }
     }
 
@@ -159,7 +183,7 @@ class MediaResumeListener @Inject constructor(
                 Log.d(TAG, "Checking for service component for " + data.packageName)
                 val pm = context.packageManager
                 val serviceIntent = Intent(MediaBrowserService.SERVICE_INTERFACE)
-                val resumeInfo = pm.queryIntentServices(serviceIntent, 0)
+                val resumeInfo = pm.queryIntentServicesAsUser(serviceIntent, 0, currentUserId)
 
                 val inf = resumeInfo?.filter {
                     it.serviceInfo.packageName == data.packageName
@@ -183,7 +207,7 @@ class MediaResumeListener @Inject constructor(
     private fun tryUpdateResumptionList(key: String, componentName: ComponentName) {
         Log.d(TAG, "Testing if we can connect to $componentName")
         mediaBrowser?.disconnect()
-        mediaBrowser = ResumeMediaBrowser(context,
+        mediaBrowser = mediaBrowserFactory.create(
                 object : ResumeMediaBrowser.Callback() {
                     override fun onConnected() {
                         Log.d(TAG, "yes we can resume with $componentName")
@@ -200,7 +224,9 @@ class MediaResumeListener @Inject constructor(
                         mediaBrowser = null
                     }
                 },
-                componentName)
+                componentName,
+                currentUserId
+            )
         mediaBrowser?.testConnection()
     }
 
@@ -235,7 +261,7 @@ class MediaResumeListener @Inject constructor(
     private fun getResumeAction(componentName: ComponentName): Runnable {
         return Runnable {
             mediaBrowser?.disconnect()
-            mediaBrowser = ResumeMediaBrowser(context,
+            mediaBrowser = mediaBrowserFactory.create(
                 object : ResumeMediaBrowser.Callback() {
                     override fun onConnected() {
                         if (mediaBrowser?.token == null) {
@@ -257,7 +283,8 @@ class MediaResumeListener @Inject constructor(
                         mediaBrowser = null
                     }
                 },
-                componentName)
+                componentName,
+                currentUserId)
             mediaBrowser?.restart()
         }
     }

packages/SystemUI/src/com/android/systemui/media/ResumeMediaBrowser.java

@@ -16,6 +16,7 @@
 
 package com.android.systemui.media;
 
+import android.annotation.UserIdInt;
 import android.app.PendingIntent;
 import android.content.ComponentName;
 import android.content.Context;
@@ -46,6 +47,9 @@ public class ResumeMediaBrowser {
     private static final String TAG = "ResumeMediaBrowser";
     private final Context mContext;
     private final Callback mCallback;
+    private MediaBrowserFactory mBrowserFactory;
+    @UserIdInt private final int mUserId;
+
     private MediaBrowser mMediaBrowser;
     private ComponentName mComponentName;
 
@@ -54,11 +58,15 @@ public class ResumeMediaBrowser {
      * @param context the context
      * @param callback used to report media items found
      * @param componentName Component name of the MediaBrowserService this browser will connect to
+     * @param userId ID of the current user
      */
-    public ResumeMediaBrowser(Context context, Callback callback, ComponentName componentName) {
+    public ResumeMediaBrowser(Context context, Callback callback, ComponentName componentName,
+            MediaBrowserFactory browserFactory, @UserIdInt int userId) {
         mContext = context;
         mCallback = callback;
         mComponentName = componentName;
+        mBrowserFactory = browserFactory;
+        mUserId = userId;
     }
 
     /**
@@ -74,7 +82,7 @@ public class ResumeMediaBrowser {
         disconnect();
         Bundle rootHints = new Bundle();
         rootHints.putBoolean(MediaBrowserService.BrowserRoot.EXTRA_RECENT, true);
-        mMediaBrowser = new MediaBrowser(mContext,
+        mMediaBrowser = mBrowserFactory.create(
                 mComponentName,
                 mConnectionCallback,
                 rootHints);
@@ -182,7 +190,7 @@ public class ResumeMediaBrowser {
         disconnect();
         Bundle rootHints = new Bundle();
         rootHints.putBoolean(MediaBrowserService.BrowserRoot.EXTRA_RECENT, true);
-        mMediaBrowser = new MediaBrowser(mContext, mComponentName,
+        mMediaBrowser = mBrowserFactory.create(mComponentName,
                 new MediaBrowser.ConnectionCallback() {
                     @Override
                     public void onConnected() {
@@ -212,6 +220,14 @@ public class ResumeMediaBrowser {
         mMediaBrowser.connect();
     }
 
+    /**
+     * Get the ID of the user associated with this broswer
+     * @return the user ID
+     */
+    public @UserIdInt int getUserId() {
+        return mUserId;
+    }
+
     /**
      * Get the media session token
      * @return the token, or null if the MediaBrowser is null or disconnected
@@ -268,7 +284,7 @@ public class ResumeMediaBrowser {
                 };
         Bundle rootHints = new Bundle();
         rootHints.putBoolean(MediaBrowserService.BrowserRoot.EXTRA_RECENT, true);
-        mMediaBrowser = new MediaBrowser(mContext,
+        mMediaBrowser =  mBrowserFactory.create(
                 mComponentName,
                 connectionCallback,
                 rootHints);

packages/SystemUI/src/com/android/systemui/media/ResumeMediaBrowserFactory.java

@@ -0,0 +1,51 @@
+/*
+ * Copyright (C) 2020 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.media;
+
+import android.annotation.UserIdInt;
+import android.content.ComponentName;
+import android.content.Context;
+
+import javax.inject.Inject;
+
+/**
+ * Testable wrapper around {@link ResumeMediaBrowser} constructor
+ */
+public class ResumeMediaBrowserFactory {
+    private final Context mContext;
+    private final MediaBrowserFactory mBrowserFactory;
+
+    @Inject
+    public ResumeMediaBrowserFactory(Context context, MediaBrowserFactory browserFactory) {
+        mContext = context;
+        mBrowserFactory = browserFactory;
+    }
+
+    /**
+     * Creates a new ResumeMediaBrowser.
+     *
+     * @param callback will be called on connection or error, and addTrack when media item found
+     * @param componentName component to browse
+     * @param userId ID of the current user
+     * @return
+     */
+    public ResumeMediaBrowser create(ResumeMediaBrowser.Callback callback,
+            ComponentName componentName, @UserIdInt int userId) {
+        return new ResumeMediaBrowser(mContext, callback, componentName, mBrowserFactory,
+                userId);
+    }
+}

packages/SystemUI/tests/src/com/android/systemui/media/MediaResumeListenerTest.kt

@@ -0,0 +1,232 @@
+/*
+ * Copyright (C) 2020 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.media
+
+import android.app.PendingIntent
+import android.content.ComponentName
+import android.content.Context
+import android.content.Intent
+import android.content.SharedPreferences
+import android.content.pm.PackageManager
+import android.content.pm.ResolveInfo
+import android.content.pm.ServiceInfo
+import android.graphics.Color
+import android.media.MediaDescription
+import android.media.session.MediaSession
+import android.provider.Settings
+import android.testing.AndroidTestingRunner
+import android.testing.TestableLooper
+import androidx.test.filters.SmallTest
+import com.android.systemui.SysuiTestCase
+import com.android.systemui.broadcast.BroadcastDispatcher
+import com.android.systemui.tuner.TunerService
+import com.android.systemui.util.concurrency.FakeExecutor
+import com.android.systemui.util.time.FakeSystemClock
+import org.junit.After
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.mockito.ArgumentCaptor
+import org.mockito.ArgumentMatchers.anyInt
+import org.mockito.Captor
+import org.mockito.Mock
+import org.mockito.Mockito
+import org.mockito.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.MockitoAnnotations
+
+private const val KEY = "TEST_KEY"
+private const val OLD_KEY = "RESUME_KEY"
+private const val APP = "APP"
+private const val BG_COLOR = Color.RED
+private const val PACKAGE_NAME = "PKG"
+private const val CLASS_NAME = "CLASS"
+private const val ARTIST = "ARTIST"
+private const val TITLE = "TITLE"
+private const val USER_ID = 0
+private const val MEDIA_PREFERENCES = "media_control_prefs"
+private const val RESUME_COMPONENTS = "package1/class1:package2/class2:package3/class3"
+
+private fun <T> capture(argumentCaptor: ArgumentCaptor<T>): T = argumentCaptor.capture()
+private fun <T> eq(value: T): T = Mockito.eq(value) ?: value
+private fun <T> any(): T = Mockito.any<T>()
+
+@SmallTest
+@RunWith(AndroidTestingRunner::class)
+@TestableLooper.RunWithLooper
+class MediaResumeListenerTest : SysuiTestCase() {
+
+    @Mock private lateinit var broadcastDispatcher: BroadcastDispatcher
+    @Mock private lateinit var mediaDataManager: MediaDataManager
+    @Mock private lateinit var device: MediaDeviceData
+    @Mock private lateinit var token: MediaSession.Token
+    @Mock private lateinit var tunerService: TunerService
+    @Mock private lateinit var resumeBrowserFactory: ResumeMediaBrowserFactory
+    @Mock private lateinit var resumeBrowser: ResumeMediaBrowser
+    @Mock private lateinit var sharedPrefs: SharedPreferences
+    @Mock private lateinit var sharedPrefsEditor: SharedPreferences.Editor
+    @Mock private lateinit var mockContext: Context
+    @Mock private lateinit var pendingIntent: PendingIntent
+
+    @Captor lateinit var callbackCaptor: ArgumentCaptor<ResumeMediaBrowser.Callback>
+    @Captor lateinit var userIdCaptor: ArgumentCaptor<Int>
+
+    private lateinit var executor: FakeExecutor
+    private lateinit var data: MediaData
+    private lateinit var resumeListener: MediaResumeListener
+
+    private var originalQsSetting = Settings.Global.getInt(context.contentResolver,
+        Settings.Global.SHOW_MEDIA_ON_QUICK_SETTINGS, 1)
+    private var originalResumeSetting = Settings.Secure.getInt(context.contentResolver,
+        Settings.Secure.MEDIA_CONTROLS_RESUME, 0)
+
+    @Before
+    fun setup() {
+        MockitoAnnotations.initMocks(this)
+
+        Settings.Global.putInt(context.contentResolver,
+            Settings.Global.SHOW_MEDIA_ON_QUICK_SETTINGS, 1)
+        Settings.Secure.putInt(context.contentResolver,
+            Settings.Secure.MEDIA_CONTROLS_RESUME, 1)
+
+        whenever(resumeBrowserFactory.create(capture(callbackCaptor), any(), capture(userIdCaptor)))
+                .thenReturn(resumeBrowser)
+
+        // resume components are stored in sharedpreferences
+        whenever(mockContext.getSharedPreferences(eq(MEDIA_PREFERENCES), anyInt()))
+                .thenReturn(sharedPrefs)
+        whenever(sharedPrefs.getString(any(), any())).thenReturn(RESUME_COMPONENTS)
+        whenever(sharedPrefs.edit()).thenReturn(sharedPrefsEditor)
+        whenever(sharedPrefsEditor.putString(any(), any())).thenReturn(sharedPrefsEditor)
+        whenever(mockContext.packageManager).thenReturn(context.packageManager)
+        whenever(mockContext.contentResolver).thenReturn(context.contentResolver)
+        whenever(mockContext.userId).thenReturn(context.userId)
+
+        executor = FakeExecutor(FakeSystemClock())
+        resumeListener = MediaResumeListener(mockContext, broadcastDispatcher, executor,
+                tunerService, resumeBrowserFactory)
+        resumeListener.setManager(mediaDataManager)
+        mediaDataManager.addListener(resumeListener)
+
+        data = MediaData(
+                userId = USER_ID,
+                initialized = true,
+                backgroundColor = BG_COLOR,
+                app = APP,
+                appIcon = null,
+                artist = ARTIST,
+                song = TITLE,
+                artwork = null,
+                actions = emptyList(),
+                actionsToShowInCompact = emptyList(),
+                packageName = PACKAGE_NAME,
+                token = token,
+                clickIntent = null,
+                device = device,
+                active = true,
+                notificationKey = KEY,
+                resumeAction = null)
+    }
+
+    @After
+    fun tearDown() {
+        Settings.Global.putInt(context.contentResolver,
+            Settings.Global.SHOW_MEDIA_ON_QUICK_SETTINGS, originalQsSetting)
+        Settings.Secure.putInt(context.contentResolver,
+            Settings.Secure.MEDIA_CONTROLS_RESUME, originalResumeSetting)
+    }
+
+    @Test
+    fun testUserUnlocked_userChangeWhileQuerying() {
+        val firstUserId = context.userId
+        val secondUserId = firstUserId + 1
+        val description = MediaDescription.Builder().setTitle(TITLE).build()
+        val component = ComponentName(PACKAGE_NAME, CLASS_NAME)
+
+        setUpMbsWithValidResolveInfo()
+        whenever(resumeBrowser.token).thenReturn(token)
+        whenever(resumeBrowser.appIntent).thenReturn(pendingIntent)
+
+        val unlockIntent =
+            Intent(Intent.ACTION_USER_UNLOCKED).apply {
+                putExtra(Intent.EXTRA_USER_HANDLE, firstUserId)
+            }
+
+        // When the first user unlocks and we query their recent media
+        resumeListener.userChangeReceiver.onReceive(context, unlockIntent)
+        whenever(resumeBrowser.userId).thenReturn(userIdCaptor.value)
+        verify(resumeBrowser, times(3)).findRecentMedia()
+
+        // And the user changes before the MBS response is received
+        val changeIntent =
+            Intent(Intent.ACTION_USER_SWITCHED).apply {
+                putExtra(Intent.EXTRA_USER_HANDLE, secondUserId)
+            }
+        resumeListener.userChangeReceiver.onReceive(context, changeIntent)
+        callbackCaptor.value.addTrack(description, component, resumeBrowser)
+
+        // Then the loaded media is correctly associated with the first user
+        verify(mediaDataManager)
+            .addResumptionControls(
+                eq(firstUserId),
+                eq(description),
+                any(),
+                eq(token),
+                eq(PACKAGE_NAME),
+                eq(pendingIntent),
+                eq(PACKAGE_NAME)
+            )
+    }
+
+    @Test
+    fun testUserUnlocked_noComponent_doesNotQuery() {
+        // Set up a valid MBS, but user does not have the service available
+        setUpMbsWithValidResolveInfo()
+        val pm = mock(PackageManager::class.java)
+        whenever(mockContext.packageManager).thenReturn(pm)
+        whenever(pm.resolveServiceAsUser(any(), anyInt(), anyInt())).thenReturn(null)
+
+        val unlockIntent =
+            Intent(Intent.ACTION_USER_UNLOCKED).apply {
+                putExtra(Intent.EXTRA_USER_HANDLE, context.userId)
+            }
+
+        // When the user is unlocked, but does not have the component installed
+        resumeListener.userChangeReceiver.onReceive(context, unlockIntent)
+
+        // Then we never attempt to connect to it
+        verify(resumeBrowser, never()).findRecentMedia()
+    }
+
+    /** Sets up mocks to successfully find a MBS that returns valid media. */
+    private fun setUpMbsWithValidResolveInfo() {
+        val pm = mock(PackageManager::class.java)
+        whenever(mockContext.packageManager).thenReturn(pm)
+        val resolveInfo = ResolveInfo()
+        val serviceInfo = ServiceInfo()
+        serviceInfo.packageName = PACKAGE_NAME
+        resolveInfo.serviceInfo = serviceInfo
+        resolveInfo.serviceInfo.name = CLASS_NAME
+        val resumeInfo = listOf(resolveInfo)
+        whenever(pm.queryIntentServicesAsUser(any(), anyInt(), anyInt())).thenReturn(resumeInfo)
+        whenever(pm.resolveServiceAsUser(any(), anyInt(), anyInt())).thenReturn(resolveInfo)
+        whenever(pm.getApplicationLabel(any())).thenReturn(PACKAGE_NAME)
+    }
+}