MediaSession: pause players in non-routed frames when pause action is received |
||||||
Issue descriptionThis is an issue when multiple frames on a page is playing media, and only the MediaSession of one frame is routed. Only the routed frame will receive the pause action. Therefore after the user clicks the pause button, only the routed frame will stop playing, while all the non-routed frames are still playing. So we still show a pause button. We should probably send a pause event to the page, and pause all players in the other frames. Maybe we need to merge to M57 if the patch is not huge.
,
Jan 27 2017
It's easier to handle it in the C++ layer, but we need to intercept the signal in DidReceiveAction. It's a bit unclean. Or maybe we should handle all play/pause related logic in C++ instead of Java?
,
Jan 30 2017
I've written a prototype CL: https://codereview.chromium.org/2660263002/ Will add tests if you think it is OK. FYI, if we don't do this fix, the current behavior will be (which is OK but weird): 1. A page has multiple frames playing media, only one of the frames is using MediaSession API. 2. The user presses PAUSE, the event is sent to the frame which uses the API. 3. The frame stops playing, but all other frames are still playing. Therefore the notification falls back to favicon and page title, and still with a PAUSE icon. 4. The user presses PAUSE again, as currently no service is routed, we fallback to suspend the entire page.
,
Feb 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ded5aed116f603b4d7e4e5b83abeb25e7f802198 commit ded5aed116f603b4d7e4e5b83abeb25e7f802198 Author: zqzhang <zqzhang@chromium.org> Date: Fri Feb 10 15:20:37 2017 [Media>Session] Pause all players for non-routed frames when receiving PAUSE action Previously, when a page uses MediaSession API, the PAUSE action is only sent to the frame that is routed, other frames may be still playing media, causing the session to be still active. In this CL, apart from sending the PAUSE action to the frame, we pause the players in all non-routed frames. BUG= 685978 Review-Url: https://codereview.chromium.org/2660263002 Cr-Commit-Position: refs/heads/master@{#449622} [modify] https://crrev.com/ded5aed116f603b4d7e4e5b83abeb25e7f802198/content/browser/media/session/media_session_impl.cc [modify] https://crrev.com/ded5aed116f603b4d7e4e5b83abeb25e7f802198/content/browser/media/session/media_session_impl_service_routing_unittest.cc
,
Feb 10 2017
,
Feb 10 2017
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
,
Feb 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9a550d13567114af60c8f287393c3bec87c3d79c commit 9a550d13567114af60c8f287393c3bec87c3d79c Author: Zhiqiang Zhang <zqzhang@google.com> Date: Fri Feb 10 16:07:28 2017 [Media>Session] Pause all players for non-routed frames when receiving PAUSE action Previously, when a page uses MediaSession API, the PAUSE action is only sent to the frame that is routed, other frames may be still playing media, causing the session to be still active. In this CL, apart from sending the PAUSE action to the frame, we pause the players in all non-routed frames. BUG= 685978 Review-Url: https://codereview.chromium.org/2660263002 Cr-Commit-Position: refs/heads/master@{#449622} (cherry picked from commit ded5aed116f603b4d7e4e5b83abeb25e7f802198) Review-Url: https://codereview.chromium.org/2685373002 . Cr-Commit-Position: refs/branch-heads/2987@{#437} Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943} [modify] https://crrev.com/9a550d13567114af60c8f287393c3bec87c3d79c/content/browser/media/session/media_session_impl.cc [modify] https://crrev.com/9a550d13567114af60c8f287393c3bec87c3d79c/content/browser/media/session/media_session_impl_service_routing_unittest.cc
,
Feb 10 2017
,
Feb 15 2017
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by mlamouri@chromium.org
, Jan 27 2017