Closed Bug 1513733 Opened 5 years ago Closed 5 years ago

start blocked AudioContext when it's source media element starts

Categories

(Core :: Web Audio, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox-esr60 --- unaffected
firefox64 --- unaffected
firefox65 --- unaffected
firefox66 + fixed

People

(Reporter: alwu, Assigned: alwu)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(5 files)

STR.
1. set "media.autoplay.block-webaudio=true"
2. go to https://alastor0325.github.io/htmltests/non_mse_tests/audioAsSourceToWebAudio.html
3. click play button

Expect.
4. audio is playing and web audio should have sound

Actual
4. audio is playing and web audio doesn't have sound

---

In this case, when user starts MediaElement which is used as a source for AudioContext, I think we should also resume AudioContext.
Hi, Paul,
May I have your opinion about resuming AudioContext when its source starts?
Thank you!
Flags: needinfo?(padenot)
Flags: needinfo?(padenot)
In order to call this method on other situations, rename it to 'StartBlockedAudioContextIfAllowed()'.
If media element is used as a source for AudioContext, we would try to start AudioContext which was not allowed
to start when media element starts playing.
Summary: start blocked AudioContext when it's source starts → start blocked AudioContext when it's source media element starts
Blocks: 1517046
No longer blocks: 1517046
See Also: → 1517324
Pushed by alwu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6d105dcaa3df
part1 : rename 'NotifyScheduledSourceNodeStarted()' r=karlt
https://hg.mozilla.org/integration/autoland/rev/7ab6eb45e6b8
part2 : try to start AudioContext when media element which is as a source for web audio starts r=cpearce,karlt
https://hg.mozilla.org/integration/autoland/rev/5ce7c992bd81
part3 : add test. r=karlt
Backout by rgurzau@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6a89226d8c15
Backed out 3 changesets for frequent failures at browser_autoplay_policy_web_audio_mediaElementAudioSourceNode.js on a CLOSED TREE
Backed out 3 changesets (bug 1513733) for frequent failures at browser_autoplay_policy_web_audio_mediaElementAudioSourceNode.js on a CLOSED TREE

Backout link: https://hg.mozilla.org/integration/autoland/rev/6a89226d8c15f0b8b9dbcb23f76f371fdefc289b

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&selectedJob=219708943&revision=5ce7c992bd81c2426c5be3c3be57e7dfa72fdd9c

Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=219708943&repo=autoland&lineNumber=5980

Log snippet: 

02:48:23     INFO - TEST-START | toolkit/content/tests/browser/browser_autoplay_policy_web_audio_mediaElementAudioSourceNode.js
02:49:09     INFO - TEST-INFO | started process screenshot
02:49:09     INFO - TEST-INFO | screenshot: exit 0
02:49:09     INFO - Buffered messages logged at 02:48:23
02:49:09     INFO - Entering test bound start_tests
02:49:09     INFO - - setup test preference -
02:49:09     INFO - - start 'testResumeAudioContextWhenMediaElementSourceStarted' -
02:49:09     INFO - Buffered messages logged at 02:48:24
02:49:09     INFO - Console message: [JavaScript Error: "The character encoding of the HTML document was not declared. The document will render with garbled text in some browser configurations if the document contains characters from outside the US-ASCII range. The character encoding of the page must be declared in the document or in the transfer protocol." {file: "https://example.com/browser/toolkit/content/tests/browser/file_video.html" line: 0}]
02:49:09     INFO - - create audio context -
02:49:09     INFO - - check AudioContext status -
02:49:09     INFO - TEST-PASS | toolkit/content/tests/browser/browser_autoplay_policy_web_audio_mediaElementAudioSourceNode.js | AudioContext is not started yet. - true == true - 
02:49:09     INFO - - create and start MediaElementAudioSourceNode -
02:49:09     INFO - TEST-PASS | toolkit/content/tests/browser/browser_autoplay_policy_web_audio_mediaElementAudioSourceNode.js | video started playing - true == true - 
02:49:09     INFO - - AudioContext should be resumed after MediaElementAudioSourceNode started -
02:49:09     INFO - Buffered messages finished
02:49:09     INFO - TEST-UNEXPECTED-FAIL | toolkit/content/tests/browser/browser_autoplay_policy_web_audio_mediaElementAudioSourceNode.js | Test timed out - 
02:49:09     INFO - GECKO(5632) | MEMORY STAT | vsize 1707MB | vsizeMaxContiguous 130580191MB | residentFast 223MB | heapAllocated 71MB
02:49:09     INFO - TEST-OK | toolkit/content/tests/browser/browser_autoplay_policy_web_audio_mediaElementAudioSourceNode.js | took 45034ms
02:49:09     INFO - Not taking screenshot here: see the one that was previously logged
02:49:09     INFO - TEST-UNEXPECTED-FAIL | toolkit/content/tests/browser/browser_autoplay_policy_web_audio_mediaElementAudioSourceNode.js | Found a tab after previous test timed out: https://example.com/browser/toolkit/content/tests/browser/file_video.html - 
02:49:09     INFO - GECKO(5632) | JavaScript error: , line 0: InvalidStateError: An attempt was made to use an object that is not, or is no longer, usable
02:49:09     INFO - checking window state
Flags: needinfo?(alwu)
When we block AudioContext in the time of construction, we would have a internal suspend call which is
used to stop AudioContext from starting. The suspend call would eventually dispatch state change task,
but we don't need that in this situation, because AudioContext would keep staying at 'suspended' state.

In this situation, if we dispatch the state change task for 'suspend' in the beginning, the AudioContext's
state would be changed incorrectly when we resume the AudioContext soon after we block it. The reason is
that the state change task for 'suspend' which is called in the time of contruction would be executed after
the state change task for 'resume' because MSG finishs 'resume' operation faster than finishs 'suspend' operation
even if the suspend operation has come before the resume operation.
Attached file Test fail log
From the log, we can see the reason of intermittent test fail is because the state change task for 'suspend' which is called at the time of AudioContext contruction came late after the state change task for 'resume'.

It causes that even we have resumed AudioContext successfully, its state is still keeping at 'suspended' state.
Flags: needinfo?(alwu)
I found that even the new patch can fix the test's timeout problem, but there is still a leaking issue [1] which needs to be fixed...

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=e918caccf8a85b92ffad67334ff6f3506d30e9d2&selectedJob=219863559
Hi, Karlt,
Do you have any idea about the leaking in comment11? 
I suspect that is related with the lambda capturing, but I have no further evidence which can prove that assumption...
Thank you!
Flags: needinfo?(karlt)
I don't see any new strong references added in the patch here and so I guess it is something new in the test that is triggering the leak.

I would reduce what the test is doing as an experiment to find out which part exactly is triggering the leak.  Hopefully that might provide some clues.
Flags: needinfo?(karlt)
See Also: → 1518213

I filed leaking issue in bug1518213, and I'm going to change the test to simple plain-mochitest.

Attachment #9034268 - Attachment description: Bug 1513733 - part4 : do not dispatch state change task for interal suspend call. → Bug 1513733 - part4 : do not call suspendInternal() when constructing AudioContext
Pushed by alwu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/05005b52bb56
part1 : rename 'NotifyScheduledSourceNodeStarted()' r=karlt
https://hg.mozilla.org/integration/autoland/rev/c34e287f2f7c
part2 : try to start AudioContext when media element which is as a source for web audio starts r=cpearce,karlt
https://hg.mozilla.org/integration/autoland/rev/22ce92cbdf64
part3 : add test. r=baku,karlt
https://hg.mozilla.org/integration/autoland/rev/8699d4c48838
part4 : do not call suspendInternal() when constructing AudioContext r=karlt
See Also: → 1577184
You need to log in before you can comment on or make changes to this bug.