1357633 - Fennec doesn't request audio focus correctly when user resume media from page, not from media control

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Audio/Video, defect, P1)

Description

[Alastor Wu [:alwu]]

Fork from bug1348085 comment6.

---

STR:
 1. Load YouTube and play any video;
 2. Open the YouTube app and play a video.
 3. Open Nightly again and resume video playback.
 4. Open the YouTube app and resume the video.

Results:
 After the 4th step video playback on Nightly does not pause. Is this to be expected? If yes the issue is fixed then.

Comment 1

[Alastor Wu [:alwu]]

Attachment: Bug 1357633 - part1 : request audio focus again when resume media from page.

When user resume media from page (eg. script call media.play() or clicking play icon), we
should notify audio playing state changed so that we can request audio focus again.

Review commit: https://reviewboard.mozilla.org/r/133868/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/133868/

Comment 2

[Alastor Wu [:alwu]]

Attachment: Bug 1357633 - part2 : add tests.

Based on bug1357639, add new test tasks.

Review commit: https://reviewboard.mozilla.org/r/134282/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/134282/

Comment 3

[Alastor Wu [:alwu]]

Comment on attachment 8861848 [details]
Bug 1357633 - part1 : request audio focus again when resume media from page.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/133868/diff/1-2/

Comment 4

[Alastor Wu [:alwu]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=85fa6f8cc9a917455f41dd2abd43e1cf22cdc17d

Comment 5

[Alastor Wu [:alwu]]

Comment on attachment 8861848 [details]
Bug 1357633 - part1 : request audio focus again when resume media from page.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/133868/diff/2-3/

Comment 6

[Alastor Wu [:alwu]]

Comment on attachment 8862355 [details]
Bug 1357633 - part2 : add tests.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/134282/diff/1-2/

Comment 7

[Alastor Wu [:alwu]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f9dd5459c2cabeccb6c967a28a8ade6062e244f4

Comment 8

[Alastor Wu [:alwu]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=308364d227535959e8f18db402e6abdf99682a99

Comment 9

[Alastor Wu [:alwu]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8c9c75f13bb6a320a5783929c6ff18010902827a

Comment 10

[Alastor Wu [:alwu]]

Comment on attachment 8861848 [details]
Bug 1357633 - part1 : request audio focus again when resume media from page.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/133868/diff/3-4/

Comment 11

[Alastor Wu [:alwu]]

Comment on attachment 8862355 [details]
Bug 1357633 - part2 : add tests.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/134282/diff/2-3/

Comment 12

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8862355 [details]
Bug 1357633 - part2 : add tests.

https://reviewboard.mozilla.org/r/134282/#review139110

::: mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/MediaPlaybackTest.java:238
(Diff revision 3)
> +     * Can communicte with JS bey getJS(), but caller should create and destroy
> +     * JSBridge manually.
> +     */
> +    protected JavascriptBridge getJS() {
> +        if (mJs == null) {
> +            mAsserter.ok(false,

How about "mAsserter.ok(mJs != null)"?

Comment 13

[Andrea Marchesini [:baku]]

Comment on attachment 8861848 [details]
Bug 1357633 - part1 : request audio focus again when resume media from page.

https://reviewboard.mozilla.org/r/133868/#review139634

Comment 14

[Alastor Wu [:alwu]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=24b23632edab0aed6e58f1380c1a6cb188dda7f6

Comment 15

[Alastor Wu [:alwu]]

Comment on attachment 8861848 [details]
Bug 1357633 - part1 : request audio focus again when resume media from page.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/133868/diff/4-5/

Comment 16

[Alastor Wu [:alwu]]

Comment on attachment 8862355 [details]
Bug 1357633 - part2 : add tests.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/134282/diff/3-4/

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1c9d8ee77aca
part1 : request audio focus again when resume media from page. r=baku
https://hg.mozilla.org/integration/autoland/rev/152a4024af04
part2 : add tests. r=sebastian

Comment 18

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/1c9d8ee77aca
https://hg.mozilla.org/mozilla-central/rev/152a4024af04

Comment 19

[Bogdan Surd, QA [:bsurd, NI]]

Device: 
 - Nexus 5 (Android 6.0.1);

Build:
 - Nightly 55.0a1 (2017-05-17);

Hello,

 I've verified this and audio/video playback pauses when the user switches to the YouTube app. But if the user goes and plays a video from the Photos app the playback is not paused.
 Based on this I am reopening the issue since the fix only works with the YouTube app and ignores other audio/video apps.

Precondition:
 - make sure you have a video saved on your device.

STR:
 1. Go to https://goo.gl/odcAeP and play the video;
 2. Open the Photos app, find and play your video;

After step 2 the video/audio playback in Fennec should stop.

Comment 20

[Alastor Wu [:alwu]]

It's Photos app's bug, it doesn't request system level audio focus properly. You can try to use other music apps, and they also won't be paused when starting video via Photo apps. 

The system audio focus is requested by app, so if the app doesn't implement that, it won't affect any other apps, and vice versa.

---

Please see the following video, you can notice that when video starts playing via Photo app, it doesn't pause the background music from Music app.

https://goo.gl/cLlBl1

Comment 21

[Bogdan Surd, QA [:bsurd, NI]]

Hello,

 I have tried it with different audio apps and it would seem that the photos app does not properly request audio focus. Thank you Alastor for clearing that up.