Closed Bug 956704 Opened 7 years ago Closed 7 years ago

"Acid Defender" WebAudio game audio doesn't continue playing after canceling the multiple tabs warning


(Core :: DOM: Core & HTML, defect)

28 Branch
Windows 7
Not set



Tracking Status
firefox26 --- unaffected
firefox27 --- unaffected
firefox28 + verified
firefox29 + verified
firefox30 --- verified
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed


(Reporter: pauly, Assigned: mrbkap)




(Keywords: regression)


(3 files, 1 obsolete file)

1. Open and enter "Jam Mode"
2. Open a new empty tab
3. Switch back to the game tab
4. Press the X button to close Firefox
5. Cancel the multiple tabs warning

Actual results:
The game is stopped

Expected results:
The game should continue playing

Reproducible on 28.0a2 (2014-01-06), 29.0a1 (2014-01-05).
Works fine on FF 27b2.
Last good nightly: 2013-12-17
First bad nightly: 2013-12-18

Keywords: regression
(In reply to Paul Silaghi, QA [:pauly] from comment #1)
> Last good nightly: 2013-12-17
> First bad nightly: 2013-12-18
> Pushlog:
> pushloghtml?fromchange=b1e5ade62913&tochange=862cb6a1cc88

Thanks Paul, could you try regressing this further with the tinderbox builds for this range?
Version: Trunk → 28 Branch
Last good revision: cbe04dbc2bd6
First bad revision: 08dc60299942
Blocks: 933483
Taking for investigation.
Assignee: nobody → paul
Component: Web Audio → DOM
This works just fine on last tip. Feel free to reopen if you can repro.
Closed: 7 years ago
Resolution: --- → WORKSFORME
Still repro for me on 29.0a1 (2014-01-14), Win 7 x64.
Resolution: WORKSFORME → ---
Attached file testcase-blur.html
So, I've found the issue, and the attached testcase show the difference in behaviour before and after mrbkap patch. The testcase adds listeners on the "blur" and "focus" events.

1. Have a Firefox release (that does not have mrbkap's patch), and a Nightly build (that has it) at hand
2. Open Firefox release, ensuring that the option that says that Firefox should warn you if you have multiple tab on window closing (in setting, tab)
3. Open a new window
4. Open a new tab
5. Load the testcase in this new tab
6. Click on the arrow in order to trigger the confirmation window, notice that this sends a "blur" event to the document.
7. Cancel the window closing, notice that a "focus" event is sent to the document

Now, repeat step 2 - 7 in the Nightly build, that has the patch.
Notice that when the confirmation window appears, the "blur" event is not send to the document.
Notice that when the cancel button is clicked, both events are sent to the document.

This is the source of the change in behaviour in the demo, as the demo code has an elaborate scheduling logic based on "blur" and "focus".
Blake, is that something your patch could cause?
Flags: needinfo?(mrbkap)
Hi Blake, Per Comment 8, this appears to be something your patch caused.  So I'm reassigning this bug to you.  If you think we're wrong, please let us know.  Thanks.
Assignee: paul → mrbkap
I looked into this today. I have a patch that should fix this, but partially regresses bug 933483. I'll have it finished tomorrow.
Flags: needinfo?(mrbkap)
Blocks: 965690
Attached patch Only suspend animation frames (obsolete) — Splinter Review
This should be the safest patch for this bug. Unfortunately, when I try this patch on the testcase in bug 933483, I sometimes fail after dismissing my modal alert. I haven't found STR for it yet, and my debugging has been hampered by lldb's sad interaction with the JITs.  I'll look again next week, but I wanted to get the patch into review in the meantime.
Attachment #8376428 - Flags: review?(bugs)
Comment on attachment 8376428 [details] [diff] [review]
Only suspend animation frames

This doesn't have the same setup for animation pausing as what we have for
event handling suppressing. Things get wrong if some new document is loaded 
while animations are paused etc.

Attachment #8376428 - Flags: review?(bugs) → review-
Comment on attachment 8378520 [details] [diff] [review]
Introduce a mechanism for only suppressing animation frames.

Docshell part isn't super pretty, but should be rare case.
Attachment #8378520 - Flags: review?(bugs) → review+
Attachment #8378522 - Flags: review?(bugs) → review+
Blocks: 976609
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Blocks: 964076
Paul, please verify this is fixed in tomorrow's Nightly.
Flags: needinfo?(paul.silaghi)
Verified fixed FF 30.0a1(2014-02-28) Win 7, Ubuntu 13.04 and Mac OS X 10.8.5.
Flags: needinfo?(paul.silaghi)
Duplicate of this bug: 965690
What's the risk/reward here of uplifting? We're building our final FF28 beta today - please let us know asap if this is a low enough risk fix to take on Beta.
Flags: needinfo?(mrbkap)
Comment on attachment 8378520 [details] [diff] [review]
Introduce a mechanism for only suppressing animation frames.

We should definitely try to get this in on beta.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 933483
User impact if declined: Various bugs dealing with alerts, confirmations and other UI elements, affecting web pages as well.
Testing completed (on m-c, etc.): This has been on m-c for about a week with no known regressions.
Risk to taking this patch (and alternatives if risky): This patch should be less risky than the patch for bug 933483, as it effectively is a less risky version of that patch.
String or IDL/UUID changes made by this patch: None.
Attachment #8378520 - Flags: approval-mozilla-beta?
Attachment #8378520 - Flags: approval-mozilla-aurora?
Flags: needinfo?(mrbkap)
Comment on attachment 8378522 [details] [diff] [review]
Only suppress animation frames when entering a modal state.

Need this patch to actually fix the bug.
Attachment #8378522 - Flags: approval-mozilla-beta?
Attachment #8378522 - Flags: approval-mozilla-aurora?
Attachment #8378522 - Flags: approval-mozilla-beta?
Attachment #8378522 - Flags: approval-mozilla-beta+
Attachment #8378522 - Flags: approval-mozilla-aurora?
Attachment #8378522 - Flags: approval-mozilla-aurora+
Attachment #8378520 - Flags: approval-mozilla-beta?
Attachment #8378520 - Flags: approval-mozilla-beta+
Attachment #8378520 - Flags: approval-mozilla-aurora?
Attachment #8378520 - Flags: approval-mozilla-aurora+
Keywords: verifyme
Reproduced with Nightly from 2014-01-20 by using STR from comment 0.

Verified as fixed with Firefox 28 beta 9 (Build ID: 20140306171728):
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0
Mozilla/5.0 (X11; Linux x86_64; rv:28.0) Gecko/20100101 Firefox/28.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:28.0) Gecko/20100101 Firefox/28.0
Verified as fixed using latest Aurora 29.0a2 (20140314004001) under Win 7 64-bit, Ubuntu 32-bit and Mac OSX 10.8.5.
Depends on: 1191942
Component: DOM → DOM: Core & HTML
Blocks: 965673
You need to log in before you can comment on or make changes to this bug.