720289 - Leaving a File picker dialog open triggers an unresponsive script dialog

Status: VERIFIED FIXED

(Core :: Widget: Win32, defect)

Description

[Alice0775 White]

Build Identifier:
http://hg.mozilla.org/mozilla-central/rev/244711942710
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:12.0a1) Gecko/20120122 Firefox/12.0a1 ID:20120122031050
and
http://hg.mozilla.org/releases/mozilla-aurora/rev/24cb0e50df85
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a2) Gecko/20120122 Firefox/11.0a2 ID:20120122042010

Leaving a File picker dialog open triggers an unresponsive script dialog
*This also happen on Windows XP Sp3.
*This does not happen on ubuntu10.40+GNOME2.30.2.

This is similar to Bug 663275, but different regression range.

Reproducible: Always

Steps to Reproduce:
1. Start Firefox with clean profile
2. Open Google Reader(login and subscribe RSS) 
   (To make it easy to reproduce)
3. Open 2-3 additional tabs
4. Open File Picker (Ctrl+O)
5. Wait for a while (40-60 sec)
6. Close the File picker dialog

Actual Results:
  Script busy dialog popped up.
  The script URL and line# appears at random.

>A script on this page may be busy, or it may have 
>stopped responding. You can stop the script now, 
>or you can continue to see if the script will complete.
>
>Script: chrome://global/content/bindings/browser.xml:0


>A script on this page may be busy, or it may have 
>stopped responding. You can stop the script now, 
>or you can continue to see if the script will complete.
>
>Script: chrome://browser/content/browser.js:5665

etc...

Expected Results:
  Script busy dialog should not pop up.


Regression window(m-c)
Works:
http://hg.mozilla.org/mozilla-central/rev/5efcb9c3b375
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111216 Firefox/11.0a1 ID:20111216031140
Fails:
http://hg.mozilla.org/mozilla-central/rev/dcf6e5163e63
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111216 Firefox/11.0a1 ID:20111216035316
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5efcb9c3b375&tochange=dcf6e5163e63


Regression window(m-i)
Works:
http://hg.mozilla.org/integration/mozilla-inbound/rev/956d36a82987
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111215 Firefox/11.0a1 ID:20111215124517
Fails:
http://hg.mozilla.org/integration/mozilla-inbound/rev/5c3c5e4db3ad
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111215 Firefox/11.0a1 ID:20111215130616
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=956d36a82987&tochange=5c3c5e4db3ad

Triggered by:
d737ffa1f92d	Luke Wagner — Bug 699974 - Plug several holes that prevent stopping slow scripts (r=jst)

Comment 1

[Alice0775 White]

More easy STR.

Steps to Reproduce:
1. Start Firefox with clean profile
2. Open http://www.webkit.org/perf/sunspider-0.9.1/sunspider-0.9.1/driver.html
3. Open File Picker (Ctrl+O)
4. Wait for a while (40-60 sec)
5. Close the File picker dialog

Comment 2

[Alex Keybl [:akeybl]]

Tracking for FF11. Assigning to Luke since this is a suspected regression from 699974.

Comment 3

[Luke Wagner [:luke]]

The basic call stack is:
 1. main loop
 2. file opener chrome JS
 3. nested event loop (waiting for mouse click)
 4. sunspider content JS
 5. call to set innerHTML
 6. call to some onPageHide chrome JS in browser.xul

The nsJSContext used for (2) and (6) is the same so the fact that (2) hasn't returned causes the slow script dialog in (6).  The simple fix (for FF11/12) is to just back out the patch; it is pretty simple.  That of course goes back to the original bug.  Thinking a bit more about it, it seems what is really needed is for nsJSContext to keep a stack of timer values so that each re-entrance can be evaluated for slow-ness independently.

Comment 4

[Luke Wagner [:luke]]

Attachment: backout everything but the NS_ENSURE_TRUE(prompt, JS_FALSE)

The backout fixes the slow-script hang.  Thanks for the great STR!

I guess I'll file a follow-up bug for making a real fix and maybe a real DOM developer who really knows what they are doing can fix it.

Comment 5

[Luke Wagner [:luke]]

jst: review ping

Comment 6

[Johnny Stenback (:jst)]

Comment on attachment 596170 [details] [diff] [review]
backout everything but the NS_ENSURE_TRUE(prompt, JS_FALSE)

r=jst, sorry for the delay :(

Comment 7

[Luke Wagner [:luke]]

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

Comment 8

[Luke Wagner [:luke]]

Comment on attachment 596170 [details] [diff] [review]
backout everything but the NS_ENSURE_TRUE(prompt, JS_FALSE)

[Approval Request Comment]
Regression caused by (bug #): 699974
User impact if declined: Spurious slow-script dialogs
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): Simple patch and reverts to a previous state, so risk is low

Comment 9

[Alex Keybl [:akeybl]]

Comment on attachment 596170 [details] [diff] [review]
backout everything but the NS_ENSURE_TRUE(prompt, JS_FALSE)

[Triage Comment]
Approving for Aurora 12 and Beta 11 since this is basically a backout to a known low-risk state. We also hope that this fix will mitigate bug 718542 and bug 729328.

Comment 10

[Luke Wagner [:luke]]

https://hg.mozilla.org/releases/mozilla-beta/rev/927cf98d708a
https://hg.mozilla.org/releases/mozilla-aurora/rev/aca232a7c04b

Comment 11

[Ed Morley [:emorley]]

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

Comment 12

[Simona Badau, Desktop QA]

Verified using Firefox 11 beta 6 on Windows 7 and Windows XP - when using the steps to reproduce from Comment 1 no unresponsive script dialog is triggered.

Mozilla/5.0 (Windows NT 6.1; rv:11.0) Gecko/20100101 Firefox/11.0
Mozilla/5.0 (Windows NT 5.1; rv:11.0) Gecko/20100101 Firefox/11.0

Comment 13

[Ioana (away)]

Verified as fixed using the steps from comment 0 and from comment 1 on:
Mozilla/5.0 (Windows NT 6.1; rv:12.0) Gecko/20100101 Firefox/12.0
Mozilla/5.0 (Windows NT 5.1; rv:12.0) Gecko/20100101 Firefox/12.0
20120320212821