AudioBufferSourceNode and OscillatorNode don't dispatch ended event when OfflineAudioContext completes soon after source end

RESOLVED FIXED in Firefox 26

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: karlt, Assigned: karlt)

Tracking

({regression})

Trunk
mozilla27
x86_64
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox26 verified, firefox27 verified, b2g-v1.2 fixed)

Details

Attachments

(3 attachments)

See attached testcase.

Expected results:
ended
complete

Actual results:
complete
Attachment #801360 - Attachment is patch: false
Attachment #801360 - Attachment mime type: text/plain → text/html
Depends on: 918212
Summary: AudioBufferSourceNode doesn't dispatch ended event when OfflineAudioContext completes soon after source end → AudioBufferSourceNode and OscillatorNode don't dispatch ended event when OfflineAudioContext completes soon after source end
Hi Karl, I have STR for oscillator node doesn't stop when moving to background but for b2g.
Is that related to this bug?

Use device: buri/unagi

STR:
1. Start oscillator node
2. Press home key (oscillator page moved to background)

EXPECT:
- oscillator stops

ACTUAL:
- It keeps making sound.
Flags: needinfo?(karlt)
That's a different issue, thanks Paul.  Please file separately.
Flags: needinfo?(karlt)
Assignee: nobody → karlt
Blocks: 882543, 923301
Keywords: regression
This is not enough to fix offline-oscillator-ended-2.html because OscillatorNodeEngine doesn't set aFinished until the block after it finishes, but I'll file another bug for that.

This is what I want for bug 923301.
Attachment #820788 - Flags: review?(paul)
Comment on attachment 820788 [details] [diff] [review]
always dispatch main thread updates after non-realtime graph finishes

Review of attachment 820788 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/test/crashtests/offline-buffer-source-ended-1.html
@@ +9,5 @@
> +var source = context.createBufferSource();
> +source.buffer = context.createBuffer(1, 12000, context.sampleRate);
> +source.onended = function(e) {
> +  document.documentElement.removeAttribute("class");
> + dump("ended\n");

Remove this.
Attachment #820788 - Flags: review?(paul) → review+
Blocks: 930257
https://hg.mozilla.org/mozilla-central/rev/edbd846f2f18
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Comment on attachment 820788 [details] [diff] [review]
always dispatch main thread updates after non-realtime graph finishes

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Web Audio
User impact if declined: Bug 923301 and so bug 898291 depend on this fix.
Testing completed (on m-c, etc.): on m-c Aurora, in testsuite.
Risk to taking this patch (and alternatives if risky):
very low. small change to remove over-optimizing.
String or IDL/UUID changes made by this patch: none.
Attachment #820788 - Flags: approval-mozilla-beta?
Attachment #820788 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Keywords: verifyme
Is there any help needed for manual testing here?
Flags: needinfo?(karlt)
The testcase that is now fixed here is running in the automated testsuite, so no manual testing is required thanks.  Bug 930257 covers what is not fixed here.
FWIW running the testcases manually requires browser.dom.window.dump.enabled set to true for terminal output.
Flags: needinfo?(karlt)
(In reply to Karl Tomlinson (:karlt) from comment #12)
> The testcase that is now fixed here is running in the automated testsuite,
> so no manual testing is required thanks.  Bug 930257 covers what is not
> fixed here.
> FWIW running the testcases manually requires browser.dom.window.dump.enabled
> set to true for terminal output.

Verified as fixed on Firefox 26 beta 7 (Build ID: 20131122094025) and latest Aurora (build ID: 20131124105503): after browser.dom.window.dump.enabled is set to true, "complete" and "ended" are shown in the terminal for the first attachment, although not for the second attachment as mentioned in comment 4 (bug 930257).
You need to log in before you can comment on or make changes to this bug.