282940 - Switch Mozilla/Firefox to CFRunLoopSource-based plevent handling

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Simon Fraser [no longer active]]

I see no reason why Mozilla/FF should use the old Carbon Event-based plevent
handling. Indeed, since the Carbon Events are dispatched at low priority, this
causes known issues with the accumulation of plevents during busy periods, and
their subsequent flurries at idle time (the Flash plugin suffers from this, and
there are other timer-related issues (see bug 258370).

Camino now has plevent code based on CFRunLoopSources, which should work just
fine with the WaitNextEvent model that Mozilla/FF still use. We should just use
that for all Mac products.

Comment 1

[Mark Mentovai]

Is this bug just about bringing the patch to bug 271050 to the fox, lizard, and
friends?

With MAC_USE_CFRUNLOOPSOURCE selected for event management in
xpcom/threads/plevent.c, the awful experiences in Firefox with Flash movies
bringing the browser to a halt are a memory.  You've got my support here.  This
is a great gift for Mac users.

Comment 2

[Simon Fraser [no longer active]]

Yes, this should be as simple as flipping the #define in plevent.c for Firefox.
Thunderbird should probably have some testing with this flag too, but reports
are that it's an improvement there too (bug 258370).

Comment 3

[Josh Aas]

Attachment: v1.0 + cleanup

Comment 4

[Jo Hermans]

regression (in build mentioned at
<http://weblogs.mozillazine.org/josh/archives/2005/06/firefox_mac_bui.html>) :
bug 298112

Comment 5

[jag (Peter Annema)]

Just for the record, this added about 60 ms to Ts, 40 ms to Txul and 10 ms to Tp
on Monkey (Mac OS X dep) tinderbox. Some of that increase later got masked by
Neil's backout of his test patch for bug 297155, but take a look at the graphs
and numbers on the tinderbox page around the time of the check-in for this bug.

Not saying we should undo this patch, just wanted to call it out, see if maybe
there are some ideas floating around on how to regain some of this loss, perhaps
things we can improve now that we've got this patch in?

Comment 6

[Josh Aas]

Marking the bug fixed, but we should continue the discussion per the comment above.

Comment 7

[Simon Fraser [no longer active]]

Pageload increased in Camino when the equivalent patch was landed. What happens
is that we end up doing more redraws during page load on some pages (how much
depends on how fast your network is), because PLEvents get handled more
promptly. I believe our behavior now is "more correct", and we should just live
with the Tp increase.

The Ts and Txul increases might warrant some investigation.

Comment 8

[Jo Hermans]

another regression: bug 298502

Comment 9

[Uri Bernstein]

Bug 298677 and bug 299384 (a crash), both related to scrollbars, are also likely
regressions of this.

Comment 10

[Mark Mentovai]

*** Bug 299579 has been marked as a duplicate of this bug. ***