The invalid assert has now been fixed. I still don't think we should do this for the reasons that are in this thread.
Another issue is that developers can replace the Date constructor anyway they like. If the developer does that, then the replacement for Date will not have the new method and attempting to call it will throw an exception that we need to handle somehow. If we handle that gracefully, the end result will be that there is no change to the Date constructor. So a very simple workaround for a developer who wants a sleep in the unload handler is to replace the Date constructor with some other function that uses the real Date constructor to get the time. Cheers, -- Mads On Sat, Aug 29, 2009 at 11:34 PM, Mike Belshe<mbel...@google.com> wrote: > I agree with Dean on this one. Maybe it's a fool's errand to even try > anything. > My first choice is to nuke the unload handlers. However, there are > legitimate sites saying "you're about to lose your data", so that doesn't > work. > My second choice would be to have users give approval for unload handlers, > or auto detect which are junk, but I can't think of any reasonable > algorithms to do this? Could we have a cloud-based vote system? Maybe that > is all junk too. > Lastly, my thought is to change the UI. What if we allow the page > transition, immediately, and on the new window you get one of those yellow > infobars saying "the last page did not close" with buttons to "<view it>" or > "<close it>". Optionally, if the page was good enough to use one of our > alert dialogs, we could even display the page's message in there. Hopefully > this would get our page transitions faster. > I'd be willing to bet there are more onunload handlers I don't want to run > than ones I do want to run. To Dean's point, even trying to do what I'm > suggesting may just force these folks into even worse tricks to try to track > the user. > chrome needs adblock, i guess. > Mike > > On Fri, Aug 28, 2009 at 9:41 AM, Dean McNamee <de...@chromium.org> wrote: >> >> John, >> >> If we pushed out the getTime change, it will be easily to realize what >> we've done and quickly adapt to doing something different. Basically >> I think it's an ugly hack that will have a small and likely temporary >> effect on the real problem. >> >> On Fri, Aug 28, 2009 at 9:00 AM, John Abd-El-Malek <j...@google.com> wrote: >> > Thanks Mads for the feedback, comments below. >> > >> > On Fri, Aug 28, 2009 at 2:50 AM, Mads Sig Ager <a...@chromium.org> >> > wrote: >> >> >> >> John, >> >> >> >> I'll look into the assertion failure. I'm not sure why you are >> >> hitting that one - that assert is exactly setup that way to allow eval >> >> in extensions but not in our own native function code. >> >> >> >> That said, I think this is a pretty nasty hack and I think we should >> >> avoid putting it in if we can. There are so many ways users can make >> >> >> >> stuff in onunload handlers take a long time. >> > >> > I agree it's a hack and I would rather not put it in either. However we >> > have a problem in that we see web pages sleeping for 2 seconds. The end >> > effect is that it looks like Chrome is slow. The user can't tell that >> > it's >> > the page or the the browser, but they have given us the signal that they >> > want to leave the page. Making them wait a few seconds gives a poor >> > experience, i.e. see all the comments at http://crbug.com/7823. >> > >> >> >> >> Lying about the time to >> >> avoid one case seems like a bad idea. I'm not sure I agree that it is >> >> the browser's problem that a user decides to perform long-running >> >> operations in onunload. >> > >> > Just to make things clear, which user are you talking about, the person >> > using the browser or the person who wrote the slow page? The person >> > surfing >> > has no idea about how the page is written, and just made a decision to >> > leave >> > the page. If it was up to them, they wouldn't care to stay on a page >> > for a >> > few seconds to let it do whatever tracking it wants. >> > >> > For the person writing the page, there are better ways of doing this. >> > i.e. >> > they can have a bunch of handlers that wait progressively more and more >> > time, so that people with fast connections still don't have to wait this >> > long. Or as browser developers, we can provide them with better methods >> > of >> > doing this, so they don't slow down navigations at all. We're going to >> > do >> > that with <a ping>, which is in HTML5. Once that's implemented, we will >> > do >> > developer outreach to get them to use it instead of the sleep hacks. >> >> >> >> If we believe that the browser should >> >> disallow that, maybe we should explicitly set a timout after which >> >> javascript execution in onunload will be forcefully terminated? That >> >> will probably have other issues though. >> > >> > We discussed a bunch of these options (I should have sent a link to the >> > bug >> > in the message earlier, it goes through some of the problems of doing >> > something like this). The problem is that this opens up a can of worms >> > in >> > terms of site compatibility. Some sites (i.e. docs) might do a sync XHR >> > to >> > save the draft of the document, while others like Gmail might pop up a >> > dialog box if there's unsaved emails. Any timeout would have false >> > positives on legitimate code depending on the machine hardware, and >> > would >> > break some sites. This approach works, IMO, because I can't find any >> > legitimate site that needs to call getTime 1000 (or even 10000 if we >> > choose >> > that instead) times in an unload handler. It's a surgical way to get >> > rid of >> > sleeps. >> > I look at this as simulating having a slow cpu and latency. i.e. when >> > the >> > script calls getTime, the cpu runs a different process and returns after >> > 1ms. After the sleeps loop is done, the request is still pending >> > because of >> > a slow network connection. The site already has to deal with machines >> > like >> > this, so we're not breaking them. >> > >> > I'm open to any other solutions that people can think of which have less >> > side-effects. >> > Thanks, >> > John >> >> >> >> Cheers, -- Mads >> >> >> >> On Fri, Aug 28, 2009 at 5:06 AM, John Abd-El-Malek<j...@google.com> >> >> wrote: >> >> > Hi, >> >> > In investigating a bug where tabs take a long time to close, I >> >> > tracked >> >> > the >> >> > issue to sites simulating sleep in their unload handlers. They do >> >> > this >> >> > by >> >> > looping until Date.getTime changes by enough ms. I'm now prototyping >> >> > disabling these sleeps by skewing the time after a large number of >> >> > getTime >> >> > calls in an unload handler. Here's the changelist where I >> >> > implemented >> >> > it >> >> > using a V8 extension: http://codereview.chromium.org/178010/show. >> >> > This >> >> > is >> >> > the short term fix, while we implement better methods for having >> >> > requests >> >> > outlive a page, so that the ad networks can do whatever they need to >> >> > do >> >> > without blocking the user. >> >> > The problem I'm running into is that in overriding the Date >> >> > constructor >> >> > I >> >> > needed to use eval. This was the only way I could find where I can >> >> > override >> >> > the constructor with no arguments and have that go through my logic >> >> > of >> >> > skewing the time, while maintaining existing behavior for the other >> >> > constructors. The problem is I run into this assert in >> >> > parser.cc:"ASSERT(extension_ != NULL || !Bootstrapper::IsActive());", >> >> > which >> >> > is triggered by my use of eval for an internal script. >> >> > I wanted to see what you guys think of this. Is there another way >> >> > that >> >> > you'd prefer that this be implemented, i.e. have something in the V8 >> >> > API >> >> > which allows a user to filter getTime calls? Or if this approach is >> >> > ok, >> >> > can >> >> > we take out that assert, or have a way for an extension to not >> >> > trigger >> >> > it? >> >> > >> >> > Thanks, >> >> > John >> >> > >> >> > > >> >> > >> > >> > >> > > >> > >> >> >> > > --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
