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

Status: RESOLVED FIXED

(Core :: Web Audio, defect)

Description

[Karl Tomlinson (:karlt)]

Attachment: offline-source-ended-2.html

See attached testcase.

Expected results:
ended
complete

Actual results:
complete

Comment 1

[Karl Tomlinson (:karlt)]

Attachment: offline-oscillator-ended-2.html

Comment 2

[Paul Yang [:pyang] (away)]

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.

Comment 3

[Karl Tomlinson (:karlt)]

That's a different issue, thanks Paul.  Please file separately.

Comment 4

[Karl Tomlinson (:karlt)]

Attachment: always dispatch main thread updates after non-realtime graph finishes

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.

Comment 5

[Paul Adenot (:padenot)]

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.

Comment 6

[Karl Tomlinson (:karlt)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/edbd846f2f18

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/edbd846f2f18

Comment 8

[Karl Tomlinson (:karlt)]

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.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/8deaeda0c9b2

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/8deaeda0c9b2

Comment 11

[Bogdan Maris, Desktop Test Engineering]

Is there any help needed for manual testing here?

Comment 12

[Karl Tomlinson (: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.

Comment 13

[Ada [:adalucinet]]

(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).