Bug 24096

Summary: Use PLATFORM(CF) instead of PLATFORM(MAC) in CurrentTime.cpp
Product: WebKit Reporter: Jeremy Moskovich <playmobil>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: aroben, eric
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Attachments:
Description Flags
PLATFORM(CF)
aroben: review-
Address aroben's comments from previous patch aroben: review+

Jeremy Moskovich
Reported 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.
Attachments
PLATFORM(CF) (1.18 KB, patch)
2009-02-23 08:31 PST, Jeremy Moskovich
aroben: review-
Address aroben's comments from previous patch (3.70 KB, patch)
2009-02-23 08:57 PST, Jeremy Moskovich
aroben: review+
Jeremy Moskovich
Comment 1 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?
Adam Roben (:aroben)
Comment 2 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.
Jeremy Moskovich
Comment 3 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.
Adam Roben (:aroben)
Comment 4 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
Dimitri Glazkov (Google)
Comment 5 2009-02-23 09:55:52 PST
Note You need to log in before you can comment on or make changes to this bug.