Bug 24096 - Use PLATFORM(CF) instead of PLATFORM(MAC) in CurrentTime.cpp
Summary: Use PLATFORM(CF) instead of PLATFORM(MAC) in CurrentTime.cpp
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-02-23 08:29 PST by Jeremy Moskovich
Modified: 2010-03-11 04:30 PST (History)
2 users (show)

See Also:


Attachments
PLATFORM(CF) (1.18 KB, patch)
2009-02-23 08:31 PST, Jeremy Moskovich
aroben: review-
Details | Formatted Diff | Diff
Address aroben's comments from previous patch (3.70 KB, patch)
2009-02-23 08:57 PST, Jeremy Moskovich
aroben: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Moskovich 2009-02-23 08:29:20 PST
This change allows Chrome on OS X to use the CF functions for currentTime() rather than the POSIX variant.
Comment 1 Jeremy Moskovich 2009-02-23 08:31:31 PST
Created attachment 27882 [details]
PLATFORM(CF)

Is PLATFORM(CF) OK to use here?  A concern has been raised about Apple's CF variant on Windows, if this is a problem, would PLATFORM(DARWIN) && PLATFORM(CF) be a better choice?
Comment 2 Adam Roben (:aroben) 2009-02-23 08:47:42 PST
(In reply to comment #1)
> Is PLATFORM(CF) OK to use here?  A concern has been raised about Apple's CF
> variant on Windows, if this is a problem, would PLATFORM(DARWIN) &&
> PLATFORM(CF) be a better choice?

We still want Windows to use the WIN_OS code, as it is higher-resolution than CFAbsoluteTimeGetCurrent on Windows. One option would be to do what you proposed: PLATFORM(DARWIN) && PLATFORM(CF). Another option would be to make the WIN_OS code the first branch of the #if/#elif chain. I think I prefer the latter.
Comment 3 Jeremy Moskovich 2009-02-23 08:57:15 PST
Created attachment 27883 [details]
Address aroben's comments from previous patch

Thanks, I've put the Windows case first and added a comment so the reasoning behind the change isn't lost.
Comment 4 Adam Roben (:aroben) 2009-02-23 09:10:37 PST
Comment on attachment 27883 [details]
Address aroben's comments from previous patch

> +#elif PLATFORM(GTK)
> +
> +// Note: GTK on Windows will pick up the PLATFORM(WIN) implementation above which provides
> +// better accuracy compared with Windows implementation of g_get_current_time:
> +// (http://www.google.com/codesearch/p?hl=en#HHnNRjks1t0/glib-2.5.2/glib/gmain.c&q=g_get_current_time).
> +// Non-Windows GTK builds could use gettimeofday() directly but for the sake of consistency lets use GTK function.

Looks like this patch helps out GTK/win, too!

r=me
Comment 5 Dimitri Glazkov (Google) 2009-02-23 09:55:52 PST
Landed as http://trac.webkit.org/changeset/41149.