[webkit-reviews] review requested: [Bug 23705] Messages from workers can take over main thread event loop : [Attachment 27458] Updated patch, v3

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Feb 7 22:25:10 PST 2009


Dmitry Titov <dimich at chromium.org> has asked  for review:
Bug 23705: Messages from workers can take over main thread event loop
https://bugs.webkit.org/show_bug.cgi?id=23705

Attachment 27458: Updated patch, v3
https://bugs.webkit.org/attachment.cgi?id=27458&action=review

------- Additional Comments from Dmitry Titov <dimich at chromium.org>
Even better now (thanks for a thorough review!)

- of course the message object is leaking. Fixed.
- moved the '*' to the other side
- got rid of autorelease pool! If the only methods used are alloc-init-release,
none is needed.

I thought a bit about investigating main thread timers... If possible, my
proposal would be to consider it separately (create another bug?) The reasons
are:
- timers (TimerBase objects) are used in WebCore for 'delayed work' very
widely. Allowing user input to slip in between them can break the world in many
subtle ways. For example, if there was a codepath that changes the tree and
calls Timer::startOneShot(0) to do layout, then if the user moves the mouse
across the page we currently have a guarantee that hittest happens only on a
valid layout. startOneShot(0) is used in many places and this change in
assumptions may affect some of them.
- it may change behavior for some sites. For example, a page could schedule 2
timers with the close fire time and accidentally depend on the fact that user
input will not be dispatched in between - such pages can be broken if we allow
user input to go in between close timer firings.
- because of the above, the solution likely will be different for main thread -
perhaps we'll need 'urgent' and 'less urgent' timers.

Let me know what you think! :-)


More information about the webkit-reviews mailing list