249061 - no repaint over settimeout

Status: RESOLVED FIXED

(Firefox :: General, defect, P1)

Description

[Wolfgang Schwarz]

Since Mozilla (just as IE) is practically frozen during execution of a
JavaScript block, it is common to split a large JS task into smaller pieces and
let each piece call the next one via setTimeout("nextPiece()", 0). This usually
has the effect that in between the pieces the browser responds to clicks and
keystrokes and repaints the document if the DOM has changed.

With the attached testcase however, this doesn't work any more in Firefox 0.9.1
(gtk2+xft on Debian unstable). It worked in Ff 0.8.

Instructions: 
1. Open the testcase. 
2. Enter "loop [100] times, painting [1000] numbers per loop, interval [0]" into
the form and hit "Go". 

This calls the function

function loop(n, m, interval) {
  if (!self.debugWin) self.debugWin = open("about:blank","debugWin");
  for (var i=0; i<m; i++) debugWin.document.write("<pre>"+n+"."+m+"</pre>");
  if (n--) setTimeout("loop("+n+","+m+","+interval+")", interval);
}

with n=100, m=1000, interval=0. The function opens a popup window (so unblock
popups!) and writes the numbers 20.0 20.1 ... 20.999 19.0 19.1 ... 19.999 18.0
etc. into the document, with 20 setTimeout breaks in between.

Expected result: At every call of setTimeout, the browser should be responsive
to user feedback.

Actual result: The browser is frozen until all numbers have been written.
 
When I repeat the test without reloading (by just hitting "Go" again) much lower
values suffice to produce the effect, e.g. 100 - 10 - 0. That's probably because
more computation is required to write to an already very long document. If
you're on a faster machine, you might have to enter higher values (into the
first to fields).

Comment 1

[Wolfgang Schwarz]

Attachment: the testcase (identical to URL, but URL will eventually die)

Comment 2

[Boris Zbarsky [:bzbarsky]]

Is this an issue on trunk?  With a current trunk build of SeaMonkey, the browser
remains reasonably responsive while the data is being written out (I can scroll,
etc).

Comment 3

[Wolfgang Schwarz]

Interesting. Yes, I don't see the problem with yesterday's SeaMonkey nightly.
But it's still there in yesterday's Firefox nightly (from the Firefox trunk).

The difference is more obvious the more has been written to the popup window. In
SeaMonkey, the responsiveness doesn't change noticeably when I click "Go"
several times, whereas in Firefox it gets worse and worse.

(There doesn't seem to be a relevant Component in the "Firefox" product
category, so I'm leaving Product = "Browser" and set Version = "Other Branch", ok?) 

Comment 4

[Boris Zbarsky [:bzbarsky]]

No, if this is a problem on firefox _trunk_, it's a firefox bug, and not a
SeaMonkey branch bug (which is what you labeled it as).

Comment 5

[Boris Zbarsky [:bzbarsky]]

I think this is pretty major.  This effectively breaks a lot of DHTML stuff.

Current thinking is that this is a gtk2 vs gtk1 issue.  I am testing gtk1 builds.

Comment 6

[Ali Ebrahim]

(In reply to comment #5)
> I think this is pretty major.  This effectively breaks a lot of DHTML stuff.
> 
> Current thinking is that this is a gtk2 vs gtk1 issue.  I am testing gtk1 builds.

bz, remember that gtk1 is no longer supported by Firefox. If this issue is gtk1
only, then we won't block for this.

Comment 7

[Boris Zbarsky [:bzbarsky]]

This issue is either gtk2-only or Firefox-only.  I am not seeing it in the
builds I actually use and care about (gtk1 SeaMonkey builds).  Which should be
clear from the earlier comments in this bug.

Comment 8

[Boris Zbarsky [:bzbarsky]]

From aebrahim's testing, looks like this is a gtk2 issue.  It's very easy to
reproduce by running the test at
http://www.24fun.com/downloadcenter/benchjs/benchjs.html -- no repaints happen
during test 6 (see bug 249546 for description of problem).

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

GTK2's nsAppShell gives PLEvents a higher priority than redraws and repaints;
PLEvents get G_PRIORITY_HIGH_IDLE, redraws are G_PRIORITY_HIGH_IDLE+20. That
directly leads to this problem. On Windows, painting is higher priority than
PLEvents because WM_PAINT is higher priority than WM_TIMER.

This patch changes PLEvents to G_PRIORITY_DEFAULT_IDLE. It fixes this bug. I'm
not sure what else should be checked or tested, though. Blizzard?

Comment 10

[Christopher Blizzard (:blizzard)]

I'm mildly concerned about taking this.  Sadly, because we're dealing with
multiple event queues on different file descriptors (PLEvent vs. XEvents vs.
timers) it's easy to make one starve the other.  Have you tried some heavy
networking pages?  Is there a hit on page load times?  Perceived times?

Comment 11

[Boris Zbarsky [:bzbarsky]]

Just to make it clear what this bug is about... This bug causes DHTML animations
to pretty completely not work.  The browser pegs the CPU, but never paints
anything, making the user think it's frozen.

This is a very important user perception issue, if nothing else.

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

> (PLEvent vs. XEvents vs. timers)

XPCOM timers are just a thread that periodically posts PLevents to threads'
event loops. No issue there.

I believe GTK is dispatching X events, including generated resize and redraw
messages at higher priority than G_PRIORITY_DEFAULT_IDLE, according to
http://developer.gnome.org/doc/API/glib/glib-the-main-event-loop.html. I think
that's really what we want...

> Have you tried some heavy networking pages?  Is there a hit on page load
> times?  Perceived times?

There probably will be a hit on Tp since right now we're starving paints.
Perceived times will probably improve because there can be more incremental
painting. I don't know how to test this myself.

Comment 13

[Blake Ross]

-> robert 

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

checked in

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Tp on redwood went up by about 9%. Not surprising since we'll be painting
incrementally during page load now.

Comment 16

[Frederic Crozat]

Reopening : this patch only landed on seamonkey trunk but never on aviary branch.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

(In reply to comment #16)
> Reopening : this patch only landed on seamonkey trunk but never on aviary branch.

Why would that matter given that all current development is off the trunk anyway?

Comment 18

[Frederic Crozat]

Because some people (read packagers who maintains firefox 1.0 packages) might
depend on what the bug keyword is saying : 

it says "fixed-avirary1.0" but it isn't.

But since aviary branch was merged back to trunk, this won't matter to anybody
but packagers. 

Closing. Sorry for the noise.