Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(827)

Issue 21122: Fix for http://code.google.com/p/chromium/issues/detail?id=7429... (Closed)

Created:
11 years, 10 months ago by Mike Belshe
Modified:
9 years, 5 months ago
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

Fix for http://code.google.com/p/chromium/issues/detail?id=7429 The problem is that webkit's timers are all based off the WTF/CurrentTime implementation, but the bridge was doing math between a webkit that implementation of currentTime and Chromium's base/Time implementation. Subtracting two different times leads to skew, which turned out to always be negative, so the CPU was spinning for most all timers. This bug was introduced during the merge which brought in timers. From the outside, functionality was correct (timers fired at the right time), but internally we could spin the CPU in while waiting for the timer to fire. There is some code to remove from SystemTimeChromium, which I will do, but that is in the webkit tree, so I'll do that separately. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=9307

Patch Set 1 #

Total comments: 1

Patch Set 2 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -2 lines) Patch
M webkit/glue/chromium_bridge_impl.cc View 1 3 chunks +4 lines, -2 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
Mike Belshe
11 years, 10 months ago (2009-02-06 03:59:35 UTC) #1
darin (slow to review)
11 years, 10 months ago (2009-02-06 06:01:16 UTC) #2
very interesting.  LGTM

http://codereview.chromium.org/21122/diff/1/2
File webkit/glue/chromium_bridge_impl.cc (right):

http://codereview.chromium.org/21122/diff/1/2#newcode27
Line 27: #include "wtf/CurrentTime.h"
nit: the convention for includes from wtf land is to use <>'s

Powered by Google App Engine
This is Rietveld 408576698