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

Issue 902519 link

Starred by 1 user
Status: Fixed
Owner:
beccahughes@chromium.org
Closed: Jan 8
Cc:
█ denizz@chromium.org
beccahughes@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug

Blocked on:
issue 910279


Sign in to add a comment

MediaSessionAccelerators feature causes AudioPlayerBrowserTest.NativeMediaKey to fail

Project Member Reported by tapted@chromium.org, Nov 6 
Chrome Version       : 72.0.3595.2
OS Version: Chrome

What steps will reproduce the problem?
1. browser_tests --gtest_filter=AudioPlayerBrowserTest.NativeMediaKey --enable-features=MediaSessionAccelerators

What is the expected result?

Test should pass


What happens instead of that?

Doesn't.



I don't think the CrOS AudioPlayer hooks any special permission to do this, so it's possible stuff on the wider web is broken too.

I think the MediaSessionAccelerators handling is swallowing the event before WebContents gets a chance to see it and, e.g., preventDefault() on the keydown event (so that the media session *never* sees it).

The test landed recently as a regression test for  Issue 899094

Comment 1 by tapted@chromium.org, Nov 7

Description: Show this description

Comment 2 by beccahughes@chromium.org, Nov 29

Owner: beccahughes@chromium.org
Status: Assigned (was: Untriaged)

Comment 3 by beccahughes@chromium.org, Nov 29

Blockedon: 910279
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 4 

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

commit 02b792508c0a9fe350a1cb8d436df2ee818a59c6
Author: Becca Hughes <beccahughes@chromium.org>
Date: Fri Jan 04 23:23:54 2019

[Media Session] Make controllable if we have playback state

This changes MediaSession to be controllable if we
have set a playback state through the routed service.

This is to fix cases where a site may not be controllable
if the media is shorter than 7 seconds but we want it to be
controllable (e.g. if it is a media app).

BUG= 902519 

Change-Id: I86ef8ffca0fb6a3e6c11b2ca90e2a3c020cb9d70
Reviewed-on: https://chromium-review.googlesource.com/c/1393444
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620102}
[modify] https://crrev.com/02b792508c0a9fe350a1cb8d436df2ee818a59c6/content/browser/media/session/media_session_impl.cc
[modify] https://crrev.com/02b792508c0a9fe350a1cb8d436df2ee818a59c6/content/browser/media/session/media_session_impl_browsertest.cc
Project Member

Comment 5 by bugdroid1@chromium.org, Jan 8 

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

commit 816d56a35764040ec688a523b171a8f1425a8afb
Author: Becca Hughes <beccahughes@chromium.org>
Date: Tue Jan 08 19:26:23 2019

[CroS Players] Set the playback state

Set the playback state through the Media Session API
for both the audio and video player. This will ensure
these apps will always be controllable even if they
are playing short duration media.

BUG= 902519 

Change-Id: Ib09e4e1edbfc58f1566397bd39d02b3f907e63e7
Reviewed-on: https://chromium-review.googlesource.com/c/1393431
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620831}
[modify] https://crrev.com/816d56a35764040ec688a523b171a8f1425a8afb/chrome/browser/chromeos/file_manager/video_player_browsertest.cc
[add] https://crrev.com/816d56a35764040ec688a523b171a8f1425a8afb/third_party/closure_compiler/externs/mediasession.js
[modify] https://crrev.com/816d56a35764040ec688a523b171a8f1425a8afb/ui/file_manager/audio_player/elements/audio_player.js
[modify] https://crrev.com/816d56a35764040ec688a523b171a8f1425a8afb/ui/file_manager/audio_player/js/BUILD.gn
[modify] https://crrev.com/816d56a35764040ec688a523b171a8f1425a8afb/ui/file_manager/audio_player/js/audio_player.js
[modify] https://crrev.com/816d56a35764040ec688a523b171a8f1425a8afb/ui/file_manager/audio_player/js/audio_player_scripts.js
[modify] https://crrev.com/816d56a35764040ec688a523b171a8f1425a8afb/ui/file_manager/base/js/BUILD.gn
[add] https://crrev.com/816d56a35764040ec688a523b171a8f1425a8afb/ui/file_manager/base/js/mediasession_types.js
[modify] https://crrev.com/816d56a35764040ec688a523b171a8f1425a8afb/ui/file_manager/integration_tests/video_player/click_control_buttons.js
[modify] https://crrev.com/816d56a35764040ec688a523b171a8f1425a8afb/ui/file_manager/video_player/js/BUILD.gn
[modify] https://crrev.com/816d56a35764040ec688a523b171a8f1425a8afb/ui/file_manager/video_player/js/cast/BUILD.gn
[modify] https://crrev.com/816d56a35764040ec688a523b171a8f1425a8afb/ui/file_manager/video_player/js/video_player.js
[modify] https://crrev.com/816d56a35764040ec688a523b171a8f1425a8afb/ui/file_manager/video_player/js/video_player_scripts.js

Comment 6 by beccahughes@chromium.org, Jan 8

Status: Fixed (was: Assigned)
The following will work now:

browser_tests --gtest_filter="*PlayerBrowserTest.NativeMediaKey" --enable-features=MediaSessionAccelerators --enable-audio-focus --enable-blink-features=MediaSession
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 14 

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

commit eb1231b06e9ff481a67faed718732c09fb99e6cc
Author: Dominic Battré <battre@chromium.org>
Date: Mon Jan 14 09:23:00 2019

Revert "[CroS Players] Set the playback state"

This reverts commit 816d56a35764040ec688a523b171a8f1425a8afb.

Reason for revert: Test that was introduced in this CL is flaky. See crbug.com/921418

Original change's description:
> [CroS Players] Set the playback state
> 
> Set the playback state through the Media Session API
> for both the audio and video player. This will ensure
> these apps will always be controllable even if they
> are playing short duration media.
> 
> BUG= 902519 
> 
> Change-Id: Ib09e4e1edbfc58f1566397bd39d02b3f907e63e7
> Reviewed-on: https://chromium-review.googlesource.com/c/1393431
> Commit-Queue: Becca Hughes <beccahughes@chromium.org>
> Reviewed-by: Naoki Fukino <fukino@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#620831}

TBR=fukino@chromium.org,beccahughes@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  902519 , 921418
Change-Id: I1935b1d41242144eb87cc2df4ae759269c979eb0
Reviewed-on: https://chromium-review.googlesource.com/c/1406814
Reviewed-by: Dominic Battré <battre@chromium.org>
Commit-Queue: Dominic Battré <battre@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622401}
[modify] https://crrev.com/eb1231b06e9ff481a67faed718732c09fb99e6cc/chrome/browser/chromeos/file_manager/video_player_browsertest.cc
[delete] https://crrev.com/38406a29e48621749daf6a3e57d5ea0939748fc9/third_party/closure_compiler/externs/mediasession.js
[modify] https://crrev.com/eb1231b06e9ff481a67faed718732c09fb99e6cc/ui/file_manager/audio_player/elements/audio_player.js
[modify] https://crrev.com/eb1231b06e9ff481a67faed718732c09fb99e6cc/ui/file_manager/audio_player/js/BUILD.gn
[modify] https://crrev.com/eb1231b06e9ff481a67faed718732c09fb99e6cc/ui/file_manager/audio_player/js/audio_player.js
[modify] https://crrev.com/eb1231b06e9ff481a67faed718732c09fb99e6cc/ui/file_manager/audio_player/js/audio_player_scripts.js
[modify] https://crrev.com/eb1231b06e9ff481a67faed718732c09fb99e6cc/ui/file_manager/base/js/BUILD.gn
[delete] https://crrev.com/38406a29e48621749daf6a3e57d5ea0939748fc9/ui/file_manager/base/js/mediasession_types.js
[modify] https://crrev.com/eb1231b06e9ff481a67faed718732c09fb99e6cc/ui/file_manager/integration_tests/video_player/click_control_buttons.js
[modify] https://crrev.com/eb1231b06e9ff481a67faed718732c09fb99e6cc/ui/file_manager/video_player/js/BUILD.gn
[modify] https://crrev.com/eb1231b06e9ff481a67faed718732c09fb99e6cc/ui/file_manager/video_player/js/cast/BUILD.gn
[modify] https://crrev.com/eb1231b06e9ff481a67faed718732c09fb99e6cc/ui/file_manager/video_player/js/video_player.js
[modify] https://crrev.com/eb1231b06e9ff481a67faed718732c09fb99e6cc/ui/file_manager/video_player/js/video_player_scripts.js
Project Member

Comment 8 by bugdroid1@chromium.org, Jan 16 (6 days ago) 

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

commit f5b540aedd4708e1a9beec346794f49594677b7b
Author: Becca Hughes <beccahughes@chromium.org>
Date: Wed Jan 16 21:47:19 2019

Reland "[CroS Players] Set the playback state"

This is a reland of 816d56a35764040ec688a523b171a8f1425a8afb

Original change's description:
> [CroS Players] Set the playback state
>
> Set the playback state through the Media Session API
> for both the audio and video player. This will ensure
> these apps will always be controllable even if they
> are playing short duration media.
>
> BUG= 902519 
>
> Change-Id: Ib09e4e1edbfc58f1566397bd39d02b3f907e63e7
> Reviewed-on: https://chromium-review.googlesource.com/c/1393431
> Commit-Queue: Becca Hughes <beccahughes@chromium.org>
> Reviewed-by: Naoki Fukino <fukino@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#620831}

Bug:  902519 
Change-Id: I494a5bc837838426286da191c6e0ff6bc5af5147
Reviewed-on: https://chromium-review.googlesource.com/c/1409804
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Commit-Queue: Becca Hughes <beccahughes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623383}
[modify] https://crrev.com/f5b540aedd4708e1a9beec346794f49594677b7b/chrome/browser/chromeos/file_manager/video_player_browsertest.cc
[add] https://crrev.com/f5b540aedd4708e1a9beec346794f49594677b7b/chrome/test/data/chromeos/file_manager/video_long.ogv
[add] https://crrev.com/f5b540aedd4708e1a9beec346794f49594677b7b/third_party/closure_compiler/externs/mediasession.js
[modify] https://crrev.com/f5b540aedd4708e1a9beec346794f49594677b7b/ui/file_manager/audio_player/elements/audio_player.js
[modify] https://crrev.com/f5b540aedd4708e1a9beec346794f49594677b7b/ui/file_manager/audio_player/js/BUILD.gn
[modify] https://crrev.com/f5b540aedd4708e1a9beec346794f49594677b7b/ui/file_manager/audio_player/js/audio_player.js
[modify] https://crrev.com/f5b540aedd4708e1a9beec346794f49594677b7b/ui/file_manager/audio_player/js/audio_player_scripts.js
[modify] https://crrev.com/f5b540aedd4708e1a9beec346794f49594677b7b/ui/file_manager/base/js/BUILD.gn
[add] https://crrev.com/f5b540aedd4708e1a9beec346794f49594677b7b/ui/file_manager/base/js/mediasession_types.js
[modify] https://crrev.com/f5b540aedd4708e1a9beec346794f49594677b7b/ui/file_manager/integration_tests/test_util.js
[modify] https://crrev.com/f5b540aedd4708e1a9beec346794f49594677b7b/ui/file_manager/integration_tests/video_player/click_control_buttons.js
[modify] https://crrev.com/f5b540aedd4708e1a9beec346794f49594677b7b/ui/file_manager/video_player/js/BUILD.gn
[modify] https://crrev.com/f5b540aedd4708e1a9beec346794f49594677b7b/ui/file_manager/video_player/js/cast/BUILD.gn
[modify] https://crrev.com/f5b540aedd4708e1a9beec346794f49594677b7b/ui/file_manager/video_player/js/video_player.js
[modify] https://crrev.com/f5b540aedd4708e1a9beec346794f49594677b7b/ui/file_manager/video_player/js/video_player_scripts.js
►

Sign in to add a comment