Commit ac270dea authored by Mounir Lamouri's avatar Mounir Lamouri Committed by Commit Bot

Media Notifications: allow to use 5 actions for pre-N devices.

There was a hard coded limitation to 3 actions because the pre-N
notifications do not look great with more than 3 actions but it is
breaking consistency for websites using the Media Session API.

This is also renaming `isRunningN()` to `isRunningAtLeastN()`.

Bug: 775471
Change-Id: I2e9289ff0daa987acf0b656a239069676b03d722
Reviewed-on: https://chromium-review.googlesource.com/747221Reviewed-by: default avatarTommy Steimel <steimel@chromium.org>
Commit-Queue: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513107}
parent cf79f965
...@@ -80,13 +80,13 @@ public class MediaNotificationManager { ...@@ -80,13 +80,13 @@ public class MediaNotificationManager {
private static final int COMPACT_VIEW_ACTIONS_COUNT = 3; private static final int COMPACT_VIEW_ACTIONS_COUNT = 3;
// The maximum number of actions in BigView media notification. // The maximum number of actions in BigView media notification.
private static final int BIG_VIEW_ACTIONS_COUNT = isRunningN() ? 5 : 3; private static final int BIG_VIEW_ACTIONS_COUNT = 5;
// Maps the notification ids to their corresponding notification managers. // Maps the notification ids to their corresponding notification managers.
private static SparseArray<MediaNotificationManager> sManagers; private static SparseArray<MediaNotificationManager> sManagers;
// Overrides N detection. The production code will use |null|, which uses the Android version // Overrides N detection. The production code will use |null|, which uses the Android version
// code. Otherwise, |isRunningN()| will return whatever value is set. // code. Otherwise, |isRunningAtLeastN()| will return whatever value is set.
@VisibleForTesting @VisibleForTesting
static Boolean sOverrideIsRunningNForTesting; static Boolean sOverrideIsRunningNForTesting;
...@@ -708,7 +708,7 @@ public class MediaNotificationManager { ...@@ -708,7 +708,7 @@ public class MediaNotificationManager {
sManagers.put(notificationId, manager); sManagers.put(notificationId, manager);
} }
private static boolean isRunningN() { private static boolean isRunningAtLeastN() {
return (sOverrideIsRunningNForTesting != null) return (sOverrideIsRunningNForTesting != null)
? sOverrideIsRunningNForTesting ? sOverrideIsRunningNForTesting
: Build.VERSION.SDK_INT >= Build.VERSION_CODES.N; : Build.VERSION.SDK_INT >= Build.VERSION_CODES.N;
...@@ -1067,7 +1067,7 @@ public class MediaNotificationManager { ...@@ -1067,7 +1067,7 @@ public class MediaNotificationManager {
builder.setLargeIcon(null); builder.setLargeIcon(null);
} else if (mMediaNotificationInfo.notificationLargeIcon != null) { } else if (mMediaNotificationInfo.notificationLargeIcon != null) {
builder.setLargeIcon(mMediaNotificationInfo.notificationLargeIcon); builder.setLargeIcon(mMediaNotificationInfo.notificationLargeIcon);
} else if (!isRunningN()) { } else if (!isRunningAtLeastN()) {
if (mDefaultNotificationLargeIcon == null) { if (mDefaultNotificationLargeIcon == null) {
int resourceId = (mMediaNotificationInfo.defaultNotificationLargeIcon != 0) int resourceId = (mMediaNotificationInfo.defaultNotificationLargeIcon != 0)
? mMediaNotificationInfo.defaultNotificationLargeIcon ? mMediaNotificationInfo.defaultNotificationLargeIcon
...@@ -1127,7 +1127,7 @@ public class MediaNotificationManager { ...@@ -1127,7 +1127,7 @@ public class MediaNotificationManager {
private void setMediaStyleNotificationText(ChromeNotificationBuilder builder) { private void setMediaStyleNotificationText(ChromeNotificationBuilder builder) {
builder.setContentTitle(mMediaNotificationInfo.metadata.getTitle()); builder.setContentTitle(mMediaNotificationInfo.metadata.getTitle());
String artistAndAlbumText = getArtistAndAlbumText(mMediaNotificationInfo.metadata); String artistAndAlbumText = getArtistAndAlbumText(mMediaNotificationInfo.metadata);
if (isRunningN() || !artistAndAlbumText.isEmpty()) { if (isRunningAtLeastN() || !artistAndAlbumText.isEmpty()) {
builder.setContentText(artistAndAlbumText); builder.setContentText(artistAndAlbumText);
builder.setSubText(mMediaNotificationInfo.origin); builder.setSubText(mMediaNotificationInfo.origin);
} else { } else {
...@@ -1150,9 +1150,6 @@ public class MediaNotificationManager { ...@@ -1150,9 +1150,6 @@ public class MediaNotificationManager {
* *
* The method assumes STOP cannot coexist with switch track actions and seeking actions. It also * The method assumes STOP cannot coexist with switch track actions and seeking actions. It also
* assumes PLAY and PAUSE cannot coexist. * assumes PLAY and PAUSE cannot coexist.
*
* TODO(zqzhang): get UX feedback to decide which actions to select when the number of actions
* is greater than that can be displayed. See https://crbug.com/667500
*/ */
private List<Integer> computeBigViewActions(Set<Integer> actions) { private List<Integer> computeBigViewActions(Set<Integer> actions) {
// STOP cannot coexist with switch track actions and seeking actions. // STOP cannot coexist with switch track actions and seeking actions.
...@@ -1164,6 +1161,8 @@ public class MediaNotificationManager { ...@@ -1164,6 +1161,8 @@ public class MediaNotificationManager {
// PLAY and PAUSE cannot coexist. // PLAY and PAUSE cannot coexist.
assert !actions.contains(MediaSessionAction.PLAY) assert !actions.contains(MediaSessionAction.PLAY)
|| !actions.contains(MediaSessionAction.PAUSE); || !actions.contains(MediaSessionAction.PAUSE);
// There can't be move actions than BIG_VIEW_ACTIONS_COUNT.
assert actions.size() <= BIG_VIEW_ACTIONS_COUNT;
int[] actionByOrder = { int[] actionByOrder = {
MediaSessionAction.PREVIOUS_TRACK, MediaSessionAction.PREVIOUS_TRACK,
...@@ -1175,35 +1174,10 @@ public class MediaNotificationManager { ...@@ -1175,35 +1174,10 @@ public class MediaNotificationManager {
CUSTOM_MEDIA_SESSION_ACTION_STOP, CUSTOM_MEDIA_SESSION_ACTION_STOP,
}; };
// First, select at most |BIG_VIEW_ACTIONS_COUNT| actions by priority. // Sort the actions based on the expected ordering in the UI.
Set<Integer> selectedActions;
if (actions.size() <= BIG_VIEW_ACTIONS_COUNT) {
selectedActions = actions;
} else {
selectedActions = new HashSet<>();
if (actions.contains(CUSTOM_MEDIA_SESSION_ACTION_STOP)) {
selectedActions.add(CUSTOM_MEDIA_SESSION_ACTION_STOP);
} else {
if (actions.contains(MediaSessionAction.PLAY)) {
selectedActions.add(MediaSessionAction.PLAY);
} else {
selectedActions.add(MediaSessionAction.PAUSE);
}
if (actions.contains(MediaSessionAction.PREVIOUS_TRACK)
&& actions.contains(MediaSessionAction.NEXT_TRACK)) {
selectedActions.add(MediaSessionAction.PREVIOUS_TRACK);
selectedActions.add(MediaSessionAction.NEXT_TRACK);
} else {
selectedActions.add(MediaSessionAction.SEEK_BACKWARD);
selectedActions.add(MediaSessionAction.SEEK_FORWARD);
}
}
}
// Second, sort the selected actions.
List<Integer> sortedActions = new ArrayList<>(); List<Integer> sortedActions = new ArrayList<>();
for (int action : actionByOrder) { for (int action : actionByOrder) {
if (selectedActions.contains(action)) sortedActions.add(action); if (actions.contains(action)) sortedActions.add(action);
} }
return sortedActions; return sortedActions;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment