New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 695967 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Android MediaRouter only (left Chro...
Closed: Feb 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

MediaNotification should always show for the last active tab

Project Member Reported by zqzh...@chromium.org, Feb 24 2017

Issue description

There is an issue when there are two tabs.

1. Tab A starts playing audio, we show notification for Tab A
2. Tab B starts playing audio, we show notification for Tab B
3. Tab A lost audio focus and we update the notification for Tab A

We should block 3 if the new update is from a tab that is paused and the tab id mismatches.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1ee77687069628ebd0bba3ff791a5ee6c8d17b41

commit 1ee77687069628ebd0bba3ff791a5ee6c8d17b41
Author: zqzhang <zqzhang@chromium.org>
Date: Mon Feb 27 14:24:00 2017

[Media>UI] Fix a media notification update issue when audio focus changes between tabs

There's a issue which is caused by the asynchronousness of audio focus.
When tab B gains focus from tab A, we will first receive the event that
A becomes playing, and then B becomes paused. We should ignore the
notification updates when the tab id mismatches the current notification
and the tab is paused.

BUG= 695967 

Review-Url: https://codereview.chromium.org/2714993002
Cr-Commit-Position: refs/heads/master@{#453206}

[modify] https://crrev.com/1ee77687069628ebd0bba3ff791a5ee6c8d17b41/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java
[modify] https://crrev.com/1ee77687069628ebd0bba3ff791a5ee6c8d17b41/chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java

Labels: Merge-Request-57
Project Member

Comment 3 by sheriffbot@chromium.org, Feb 27 2017

Labels: -Merge-Request-57 Hotlist-Merge-Approved Merge-Approved-57
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 4 by bugdroid1@chromium.org, Feb 27 2017

Labels: -merge-approved-57 merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5dfc4b56d28e35a36a63c77bad993850f966a3fd

commit 5dfc4b56d28e35a36a63c77bad993850f966a3fd
Author: Zhiqiang Zhang <zqzhang@google.com>
Date: Mon Feb 27 14:54:20 2017

[Media>UI] Fix a media notification update issue when audio focus changes between tabs

There's a issue which is caused by the asynchronousness of audio focus.
When tab B gains focus from tab A, we will first receive the event that
A becomes playing, and then B becomes paused. We should ignore the
notification updates when the tab id mismatches the current notification
and the tab is paused.

BUG= 695967 

Review-Url: https://codereview.chromium.org/2714993002
Cr-Commit-Position: refs/heads/master@{#453206}
(cherry picked from commit 1ee77687069628ebd0bba3ff791a5ee6c8d17b41)

Review-Url: https://codereview.chromium.org/2717293003 .
Cr-Commit-Position: refs/branch-heads/2987@{#695}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/5dfc4b56d28e35a36a63c77bad993850f966a3fd/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java
[modify] https://crrev.com/5dfc4b56d28e35a36a63c77bad993850f966a3fd/chrome/android/javatests/src/org/chromium/chrome/browser/media/ui/NotificationTitleUpdatedTest.java

Status: Fixed (was: Started)
Cc: krav...@chromium.org
Hi kravula,

Steps to verify:

1. Navigate to https://xxyzzzq.github.io/sandbox/media-session/full-test.html, click "play/pause". The playback should start.
2. Observe the notification.
3. Open a new tab, navigate to https://xxyzzzq.github.io/sandbox/media-session/simple-audio.html. Play the audio. The playback should start.
4. Observe the notification.

Behavior before the fix:
The media notification in step 4 is the same with step 2

Behavior after the fix:
The media notification in step 4 should be different (showing title "Simple Audio Test page" and with a image saying "I am a FavIcon")

Status: Verified (was: Fixed)
Verified in M57-57.0.2987.88 build with above verification steps.

Sign in to add a comment