[webkit-dev] Limiting slow unload handlers (Re: Back/forward cache for pages with unload handlers)

[Maciej Stachowiak]

On Sep 16, 2009, at 1:58 PM, John Abd-El-Malek wrote:

>
>
> On Wed, Sep 16, 2009 at 1:53 PM, Maciej Stachowiak <mjs at apple.com>  
> wrote:
>
> On Sep 16, 2009, at 11:23 AM, Geoffrey Garen wrote:
>
> Given some of our (Chromium-team) recent investigation into the  
> contents of unload handlers, I'm not sure how much this move will  
> help users, even if you don't revert it.  Lots of unload handlers  
> busy-wait while doing async XHR in order to try to guarantee  
> delivery of tracking pings.  Authors will probably elect to copy  
> this code to the pagehide handler rather than moving it, and the  
> experience for users is still going to be extremely sluggish  
> navigation out of these pages.
>
> While it's great that you've made improvements for unload handlers  
> (or maybe not-so-great, since you made them only in v8),
>
> Note I had sent an email to a few WebKit folks to see if they're  
> interested in doing this in JSC as well, but since I didn't hear  
> back, I kept it in V8 only.

I'd recommend doing things like that on webkit-dev instead of private  
mail, for effective communication. I don't think either Geoff or Brady  
was Cc'd on your private email for instance. The mailing list will do  
a better job of finding people who can pay attention to the issue.

>
>
> I think a good way to deal with poorly written unload handlers is to  
> temporarily set the slow script timeout to a much lower value during  
> execution of unload. This would not require any JS-engine-specific  
> changes to work.
>
> I'm personally not a fan of the slow script dialog, as the average  
> user doesn't understand this enough to make a decision (i.e. they  
> don't know if they click this whether they'll lose data, they don't  
> even understand what a script is, etc).

Ah, you're right. We should make sure not to trigger the dialog.

>
> Either way though, I don't think it'll work in this case.  I've seen  
> pages have 8 beforeunload/unload handlers each sleeping for 200ms,  
> just so that they don't have 1 handler that'll trip the slow script  
> detection.  If we decrease the timeout for unload handlers, they  
> would just increase the number of registered handlers proportionally.

I think that setting an upper bound on the amount of time that can be  
spent in all unload handlers is a better solution than hacking the  
behavior of the Date API. Because (a) It's less likely to have  
unexpected side effects; and (b) there's no way for content authors to  
work around it, so we are less likely to end up in an "arms race"  
situation. There were worries expressed that swapping or context  
switching might trigger false positives, but I expect this is unlikely  
in practice, based on our experience with the slow script dialog.

 From reading the comments in the bug, it does not look like we have  
consensus on the right approach, even among Chrome developers.

Regards,
Maciej

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20090916/e0290d48/attachment.html>