Bug 14269 - REGRESSION: Gmail links stop working after computer sleep
Summary: REGRESSION: Gmail links stop working after computer sleep
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 523.x (Safari 3)
Hardware: Mac OS X 10.4
: P1 Normal
Assignee: Nobody
URL: http://mail.google.com/mail
Keywords: HasReduction, InRadar, Regression
Depends on:
Blocks:
 
Reported: 2007-06-20 23:48 PDT by Ruben Bakker
Modified: 2007-08-25 14:51 PDT (History)
3 users (show)

See Also:


Attachments
WebCore/Timer.cpp patch to work around the problem (818 bytes, patch)
2007-06-21 00:06 PDT, Ruben Bakker
ddkilzer: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ruben Bakker 2007-06-20 23:48:47 PDT
The problem occurs when using Gmail and putting the Mac to sleep. After wakeup the Gmail page is not responsive, e.g., most links do not work anymore. After a delay of about 1 to 3 minutes the links start working again. A page reload does solve the problem, too.

Steps to duplicate the problem:
1. Open http://mail.google.com/mail and login
2. While on the "Inbox" page: put your Mac to sleep
3. Wait a least 5 minutes (If you wait less, you cannot dup the problem).
4. Wakeup your Mac
5. Press the "Refresh" link or "Sent mail" link -> nothing happens
6. Wait about 1 to 2 minutes and the links work again

I put "Regression" in the title, as the problem cannot be duplicated in Safari 2 (Tiger/419.x) .
Comment 1 Ruben Bakker 2007-06-21 00:06:14 PDT
Created attachment 15154 [details]
WebCore/Timer.cpp patch to work around the problem
Comment 2 Ruben Bakker 2007-06-21 00:15:30 PDT
After some debugging I found the WebCore SharedTimer to be the problem. Normally, the SharedTimer is restarted when the time of the first Timer on the timerHeap changes. 

After a sleep however, the first Timer on the timerHeap has an absolute time that is in the past. The SharedTimer doesn't get updated when new Timers (for loading pages) are added as they never can become the first timer. 

My workaround (see attached patch) is to always update the SharedTimer, even when the timer is not the the first timer.

I hope this makes sense :-).




Comment 3 David Kilzer (:ddkilzer) 2007-06-21 07:33:49 PDT
Updating bug since it's a regression.

Comment 4 David Kilzer (:ddkilzer) 2007-06-21 07:40:30 PDT
Comment on attachment 15154 [details]
WebCore/Timer.cpp patch to work around the problem

Thanks for the patch and the detective work, Ruben!

NOTE: I'm not reviewing the correctness of the code changes in the patch, but instead the patch itself.

1. Please create a ChangeLog entry for this patch using WebKitTools/Scripts/prepare-ChangeLog.

2. Please use WebKitTools/Scripts/svn-create-patch to generate a patch of the ChangeLog entry and the code fix.

3. All changes need a layout test unless it's not possible to make one.  (In this case, I'd say it's not possible to emulate sleeping for over 5 minutes in the layout tests, so just note this in the ChangeLog entry as well.)

More details here:  http://webkit.org/coding/contributing.html

Please make the above corrections and attach a new patch.  Please also set the "review?" flag on the patch so that reviewers will notice it.  Thanks again!
Comment 5 David Kilzer (:ddkilzer) 2007-06-21 08:54:32 PDT
Confirmed with a local debug build of WebKit r23713 with Safari 3.0 (522.11) on Mac OS X 10.4.10 (8R218).

Nice job spotting this bug and tracking down the cause and fix!!

I noticed that the cursor in the Bugzilla "Additional Comments" text area doesn't blink during this period after wake, either!  :)

Comment 6 Ruben Bakker 2007-06-21 09:09:49 PDT
Hi David,
Thanks for confirming this bug!
I will follow "the process" and attach a new patch shortly.
Comment 7 David Kilzer (:ddkilzer) 2007-06-21 17:13:29 PDT
<rdar://problem/5286444>
Comment 8 Anders Carlsson 2007-08-01 14:24:18 PDT
This might not be a good idea to do performance-wise as it would involve destroying and then creating a new CFRunLoopTimer each time a timer is added.

Darin, is that acceptable or do you have a better idea?
Comment 9 Darin Adler 2007-08-01 15:22:31 PDT
(In reply to comment #8)
> Darin, is that acceptable or do you have a better idea?

This doesn't fix the real problem. What it does is make scheduling a new timer "tickle" the shared timer and make it fire. There must be a better fix where we make the shared timer fire without scheduling a new timer.
Comment 10 Anders Carlsson 2007-08-01 18:03:40 PDT
We could add a new shared timer function that we'd always call when adding a timer. On the Mac, this would check if the fire date is in the past and then re-set the timer. This would rely on new timers always being added sooner or later though.
Comment 11 Anders Carlsson 2007-08-03 13:41:38 PDT
Committed revision 24851.
Comment 12 Ruben Bakker 2007-08-25 14:51:11 PDT
I wasn't able to duplicate the problem anymore.
Thanks for fixing this bug!