Support time zone change notifications on linux#64562
Conversation
|
EWS run on previous version of this PR (hash cc3da80) Details |
cc3da80 to
246cf78
Compare
|
EWS run on previous version of this PR (hash 246cf78) Details |
Constellation
left a comment
There was a problem hiding this comment.
Maybe we can first try to use D-Bus, and then we can fallback to FilePathWatcher.
|
EWS run on previous version of this PR (hash d875e19) Details |
iOS Safer C++ Build #19907 (d875e19)❌ Found 1 failing file with 1 issue. Please address these issues before landing. See WebKit Guidelines for Safer C++ Programming.
|
macOS Safer C++ Build #101571 (d875e19)❌ Found 1 failing file with 1 issue. Please address these issues before landing. See WebKit Guidelines for Safer C++ Programming.
|
d875e19 to
c72caff
Compare
|
EWS run on previous version of this PR (hash c72caff) Details |
macOS Safer C++ Build #102002 (c72caff)❌ Found 1 failing file with 1 issue. Please address these issues before landing. See WebKit Guidelines for Safer C++ Programming.
|
iOS Safer C++ Build #20358 (c72caff)❌ Found 1 failing file with 1 issue. Please address these issues before landing. See WebKit Guidelines for Safer C++ Programming.
|
c72caff to
85f8757
Compare
|
EWS run on previous version of this PR (hash 85f8757) Details |
85f8757 to
5be1bd5
Compare
|
EWS run on previous version of this PR (hash 5be1bd5) Details |
5be1bd5 to
5a95593
Compare
|
EWS run on previous version of this PR (hash 5a95593) Details |
macOS Safer C++ Build #102829 (5a95593)❌ Found 1 failing file with 1 issue. Please address these issues before landing. See WebKit Guidelines for Safer C++ Programming. |
Constellation
left a comment
There was a problem hiding this comment.
Maybe, it is good to have some eyes from glib / GTK folks, but Cocoa side and JSC change looks fine to me.
| // observe time zone changes. | ||
| dateCache.resetIfNecessary(); | ||
| if (dateCache.hasTimeZoneChange()) [[unlikely]] | ||
| dateCache.clearForTimezoneChange(); |
| auto scope = DECLARE_THROW_SCOPE(vm); | ||
|
|
||
| String tz = callFrame->argument(0).toWTFString(globalObject); | ||
| RETURN_IF_EXCEPTION(scope, encodedJSValue()); |
| auto scope = DECLARE_THROW_SCOPE(vm); | ||
|
|
||
| String tz = callFrame->argument(0).toWTFString(globalObject); | ||
| RETURN_IF_EXCEPTION(scope, encodedJSValue()); |
5a95593 to
8c3cd87
Compare
|
EWS run on previous version of this PR (hash 8c3cd87) Details |
| if (name STREQUAL "GLib") | ||
| message(FATAL_ERROR "glib-2.0 is required") | ||
| endif () | ||
|
|
There was a problem hiding this comment.
If you don't have glib, then you get a recursion error instead of a proper error message. This mechanism requires the JSCOnly port to be built with glib now too (or rather, requires glib to run these tests), exposing this issue.
| { | ||
| static std::once_flag onceKey; | ||
| std::call_once(onceKey, [] { | ||
| localtimeWatcher() = makeUnique<FilePathWatcher>("/etc/localtime", [] { |
There was a problem hiding this comment.
Should probably use an ASCIILiteral instead of a raw const char*.
There was a problem hiding this comment.
Nice catch. I opted for a string to allow dynamic paths to work too, WDYT?
|
|
||
| WTF_MAKE_TZONE_ALLOCATED_IMPL(FilePathWatcher); | ||
|
|
||
| FilePathWatcher::FilePathWatcher(const char* path, Function<void()>&& handler) |
There was a problem hiding this comment.
ASCIILIteral instead of a raw const char*.
8c3cd87 to
b439d1e
Compare
|
EWS run on current version of this PR (hash b439d1e) Details |
https://bugs.webkit.org/show_bug.cgi?id=314414 Reviewed by Yusuke Suzuki. We add a timezone change observer for linux that watches both dbus and /etc/localtime. We add some testing helpers and some generated tests too. This uncovered an existing bug with DST time changes. * JSTests/stress/intl-datetimeformat-cache-dst-transition.js: Added. (expect): (setTimeout): * JSTests/stress/intl-datetimeformat-cache-tz-hit.js: Added. (expect): (testTZ): * JSTests/stress/intl-datetimeformat-cache-tz-watcher.js: Added. (expect): (slowTZ): (setTimeout): * JSTests/stress/intl-datetimeformat-stale-data-across-tz-change.js: Added. (expect): (setTimeout): * JSTests/stress/temporal-cache-dst-transition.js: Added. (expect): (setTimeout): * Source/JavaScriptCore/SaferCPPExpectations/UnretainedCallArgsCheckerExpectations: * Source/JavaScriptCore/runtime/DateInstanceCache.h: (JSC::DateInstanceCache::reset): * Source/JavaScriptCore/runtime/JSDateMath.cpp: (JSC::DateCache::DateCache): (JSC::retrieveTimeZoneInformation): (JSC::DateCache::clearForTimeZoneChange): (JSC::timeZoneChangeNotification): Deleted. (JSC::DateCache::resetIfNecessarySlow): Deleted. * Source/JavaScriptCore/runtime/JSDateMath.h: (JSC::DateCache::hasTimeZoneChange): (JSC::DateCache::resetIfNecessary): Deleted. * Source/JavaScriptCore/runtime/VM.cpp: (JSC::VM::executeEntryScopeServicesOnEntry): * Source/JavaScriptCore/tools/JSDollarVM.cpp: (JSC::JSC_DEFINE_HOST_FUNCTION): (JSC::JSDollarVM::finishCreation): * Source/WTF/WTF.xcodeproj/project.pbxproj: * Source/WTF/wtf/CMakeLists.txt: * Source/WTF/wtf/PlatformCocoa.cmake: * Source/WTF/wtf/PlatformGTK.cmake: * Source/WTF/wtf/PlatformJSCOnly.cmake: * Source/WTF/wtf/PlatformWPE.cmake: * Source/WTF/wtf/TimeZone.cpp: Added. (WTF::lastTimeZoneID): (WTF::timeZoneDidChange): (WTF::listenForTimeZoneChangeNotifications): (WTF::setHostTimeZoneForTesting): * Source/WTF/wtf/TimeZone.h: Added. * Source/WTF/wtf/cocoa/TimeZoneCocoa.cpp: Added. (WTF::timeZoneChangeNotification): (WTF::listenForTimeZoneChangeNotifications): * Source/WTF/wtf/glib/FilePathWatcher.cpp: Added. (WTF::FilePathWatcher::FilePathWatcher): (WTF::FilePathWatcher::~FilePathWatcher): (WTF::FilePathWatcher::fileChangedCallback): * Source/WTF/wtf/glib/FilePathWatcher.h: Added. (WTF::FilePathWatcher::isActive const): * Source/WTF/wtf/glib/TimeZoneGLib.cpp: Added. (WTF::localtimeWatcher): (WTF::timedate1Proxy): (WTF::timedate1PropertiesChanged): (WTF::onTimedate1ProxyReady): (WTF::listenForTimeZoneChangeNotifications): * Source/cmake/FindGLib.cmake: * Tools/Scripts/run-gtk-tests: * Tools/Scripts/run-timezone-glib-e2e-test: Added. (find_testwtf): (spawn_private_bus): (own_timedate1): (emit_timezone_changed): (main): * Tools/Scripts/run-wpe-tests: * Tools/TestWebKitAPI/PlatformGTK.cmake: * Tools/TestWebKitAPI/PlatformJSCOnly.cmake: * Tools/TestWebKitAPI/PlatformWPE.cmake: * Tools/TestWebKitAPI/Tests/WTF/glib/FilePathWatcher.cpp: Added. (TestWebKitAPI::TEST(WTF_FilePathWatcher, FiresOnWrite)): (TestWebKitAPI::TEST(WTF_FilePathWatcher, FiresOnSymlinkSwap)): (TestWebKitAPI::TEST(WTF_FilePathWatcher, IgnoresUnrelatedSiblings)): * Tools/TestWebKitAPI/Tests/WTF/glib/TimeZoneGLib.cpp: Added. (TestWebKitAPI::WTF_TimeZoneGLib::SetUpTestSuite): (TestWebKitAPI::WTF_TimeZoneGLib::TearDownTestSuite): (TestWebKitAPI::TEST_F(WTF_TimeZoneGLib, FiresOnTimezonePropertyChange)): (TestWebKitAPI::TEST_F(WTF_TimeZoneGLib, FiresOnInvalidatedTimezone)): (TestWebKitAPI::TEST_F(WTF_TimeZoneGLib, IgnoresUnrelatedProperties)): (TestWebKitAPI::TEST(WTF_TimeZoneGLib_External, ReceivesExternalSignal)): Canonical link: https://commits.webkit.org/313750@main
b439d1e to
11993c8
Compare
|
Committed 313750@main (11993c8): https://commits.webkit.org/313750@main Reviewed commits have been landed. Closing PR #64562 and removing active labels. |
11993c8
b439d1e