Merge "Fix potential NPE when connecting" into sc-dev am: 7aa1bc6b7d am: 4509ec1ef0 am: 536ab4610a (3f37aaf1) · Commits · e / os / android_frameworks_base

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

+2 −28

Original line number	Diff line number	Diff line
		@@ -23,7 +23,6 @@ import android.content.Intent
		import android.content.IntentFilter
		import android.content.pm.PackageManager
		import android.media.MediaDescription
		import android.media.session.MediaController
		import android.os.UserHandle
		import android.provider.Settings
		import android.service.media.MediaBrowserService
		@@ -171,6 +170,7 @@ class MediaResumeListener @Inject constructor(
		if (useMediaResumption) {
		// If this had been started from a resume state, disconnect now that it's live
		mediaBrowser?.disconnect()
		mediaBrowser = null
		// If we don't have a resume action, check if we haven't already
		if (data.resumeAction == null && !data.hasCheckedForResume &&
		!blockedApps.contains(data.packageName) && data.isLocalSession) {
		@@ -201,7 +201,6 @@ class MediaResumeListener @Inject constructor(
		*/
		private fun tryUpdateResumptionList(key: String, componentName: ComponentName) {
		Log.d(TAG, "Testing if we can connect to $componentName")
		mediaBrowser?.disconnect()
		mediaBrowser = mediaBrowserFactory.create(
		object : ResumeMediaBrowser.Callback() {
		override fun onConnected() {
		@@ -211,7 +210,6 @@ class MediaResumeListener @Inject constructor(
		override fun onError() {
		Log.e(TAG, "Cannot resume with $componentName")
		mediaDataManager.setResumeAction(key, null)
		mediaBrowser?.disconnect()
		mediaBrowser = null
		}

		@@ -224,7 +222,6 @@ class MediaResumeListener @Inject constructor(
		Log.d(TAG, "Can get resumable media from $componentName")
		mediaDataManager.setResumeAction(key, getResumeAction(componentName))
		updateResumptionList(componentName)
		mediaBrowser?.disconnect()
		mediaBrowser = null
		}
		},
		@@ -262,30 +259,7 @@ class MediaResumeListener @Inject constructor(
		*/
		private fun getResumeAction(componentName: ComponentName): Runnable {
		return Runnable {
		mediaBrowser?.disconnect()
		mediaBrowser = mediaBrowserFactory.create(
		object : ResumeMediaBrowser.Callback() {
		override fun onConnected() {
		if (mediaBrowser?.token == null) {
		Log.e(TAG, "Error after connect")
		mediaBrowser?.disconnect()
		mediaBrowser = null
		return
		}
		Log.d(TAG, "Connected for restart $componentName")
		val controller = MediaController(context, mediaBrowser!!.token)
		val controls = controller.transportControls
		controls.prepare()
		controls.play()
		}

		override fun onError() {
		Log.e(TAG, "Resume failed for $componentName")
		mediaBrowser?.disconnect()
		mediaBrowser = null
		}
		},
		componentName)
		mediaBrowser = mediaBrowserFactory.create(null, componentName)
		mediaBrowser?.restart()
		}
		}

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

+58 −23

Original line number	Diff line number	Diff line
		@@ -16,6 +16,7 @@

		package com.android.systemui.media;

		import android.annotation.Nullable;
		import android.app.PendingIntent;
		import android.content.ComponentName;
		import android.content.Context;
		@@ -47,7 +48,7 @@ public class ResumeMediaBrowser {

		private static final String TAG = "ResumeMediaBrowser";
		private final Context mContext;
		private final Callback mCallback;
		@Nullable private final Callback mCallback;
		private MediaBrowserFactory mBrowserFactory;
		private MediaBrowser mMediaBrowser;
		private ComponentName mComponentName;
		@@ -58,8 +59,8 @@ public class ResumeMediaBrowser {
		* @param callback used to report media items found
		* @param componentName Component name of the MediaBrowserService this browser will connect to
		*/
		public ResumeMediaBrowser(Context context, Callback callback, ComponentName componentName,
		MediaBrowserFactory browserFactory) {
		public ResumeMediaBrowser(Context context, @Nullable Callback callback,
		ComponentName componentName, MediaBrowserFactory browserFactory) {
		mContext = context;
		mCallback = callback;
		mComponentName = componentName;
		@@ -93,27 +94,35 @@ public class ResumeMediaBrowser {
		List<MediaBrowser.MediaItem> children) {
		if (children.size() == 0) {
		Log.d(TAG, "No children found for " + mComponentName);
		if (mCallback != null) {
		mCallback.onError();
		}
		} else {
		// We ask apps to return a playable item as the first child when sending
		// a request with EXTRA_RECENT; if they don't, no resume controls
		MediaBrowser.MediaItem child = children.get(0);
		MediaDescription desc = child.getDescription();
		if (child.isPlayable() && mMediaBrowser != null) {
		if (mCallback != null) {
		mCallback.addTrack(desc, mMediaBrowser.getServiceComponent(),
		ResumeMediaBrowser.this);
		}
		} else {
		Log.d(TAG, "Child found but not playable for " + mComponentName);
		if (mCallback != null) {
		mCallback.onError();
		}
		}
		}
		disconnect();
		}

		@Override
		public void onError(String parentId) {
		Log.d(TAG, "Subscribe error for " + mComponentName + ": " + parentId);
		if (mCallback != null) {
		mCallback.onError();
		}
		disconnect();
		}

		@@ -121,7 +130,9 @@ public class ResumeMediaBrowser {
		public void onError(String parentId, Bundle options) {
		Log.d(TAG, "Subscribe error for " + mComponentName + ": " + parentId
		+ ", options: " + options);
		if (mCallback != null) {
		mCallback.onError();
		}
		disconnect();
		}
		};
		@@ -138,14 +149,21 @@ public class ResumeMediaBrowser {
		Log.d(TAG, "Service connected for " + mComponentName);
		if (mMediaBrowser != null && mMediaBrowser.isConnected()) {
		String root = mMediaBrowser.getRoot();
		if (!TextUtils.isEmpty(root) && mMediaBrowser != null) {
		if (!TextUtils.isEmpty(root)) {
		if (mCallback != null) {
		mCallback.onConnected();
		}
		if (mMediaBrowser != null) {
		mMediaBrowser.subscribe(root, mSubscriptionCallback);
		}
		return;
		}
		}
		if (mCallback != null) {
		mCallback.onError();
		}
		disconnect();
		}

		/**
		* Invoked when the client is disconnected from the media browser.
		@@ -153,7 +171,9 @@ public class ResumeMediaBrowser {
		@Override
		public void onConnectionSuspended() {
		Log.d(TAG, "Connection suspended for " + mComponentName);
		if (mCallback != null) {
		mCallback.onError();
		}
		disconnect();
		}

		@@ -163,16 +183,18 @@ public class ResumeMediaBrowser {
		@Override
		public void onConnectionFailed() {
		Log.d(TAG, "Connection failed for " + mComponentName);
		if (mCallback != null) {
		mCallback.onError();
		}
		disconnect();
		}
		};

		/**
		* Disconnect the media browser. This should be called after restart or testConnection have
		* completed to close the connection.
		* Disconnect the media browser. This should be done after callbacks have completed to
		* disconnect from the media browser service.
		*/
		public void disconnect() {
		protected void disconnect() {
		if (mMediaBrowser != null) {
		mMediaBrowser.disconnect();
		}
		@@ -183,7 +205,8 @@ public class ResumeMediaBrowser {
		* Connects to the MediaBrowserService and starts playback.
		* ResumeMediaBrowser.Callback#onError or ResumeMediaBrowser.Callback#onConnected will be called
		* depending on whether it was successful.
		* ResumeMediaBrowser#disconnect should be called after this to ensure the connection is closed.
		* If the connection is successful, the listener should call ResumeMediaBrowser#disconnect after
		* getting a media update from the app
		*/
		public void restart() {
		disconnect();
		@@ -195,7 +218,10 @@ public class ResumeMediaBrowser {
		public void onConnected() {
		Log.d(TAG, "Connected for restart " + mMediaBrowser.isConnected());
		if (mMediaBrowser == null \|\| !mMediaBrowser.isConnected()) {
		if (mCallback != null) {
		mCallback.onError();
		}
		disconnect();
		return;
		}
		MediaSession.Token token = mMediaBrowser.getSessionToken();
		@@ -203,18 +229,27 @@ public class ResumeMediaBrowser {
		controller.getTransportControls();
		controller.getTransportControls().prepare();
		controller.getTransportControls().play();
		if (mCallback != null) {
		mCallback.onConnected();
		}
		// listener should disconnect after media player update
		}

		@Override
		public void onConnectionFailed() {
		if (mCallback != null) {
		mCallback.onError();
		}
		disconnect();
		}

		@Override
		public void onConnectionSuspended() {
		if (mCallback != null) {
		mCallback.onError();
		}
		disconnect();
		}
		}, rootHints);
		mMediaBrowser.connect();
		}

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

+36 −3

Original line number	Diff line number	Diff line
		@@ -89,6 +89,7 @@ class MediaResumeListenerTest : SysuiTestCase() {
		@Mock private lateinit var dumpManager: DumpManager

		@Captor lateinit var callbackCaptor: ArgumentCaptor<ResumeMediaBrowser.Callback>
		@Captor lateinit var actionCaptor: ArgumentCaptor<Runnable>

		private lateinit var executor: FakeExecutor
		private lateinit var data: MediaData
		@@ -224,9 +225,6 @@ class MediaResumeListenerTest : SysuiTestCase() {
		// But we do not tell it to add new controls
		verify(mediaDataManager, never())
		.addResumptionControls(anyInt(), any(), any(), any(), any(), any(), any())

		// Finally, make sure the resume browser disconnected
		verify(resumeBrowser).disconnect()
		}

		@Test
		@@ -267,4 +265,39 @@ class MediaResumeListenerTest : SysuiTestCase() {
		verify(mediaDataManager, times(3)).addResumptionControls(anyInt(),
		any(), any(), any(), any(), any(), eq(PACKAGE_NAME))
		}

		@Test
		fun testGetResumeAction_restarts() {
		// Set up mocks to successfully find a MBS that returns valid media
		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.queryIntentServices(any(), anyInt())).thenReturn(resumeInfo)

		val description = MediaDescription.Builder().setTitle(TITLE).build()
		val component = ComponentName(PACKAGE_NAME, CLASS_NAME)
		whenever(resumeBrowser.testConnection()).thenAnswer {
		callbackCaptor.value.addTrack(description, component, resumeBrowser)
		}

		// When media data is loaded that has not been checked yet, and does have a MBS
		val dataCopy = data.copy(resumeAction = null, hasCheckedForResume = false)
		resumeListener.onMediaDataLoaded(KEY, null, dataCopy)

		// Then we test whether the service is valid and set the resume action
		executor.runAllReady()
		verify(resumeBrowser).testConnection()
		verify(mediaDataManager).setResumeAction(eq(KEY), capture(actionCaptor))

		// When the resume action is run
		actionCaptor.value.run()

		// Then we call restart
		verify(resumeBrowser).restart()
		}
		}
		No newline at end of file

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

+15 −10

Original line number	Diff line number	Diff line
		@@ -91,8 +91,9 @@ public class ResumeMediaBrowserTest : SysuiTestCase() {
		setupBrowserFailed()
		resumeBrowser.testConnection()

		// Then it calls onError
		// Then it calls onError and disconnects
		verify(callback).onError()
		verify(browser).disconnect()
		}

		@Test
		@@ -111,8 +112,9 @@ public class ResumeMediaBrowserTest : SysuiTestCase() {
		setupBrowserConnectionNoResults()
		resumeBrowser.testConnection()

		// Then it calls onError
		// Then it calls onError and disconnects
		verify(callback).onError()
		verify(browser).disconnect()
		}

		@Test
		@@ -132,8 +134,9 @@ public class ResumeMediaBrowserTest : SysuiTestCase() {
		setupBrowserFailed()
		resumeBrowser.findRecentMedia()

		// Then it calls onError
		// Then it calls onError and disconnects
		verify(callback).onError()
		verify(browser).disconnect()
		}

		@Test
		@@ -143,8 +146,9 @@ public class ResumeMediaBrowserTest : SysuiTestCase() {
		whenever(browser.getRoot()).thenReturn(null)
		resumeBrowser.findRecentMedia()

		// Then it calls onError
		// Then it calls onError and disconnects
		verify(callback).onError()
		verify(browser).disconnect()
		}

		@Test
		@@ -163,8 +167,9 @@ public class ResumeMediaBrowserTest : SysuiTestCase() {
		setupBrowserConnectionNoResults()
		resumeBrowser.findRecentMedia()

		// Then it calls onError
		// Then it calls onError and disconnects
		verify(callback).onError()
		verify(browser).disconnect()
		}

		@Test
		@@ -173,8 +178,9 @@ public class ResumeMediaBrowserTest : SysuiTestCase() {
		setupBrowserConnectionNotPlayable()
		resumeBrowser.findRecentMedia()

		// Then it calls onError
		// Then it calls onError and disconnects
		verify(callback).onError()
		verify(browser).disconnect()
		}

		@Test
		@@ -193,8 +199,9 @@ public class ResumeMediaBrowserTest : SysuiTestCase() {
		setupBrowserFailed()
		resumeBrowser.restart()

		// Then it calls onError
		// Then it calls onError and disconnects
		verify(callback).onError()
		verify(browser).disconnect()
		}

		@Test
		@@ -202,13 +209,11 @@ public class ResumeMediaBrowserTest : SysuiTestCase() {
		// When restart is called and we connect successfully
		setupBrowserConnection()
		resumeBrowser.restart()
		verify(callback).onConnected()

		// Then it creates a new controller and sends play command
		verify(transportControls).prepare()
		verify(transportControls).play()

		// Then it calls onConnected
		verify(callback).onConnected()
		}

		/**