Change Timers to use only Seconds, not raw doubles or std::chrono
Created attachment 295437 [details] Patch
Attachment 295437 [details] did not pass style-queue: ERROR: Source/WebKit2/WebProcess/WebPage/WebPage.cpp:317: More than one command on the same line in if [whitespace/parens] [4] ERROR: Source/WebCore/loader/cache/MemoryCache.cpp:774: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/Timer.h:159: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 3 in 247 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 295438 [details] Patch
Attachment 295438 [details] did not pass style-queue: ERROR: Source/WebKit2/WebProcess/WebPage/WebPage.cpp:317: More than one command on the same line in if [whitespace/parens] [4] ERROR: Source/WebCore/platform/Timer.h:159: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 2 in 260 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 295439 [details] Patch
Attachment 295439 [details] did not pass style-queue: ERROR: Source/WebKit2/WebProcess/WebPage/WebPage.cpp:317: More than one command on the same line in if [whitespace/parens] [4] ERROR: Source/WebCore/platform/Timer.h:159: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 2 in 291 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 295440 [details] Patch
Attachment 295440 [details] did not pass style-queue: ERROR: Source/WebKit2/WebProcess/WebPage/WebPage.cpp:317: More than one command on the same line in if [whitespace/parens] [4] ERROR: Source/WebCore/platform/Timer.h:159: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 2 in 291 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 295442 [details] Patch
Attachment 295442 [details] did not pass style-queue: ERROR: Source/WebKit2/WebProcess/WebPage/WebPage.cpp:317: More than one command on the same line in if [whitespace/parens] [4] ERROR: Source/WebCore/platform/Timer.h:159: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 2 in 293 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 295442 [details] Patch Attachment 295442 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/2575692 New failing tests: http/tests/eventsource/eventsource-retry-precision.html http/tests/eventsource/eventsource-reconnect-during-navigate-crash.html
Created attachment 295444 [details] Archive of layout-test-results from ews103 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 295442 [details] Patch Attachment 295442 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/2575681 New failing tests: http/tests/eventsource/eventsource-retry-precision.html editing/input/password-echo-textnode.html http/tests/eventsource/eventsource-reconnect-during-navigate-crash.html
Created attachment 295445 [details] Archive of layout-test-results from ews124 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 295442 [details] Patch Attachment 295442 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/2575695 New failing tests: http/tests/eventsource/eventsource-retry-precision.html http/tests/eventsource/eventsource-reconnect-during-navigate-crash.html
Created attachment 295446 [details] Archive of layout-test-results from ews117 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews117 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 295442 [details] Patch Attachment 295442 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/2575713 New failing tests: http/tests/eventsource/eventsource-retry-precision.html http/tests/eventsource/eventsource-reconnect-during-navigate-crash.html
Created attachment 295447 [details] Archive of layout-test-results from ews107 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Created attachment 295449 [details] Patch
Attachment 295449 [details] did not pass style-queue: ERROR: Source/WebKit2/WebProcess/WebPage/WebPage.cpp:317: More than one command on the same line in if [whitespace/parens] [4] ERROR: Source/WebCore/platform/Timer.h:159: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 2 in 296 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 295454 [details] Almost complete other than some Settings-related bindings issues
Created attachment 295455 [details] Almost complete other than some Settings-related bindings issues
Attachment 295455 [details] did not pass style-queue: ERROR: Source/WebKit2/WebProcess/WebPage/WebPage.cpp:317: More than one command on the same line in if [whitespace/parens] [4] ERROR: Source/WebCore/platform/Timer.h:159: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 2 in 296 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 295466 [details] Patch
Attachment 295466 [details] did not pass style-queue: ERROR: Source/WebKit2/WebProcess/WebPage/WebPage.cpp:317: More than one command on the same line in if [whitespace/parens] [4] ERROR: Source/WebCore/platform/Timer.h:159: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 2 in 303 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 295467 [details] Patch
Attachment 295467 [details] did not pass style-queue: ERROR: Source/WebKit2/WebProcess/WebPage/WebPage.cpp:317: More than one command on the same line in if [whitespace/parens] [4] ERROR: Source/WebCore/platform/Timer.h:159: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 2 in 303 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 295466 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=295466&action=review > Source/WebCore/Modules/mediasession/WebMediaSessionManager.cpp:39 > +static const Seconds taskDelayInterval = Seconds(1.0 / 10); Is the division here needed? Can this just be 0.1_s? > Source/WebCore/Modules/mediasession/WebMediaSessionManager.cpp:449 > + Seconds interval = didPlayToEnd ? watchdogTimerIntervalAfterPlayingToEnd : watchdogTimerIntervalAfterPausing; auto? > Source/WebCore/bindings/js/JSInternalSettingsGeneratedCustom.cpp:49 > + impl.setIncrementalRenderingSuppressionTimeout(Seconds(incrementalRenderingSuppressionTimeout)); I'm not a fan of adding custom bindings for something like this. Can we instead have the settings generator add a setIncrementalRenderingSuppressionTimeoutAsDouble (or something) and have that called? > Source/WebCore/bindings/js/JSInternalSettingsGeneratedCustom.cpp:65 > + impl.setPasswordEchoDuration(Seconds(passwordEchoDuration)); Same as above. > Source/WebCore/css/StyleResolver.cpp:1243 > + static const Seconds matchedDeclarationCacheSweepTime = 60_s; > + m_matchedPropertiesCacheSweepTimer.startOneShot(matchedDeclarationCacheSweepTime); Not sure this constant really adds much. > Source/WebCore/editing/AlternativeTextController.cpp:143 > + const Seconds correctionPanelTimerInterval = 0.3_s; auto? > Source/WebCore/editing/FrameSelection.cpp:2048 > + if (Seconds blinkInterval = m_frame->page()->theme().caretBlinkInterval()) auto? > Source/WebCore/html/HTMLMediaElement.cpp:164 > +static const Seconds HideMediaControlsAfterEndedDelay = Seconds(6); 6_s? > Source/WebCore/html/HTMLMediaElement.cpp:1970 > + m_progressEventTimer.startRepeating(0.350_s); I think this might read better as 350_ms given the comment. > Source/WebCore/html/MediaController.cpp:678 > + MonotonicTime now = MonotonicTime::now(); > + Seconds timedelta = now - m_previousTimeupdateTime; auto? > Source/WebCore/html/MediaElementSession.cpp:60 > +static const Seconds elementMainContentCheckInterval = 0.25_s; 250_ms? > Source/WebCore/html/shadow/MediaControlElements.cpp:140 > + Seconds duration = document().page() ? document().page()->theme().mediaControlsFadeOutDuration() : 0_s; auto? > Source/WebCore/html/shadow/MediaControlElements.cpp:191 > + Seconds duration = document().page() ? document().page()->theme().mediaControlsFadeInDuration() : 0_s; auto? > Source/WebCore/html/shadow/MediaControlElements.cpp:194 > + setInlineStyleProperty(CSSPropertyTransitionDuration, duration.value(), CSSPrimitiveValue::CSS_S); Should we have an overload of setInlineStyleProperty that takes a Seconds? > Source/WebCore/html/shadow/MediaControlElements.cpp:208 > + Seconds duration = document().page() ? document().page()->theme().mediaControlsFadeOutDuration() : 0_s; auto? > Source/WebCore/html/track/VTTRegion.cpp:59 > +static const Seconds scrollTime = Seconds(0.433); _s or _ms? > Source/WebCore/html/track/VTTRegion.cpp:406 > + Seconds duration = isScrollingRegion() ? scrollTime : 0_s; auto? > Source/WebCore/inspector/InspectorOverlay.cpp:512 > + const Seconds paintRectsUpdateInterval = 0.032_s; auto? _ms? > Source/WebCore/loader/cache/MemoryCache.cpp:54 > +static const Seconds defaultDecodedDataDeletionInterval = 0_s; why remove the auto? > Source/WebCore/page/DOMTimer.cpp:55 > +static const Seconds maxIntervalForUserGestureForwarding = 1_s; // One second matches Gecko. > +static const Seconds minIntervalForNonUserObservableChangeTimers = 1_s; // Empirically determined to maximize battery life. Why remove auto? > Source/WebCore/page/DOMTimer.cpp:434 > + // Do we really want to initialize randomizedProportion just once? I don't think we need this comment. If you want to keep it, please add a FIXME. > Source/WebCore/page/Frame.cpp:125 > +static const Seconds scrollFrequency = Seconds(1.0 / 60); This is a terrifying constant :). > Source/WebCore/page/FrameView.cpp:2844 > + static const Seconds speculativeTilingEnableDelay = 0.5_s; Is this constant needed? > Source/WebCore/page/animation/AnimationController.cpp:51 > +static const Seconds animationTimerDelay = Seconds(1.0 / 60); Not new, but this really seems like it should not be a constant, but rather something you get from settings. > Source/WebCore/platform/ThreadTimers.cpp:47 > +static const Seconds maxDurationOfFiringTimers = Seconds::fromMilliseconds(50); _ms? > Source/WebCore/platform/ThreadTimers.cpp:67 > + m_pendingSharedTimerFireTime = MonotonicTime(); Can this use { }? > Source/WebCore/platform/ThreadTimers.cpp:84 > + m_pendingSharedTimerFireTime = MonotonicTime(); { }? > Source/WebCore/platform/ThreadTimers.cpp:88 > + MonotonicTime nextFireTime = m_timerHeap.first()->m_nextFireTime; > + MonotonicTime currentMonotonicTime = MonotonicTime::now(); auto? > Source/WebCore/platform/ThreadTimers.cpp:106 > + m_pendingSharedTimerFireTime = MonotonicTime(); { } > Source/WebCore/platform/ThreadTimers.cpp:109 > + MonotonicTime fireTime = MonotonicTime::now(); > + MonotonicTime timeToQuit = fireTime + maxDurationOfFiringTimers; auto? > Source/WebCore/platform/ThreadTimers.cpp:114 > + timer->m_nextFireTime = MonotonicTime(); > + timer->m_unalignedNextFireTime = MonotonicTime(); { }? > Source/WebCore/platform/ThreadTimers.cpp:117 > + Seconds interval = timer->repeatInterval(); auto? > Source/WebCore/platform/ThreadTimers.cpp:140 > + m_pendingSharedTimerFireTime = MonotonicTime(); { }? > Source/WebCore/platform/Timer.cpp:230 > + MonotonicTime current = MonotonicTime::now(); auto? > Source/WebCore/platform/Timer.cpp:296 > + MonotonicTime fireTime = m_nextFireTime; auto? > Source/WebCore/platform/Timer.cpp:379 > + MonotonicTime oldTime = m_nextFireTime; auto? > Source/WebCore/platform/cocoa/ScrollController.mm:79 > +static const Seconds statelessScrollSnapDelay = Seconds(0.5); 0.5_s? > Source/WebCore/platform/graphics/ShadowBlur.cpp:127 > + const Seconds scratchBufferPurgeInterval = 2_s; auto? > Source/WebCore/platform/graphics/ca/TileGrid.cpp:384 > + Seconds timeUntilCohortExpires = cohort.timeUntilExpiration(); auto? > Source/WebCore/platform/graphics/ca/TileGrid.cpp:500 > + const Seconds cohortRemovalTimerSeconds = 1_s; You can remove the Seconds suffix. > Source/WebCore/platform/graphics/ca/TileGrid.cpp:509 > + const Seconds cohortLifeTime = Seconds(2); 2_s? auto? > Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.cpp:37 > +static const Seconds subimageCacheClearDelay = 1_s; why remove the auto? > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:370 > + static const Seconds readyStateTimerDelay = 60_s; auto? > Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedImageBacking.cpp:147 > +static const Seconds clearContentsTimerInterval = Seconds(3); 3_s? > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:84 > +static const Seconds defaultWatchdogTimerInterval = Seconds(1); 1_s? > Source/WebCore/rendering/RenderMarquee.cpp:171 > + m_timer.startRepeating(Seconds::fromMilliseconds(speed())); Really got to make this use RAF ;). > Source/WebCore/replay/EventLoopInputDispatcher.cpp:104 > + Seconds targetInterval = m_currentWork.timestamp - m_previousInputTimestamp; > + Seconds elapsed = MonotonicTime::now() - m_previousDispatchStartTime; auto? > Source/WebCore/svg/animation/SMILTimeContainer.cpp:207 > SMILTime delay = std::max(fireTime - elapsed, minimumDelay); Perhaps SMILTime should have a toSeconds() function that returns a WTF::Seconds? > Source/WebCore/xml/XMLHttpRequest.cpp:248 > + Seconds interval = Seconds::fromMilliseconds(m_timeoutMilliseconds ? m_timeoutMilliseconds : 60000) - (MonotonicTime::now() - m_sendingTime); auto? > Source/WebKit/Storage/StorageAreaSync.cpp:44 > +static const Seconds storageSyncInterval = Seconds(1); 1_s? > Source/WebKit/Storage/StorageTracker.cpp:54 > // after it has been idle for m_StorageDatabaseIdleInterval seconds. m_StorageDatabaseIdleInterval -> m_storageDatabaseIdleInterval > Source/WebKit/mac/Plugins/WebNetscapePluginView.mm:114 > +static const Seconds ThrottledTimerInterval = Seconds(0.25); _s? > Source/WebKit/mac/Plugins/WebNetscapePluginView.mm:133 > + Seconds timeInterval = Seconds::fromMilliseconds(m_interval); auto? > Source/WebKit/win/Plugins/PluginMessageThrottlerWin.cpp:42 > +static const Seconds MessageThrottleTimeInterval = Seconds(0.016); _s or _ms? > Source/WebKit2/NetworkProcess/NetworkLoad.cpp:314 > + Seconds delay = NetworkProcess::singleton().loadThrottleLatency(); auto? > Source/WebKit2/NetworkProcess/PingLoad.h:49 > + m_timeoutTimer.startOneShot(60000_s); Seems like this should _min > Source/WebKit2/NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp:49 > +static const Seconds preloadedEntryLifetime = 10_s; Why remove the auto? > Source/WebKit2/Shared/CacheModel.cpp:112 > + deadDecodedDataDeletionInterval = 60_s; 1_min? > Source/WebKit2/Shared/mac/RemoteLayerBackingStoreCollection.mm:35 > +const Seconds volatileBackingStoreAgeThreshold = Seconds(1); > +const Seconds volatileSecondaryBackingStoreAgeThreshold = Seconds(0.2); > +const Seconds volatilityTimerInterval = Seconds(0.2); _s? > Source/WebKit2/UIProcess/Cocoa/ViewGestureController.cpp:44 > +static const Seconds swipeSnapshotRemovalWatchdogAfterFirstVisuallyNonEmptyLayoutDuration = Seconds(3); > +static const Seconds swipeSnapshotRemovalActiveLoadMonitoringInterval = Seconds(0.25); _s? > Source/WebKit2/UIProcess/DrawingAreaProxyImpl.cpp:218 > + m_timer.startOneShot(Seconds(1)); 1_s? > Source/WebKit2/UIProcess/DrawingAreaProxyImpl.cpp:230 > + m_startTime = MonotonicTime(); { }? > Source/WebKit2/UIProcess/Plugins/PluginProcessProxy.cpp:49 > +static const Seconds minimumLifetime = Seconds(2 * 60); 2_min? > Source/WebKit2/UIProcess/Plugins/PluginProcessProxy.cpp:52 > +static const Seconds shutdownTimeout = Seconds(1 * 60); 1_min? > Source/WebKit2/UIProcess/ResponsivenessTimer.cpp:31 > +static const Seconds responsivenessTimeout = Seconds(3); 3_s? > Source/WebKit2/UIProcess/WebProcessPool.cpp:1422 > + static const Seconds maximumTimerThrottlePerPage = Seconds::fromMilliseconds(200) * 100; 200_ms (or even 20_s for the whole thing). > Source/WebKit2/UIProcess/WebProcessPool.cpp:1424 > + Seconds limit = maximumTimerThrottlePerPage * m_hiddenPageThrottlingAutoIncreasesCounter.value(); auto? > Source/WebKit2/WebProcess/Plugins/Netscape/NetscapePlugin.cpp:329 > + Seconds timeInterval = Seconds::fromMilliseconds(m_interval); auto? > Source/WebKit2/WebProcess/Plugins/PluginView.cpp:81 > +static const Seconds pluginSnapshotTimerDelay = Seconds(1.1); 1.1_s? > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:2054 > + Seconds newInterval = m_layerVolatilityTimer.repeatInterval() * 2; auto? > Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm:309 > + const Seconds initialFlushDelay = 0.5_s; > + const Seconds flushDelay = 1.5_s; > + Seconds throttleDelay = m_isThrottlingLayerFlushes ? (m_isInitialThrottledLayerFlush ? initialFlushDelay : flushDelay) : 0_s; auto? > Source/WebKit2/WebProcess/WebProcess.cpp:145 > +static const Seconds nonVisibleProcessCleanupDelay = Seconds(10); 10_s?
Comment on attachment 295467 [details] Patch rs=me when you fix all the builds / tests.
Created attachment 295482 [details] Patch
Created attachment 296118 [details] Patch
Created attachment 296120 [details] Patch
Attachment 296120 [details] did not pass style-queue: ERROR: Source/WebKit2/WebProcess/WebPage/WebPage.cpp:318: More than one command on the same line in if [whitespace/parens] [4] ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] ERROR: Source/WebCore/platform/Timer.h:159: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 3 in 295 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 296128 [details] Patch
Attachment 296128 [details] did not pass style-queue: ERROR: Source/WebKit2/WebProcess/WebPage/WebPage.cpp:318: More than one command on the same line in if [whitespace/parens] [4] ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] ERROR: Source/WebCore/platform/Timer.h:159: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 3 in 296 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 296129 [details] Patch
Attachment 296129 [details] did not pass style-queue: ERROR: Source/WebKit2/WebProcess/WebPage/WebPage.cpp:318: More than one command on the same line in if [whitespace/parens] [4] ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] ERROR: Source/WebCore/platform/Timer.h:159: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 3 in 298 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 296130 [details] Patch
Attachment 296130 [details] did not pass style-queue: ERROR: Source/WebKit2/WebProcess/WebPage/WebPage.cpp:318: More than one command on the same line in if [whitespace/parens] [4] ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] ERROR: Source/WebCore/platform/Timer.h:159: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 3 in 301 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 296131 [details] Patch
Attachment 296131 [details] did not pass style-queue: ERROR: Source/WebKit2/WebProcess/WebPage/WebPage.cpp:318: More than one command on the same line in if [whitespace/parens] [4] ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] ERROR: Source/WebCore/platform/Timer.h:159: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 3 in 301 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 296143 [details] Patch
Attachment 296143 [details] did not pass style-queue: ERROR: Source/WebKit2/WebProcess/WebPage/WebPage.cpp:318: More than one command on the same line in if [whitespace/parens] [4] ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] ERROR: Source/WebCore/platform/Timer.h:159: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 3 in 302 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 296148 [details] Patch
Attachment 296148 [details] did not pass style-queue: ERROR: Source/WebKit2/WebProcess/WebPage/WebPage.cpp:318: More than one command on the same line in if [whitespace/parens] [4] ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] ERROR: Source/WebCore/platform/Timer.h:159: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 3 in 303 files If any of these errors are false positives, please file a bug against check-webkit-style.
This all happened.