[Media, Android] Fix lockscreen and other remote controls on KK-
The cause of the crash is the following:
- user starts playback on KK
- user tries to use remote control, e.g. lock screen
- the service gets an intent without the notification id and stops itself
- onServiceDestroy gets called with an invalid notification id so it doesn't clean up the manager
- next time the playback is started, the manager calls getIntent() with an invalid notification id
- the method returns null in this case
The fix: use distinct MediaButtonReceiver subclasses for the two supported notification ids to pass the right one with the intent MediaButtonReceiver sends to the service.
BUG==551411
Review URL: https://codereview.chromium.org/1431023004
Cr-Commit-Position: refs/heads/master@{#358714}
diff --git a/chrome/android/java/AndroidManifest.xml b/chrome/android/java/AndroidManifest.xml
index 0e29e6a..b90bd68b 100644
--- a/chrome/android/java/AndroidManifest.xml
+++ b/chrome/android/java/AndroidManifest.xml
@@ -641,7 +641,12 @@
</service>
- <receiver android:name="org.chromium.chrome.browser.media.ui.MediaButtonReceiver">
+ <receiver android:name="org.chromium.chrome.browser.media.ui.MediaNotificationManager$PlaybackMediaButtonReceiver">
+ <intent-filter>
+ <action android:name="android.intent.action.MEDIA_BUTTON" />
+ </intent-filter>
+ </receiver>
+ <receiver android:name="org.chromium.chrome.browser.media.ui.MediaNotificationManager$PresentationMediaButtonReceiver">
<intent-filter>
<action android:name="android.intent.action.MEDIA_BUTTON" />
</intent-filter>
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaButtonReceiver.java b/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaButtonReceiver.java
index f8f795b..c18df6c 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaButtonReceiver.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaButtonReceiver.java
@@ -19,10 +19,14 @@
* to the service listening to them.
* This is there for backward compatibility with JB_MR0 and JB_MR1.
*/
-public class MediaButtonReceiver extends BroadcastReceiver {
+public abstract class MediaButtonReceiver extends BroadcastReceiver {
private static final String LISTENER_SERVICE_CLASS_NAME =
"org.chromium.chrome.browser.media.ui"
+ "MediaNotificationManager$ListenerService";
+ public static final String EXTRA_NOTIFICATION_ID =
+ "MediaNotificationManager.ListenerService.NOTIFICATION_ID";
+
+ public abstract int getNotificationId();
@Override
public void onReceive(Context context, Intent intent) {
@@ -39,6 +43,7 @@
assert LISTENER_SERVICE_CLASS_NAME.equals(component.getClassName());
intent.setComponent(component);
+ intent.putExtra(EXTRA_NOTIFICATION_ID, getNotificationId());
context.startService(intent);
}
}
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java b/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java
index 2c71d30..8ed80d55 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java
@@ -26,6 +26,7 @@
import android.widget.RemoteViews;
import org.chromium.base.ApiCompatibilityUtils;
+import org.chromium.base.Log;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.tab.Tab;
@@ -37,6 +38,8 @@
*/
public class MediaNotificationManager {
+ private static final String TAG = "cr_MediaNotification";
+
// We're always used on the UI thread but the LOCK is required by lint when creating the
// singleton.
private static final Object LOCK = new Object();
@@ -57,7 +60,7 @@
private static final String ACTION_STOP =
"MediaNotificationManager.ListenerService.STOP";
private static final String EXTRA_NOTIFICATION_ID =
- "MediaNotificationManager.ListenerService.NOTIFICATION_ID";
+ MediaButtonReceiver.EXTRA_NOTIFICATION_ID;
// The notification id this service instance corresponds to.
private int mNotificationId = MediaNotificationInfo.INVALID_ID;
@@ -92,9 +95,22 @@
private boolean processIntent(Intent intent) {
if (intent == null) return false;
- mNotificationId = intent.getIntExtra(
+ int notificationId = intent.getIntExtra(
EXTRA_NOTIFICATION_ID, MediaNotificationInfo.INVALID_ID);
- if (mNotificationId == MediaNotificationInfo.INVALID_ID) return false;
+
+ // The notification id must always be valid and should match the first notification id
+ // the service got via the intent.
+ if (notificationId == MediaNotificationInfo.INVALID_ID
+ || (mNotificationId != MediaNotificationInfo.INVALID_ID
+ && mNotificationId != notificationId)) {
+ Log.w(TAG, "The service intent's notification id is invalid: ", notificationId);
+ return false;
+ }
+
+ // Either the notification id matches or it's the first intent we've got.
+ mNotificationId = notificationId;
+
+ assert mNotificationId != MediaNotificationInfo.INVALID_ID;
MediaNotificationManager manager = getManager(mNotificationId);
if (manager == null || manager.mMediaNotificationInfo == null) return false;
@@ -164,6 +180,28 @@
private static final int NOTIFICATION_ID = R.id.presentation_notification;
}
+ // Two classes to specify the right notification id in the intent.
+
+ /**
+ * This class is used internally but have to be public to be able to launch the service.
+ */
+ public static final class PlaybackMediaButtonReceiver extends MediaButtonReceiver {
+ @Override
+ public int getNotificationId() {
+ return PlaybackListenerService.NOTIFICATION_ID;
+ }
+ }
+
+ /**
+ * This class is used internally but have to be public to be able to launch the service.
+ */
+ public static final class PresentationMediaButtonReceiver extends MediaButtonReceiver {
+ @Override
+ public int getNotificationId() {
+ return PresentationListenerService.NOTIFICATION_ID;
+ }
+ }
+
private static Intent getIntent(Context context, int notificationId) {
Intent intent = null;
if (notificationId == PlaybackListenerService.NOTIFICATION_ID) {
@@ -176,6 +214,19 @@
return intent.putExtra(ListenerService.EXTRA_NOTIFICATION_ID, notificationId);
}
+ private static String getButtonReceiverClassName(int notificationId) {
+ if (notificationId == PlaybackListenerService.NOTIFICATION_ID) {
+ return PlaybackMediaButtonReceiver.class.getName();
+ }
+
+ if (notificationId == PresentationListenerService.NOTIFICATION_ID) {
+ return PresentationMediaButtonReceiver.class.getName();
+ }
+
+ assert false;
+ return null;
+ }
+
/**
* Shows the notification with media controls with the specified media info. Replaces/updates
* the current notification if already showing. Does nothing if |mediaNotificationInfo| hasn't
@@ -535,7 +586,7 @@
mContext,
mContext.getString(R.string.app_name),
new ComponentName(mContext.getPackageName(),
- MediaButtonReceiver.class.getName()),
+ getButtonReceiverClassName(mMediaNotificationInfo.id)),
null);
mediaSession.setFlags(MediaSessionCompat.FLAG_HANDLES_MEDIA_BUTTONS
| MediaSessionCompat.FLAG_HANDLES_TRANSPORT_CONTROLS);
