Closed Bug 1513733 Opened 6 years ago Closed 6 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.
Attached file Bug 1513733 - part3 : add test. (deleted) —
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
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 (deleted) —
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)

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
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: