A central clock is important for the reasons James describes, but using the one from WebKit is not a good option since that would make other modules in Chromium, like base/ and net/, have a dependency on WebKit. Also, given that we started out using our own central clock, until WebKit grew its own, it made the most sense to me to add a currentTime function to WebKitClient.
More history: This is the N'th time we have fixed this bug. The last time, ChromiumCurrentTime.cpp was introduced, and we configured the build system to exclude wtf/CurrentTime.cpp. However, when we switched over to using JavaScriptCore.gypi, wtf/CurrentTime.cpp slipped back into the build. When linking Chrome, we were then linking against two implementations of currentTime! However, because they existed in separate .lib files, the linker didn't complain about duplicate symbols. It just picked one of them to use, and we never noticed. James' patch adds a #error if wtf/CurrentTime.cpp is compiled in the Chromium build, so we should never repeat this mistake again. As for which implementation of currentTime is better, I can't say for sure. I just know that we put a lot of time into our implementation, and it has served us quite well. -Darin On Tue, Oct 27, 2009 at 7:13 PM, Dmitry Titov <[email protected]> wrote: > Very cool investigation! > > I see WebKitClient::currentTime() in the upcoming WebKit API so I guess the > idea is that client of WebKit would provide "the central clock". I agree > "central clock" is a good idea. Currently in Webkit, it's provided by WTF. > Embedders (like Safari) use WTF directly, for which WTF stuff is exposed by > JavaScriptCore.exp. > > Do we want to provide central clock from Chromium because we think the > Chromium's implementation of currentTime is better? If so, wouldn't it be > simpler to just replace the WTF one? ChromiumCurrentTime.cpp looks like a > temporary hack... > > Dmitry > > > On Tue, Oct 27, 2009 at 6:48 PM, Peter Kasting <[email protected]>wrote: > >> On Tue, Oct 27, 2009 at 4:40 PM, James Robinson <[email protected]>wrote: >> >>> So what's the problem? WTF::currentTime() is implemented twice, once in >>> JavaScriptCore/wtf/CurrentTime.cpp and once in >>> webkit/api/src/ChromiumCurrentTime.cpp. The latter simply defers to >>> base::Time::Now().ToDoubleT(), the former uses a somewhat different >>> algorithm to try to normalize what the system returns. ThreadTimers.cpp was >>> linking against the JavaScriptCore/wtf/CurrentTime.cpp version and >>> webkitclient_impl.cc was linking against the >>> webkit/api/src/ChromiumCurrentTime.cpp version. >>> >> >> Arrrgghhhhhh >> >> Is there a way we can compile-time detect similar symbol names like this >> and throw fits (with a whitelist for any we know are OK)? Seems like this >> is an issue for something with linking that maruel might have looked at long >> ago. >> >> PKJ >> >> >> >> > --~--~---------~--~----~------------~-------~--~----~ Chromium Developers mailing list: [email protected] View archives, change email options, or unsubscribe: http://groups.google.com/group/chromium-dev -~----------~----~----~----~------~----~------~--~---
