Summary: | replace platform time code with WTF::currentTime() | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yong Li <yong.li.webkit> | ||||
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | commit-queue, darin, eric, joenotcharles, laszlo.gombos, manyoso, mjs, staikos | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Attachments: |
|
Description
Yong Li
2009-09-10 13:54:02 PDT
Created attachment 39385 [details]
the patch
the patch
Comment on attachment 39385 [details]
the patch
CurrentTime.h is missing in the patch.
(In reply to comment #2) > (From update of attachment 39385 [details]) > CurrentTime.h is missing in the patch. What do you mean? CurrentTime.h already exists in JavaScriptCore/wtf. I believe that in the current codebase, currentTime() is only ever used to check relative times, even though the descriptions specifies that it should return seconds since the Unix epoch. That could let ports take shortcuts by implementing it to return seconds since some other epoch, or since the system boot, or whatever's easiest for them. I don't know if any ports actually DO take such shortcuts, but this should be double-checked if this patch is landed. I was about to suggest renaming currentTime and changing the description to make it clear the way it was actually used, but I like this approach better. No need to duplicate code. Comment on attachment 39385 [details]
the patch
I'm not an expert here, but this seems reasonable and this patch has been setting in the review queue for a while with little discussion.
Looks reasonable to me too. CCing Darin and Maciej who I believe worked on this code long ago, so that they at least see this go by. Comment on attachment 39385 [details]
the patch
Rejecting patch 39385 from commit-queue.
This patch will require manual commit. Failed to run "['git', 'svn', 'dcommit']" exit_code: 1 cwd: None
Comment on attachment 39385 [details] the patch Sigh. Bug 28316, sorry. Comment on attachment 39385 [details] the patch Clearing flags on attachment: 39385 Committed r48736: <http://trac.webkit.org/changeset/48736> All reviewed patches have been landed. Closing bug. |