371469 - setTimeout can force a bypass of the slow script dialog

Status: RESOLVED WONTFIX

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

Description

[Alex Vincent [:WeirdAl]]

Attachment: testcase

Similar to bug 261633 and bug 331658, but I think this is a separate bug.  The browser does not actually hang.  Instead, the CPU spins like crazy, until the user closes the tab.

If I could suggest a solution, it'd be that whenever a function sets a timeout, note the time already elapsed and tack it on to when the timeout executes.  That way, the need for a slow script dialog becomes more apparent to the JS environment.

This bug affects Firefox 2.0.0.2 and Firefox trunk.

Comment 1

[Alex Vincent [:WeirdAl]]

I accidentally added an evil feature to the testcase by having the current window lose focus every time the setTimeout fires.  So if you have a secondary application window open (say, Chatzilla), and you try to close the afflicted window by a keystroke, you're much more likely to have the secondary window receive the keystroke and react to it...

Once you're down to the afflicted window, then of course the problem goes away.  But you still have to close every other open window first.

As an aside, the tabbrowser binding graciously replaces a window's only tab when the tab is closed with a new tab.  This means the options to get down to only one window (the afflicted one) is reduced to mouse actions to close the window or the keystroke which closes the whole window, not the tab.

Comment 2

[Jesse Ruderman]

This is pretty much exactly what we want sites with long-running scripts to do: break up their execution using setTimeout so users can still interact with the page, close the tab, etc.  We don't want to punish such scripts by showing a "slow script" dialog, but we do want to make sure the browser remains responsive, perhaps by delaying the timeout until other events have been handled.  (It took about 10 "hits" for my Cmd+W to close the window to be recognized.)

> I accidentally added an evil feature to the testcase by having the current
> window lose focus every time the setTimeout fires.

window.blur() is disabled by default in Firefox, but can be enabled through a UI pref.

Comment 3

[Alex Vincent [:WeirdAl]]

(In reply to comment #2)
> This is pretty much exactly what we want sites with long-running scripts to do:
> break up their execution using setTimeout so users can still interact with the
> page, close the tab, etc.

I still think my script is abusing the privilege, though.  :)

Comment 4

[Jesse Ruderman]

Is the problem that your script uses a timeout of 0 rather than 10?

Comment 5

[Alex Vincent [:WeirdAl]]

I don't think so; remember, there's a one-second delay between window blurs (caused by the execution of the time check loop).  A ten millisecond delay barely affects this.

Comment 6

[Jesse Ruderman]

The window.blur() issue is separate from the unresponsiveness issue.  window.blur is disabled for a good reason.

Comment 7

[Manish Singh]

This bit me in the real world. http://abcnews.go.com/ has some stupid animation code (http://a.abcnews.com/assets/js/animation.js) which they use for a scrolling news ticker. It chewed about 50% of my cpu just sitting in a background tab, which is made the browser sluggish, and the laptop battery life none too happy.

It's doing a setTimeout of 10, which is what the sample script already attached effectively does too, since the nsGlobalWindow code clamps it to that minimum.

Perhaps the timeout code should keep track of where and how often timeouts are scheduled, amd throttle down content that tries to schedule things too often.

Comment 8

[Jesse Ruderman]

WONTFIX?  I don't think we ever intend to prevent web sites from using 50% CPU.  The purpose of the "slow script" dialog is only to prevent scripts from making Firefox unresponsive.

See also bug 70821.

Comment 9

[Brendan Eich [:brendan]]

As stated, this bug should be WONTFIXed. Comment 7 asks for throttling of content in background tabs, which is an interesting idea that may deserve a spin-off bug. Comments on that idea?

/be

Comment 10

[Boris Zbarsky [:bzbarsky]]

Could be interesting, sure.

Comment 11

[Manish Singh]

I've filed bug #379535 for throttling content.

Comment 12

[Johnny Stenback (:jst)]

WONTFIX.