Allow timer throttling to be proportional to the duration of time the page has been hidden.
Created attachment 272004 [details] Fix
Comment on attachment 272004 [details] Fix View in context: https://bugs.webkit.org/attachment.cgi?id=272004&action=review > Source/WebCore/page/Page.h:620 > + Optional<double> m_timerThrottlingEnabledTime; It would be more stylish to use std::chrono::steady_clock::time_point and pals. > Source/WebCore/page/Settings.cpp:215 > + , m_hiddenPageDOMTimerThrottlingAutoIncreaseLimit(0.0) You could use { } initialization in the header. > Source/WebCore/page/Settings.h:345 > bool m_hiddenPageDOMTimerThrottlingEnabled : 1; > #endif > + double m_hiddenPageDOMTimerThrottlingAutoIncreaseLimit; > + > bool m_hiddenPageCSSAnimationSuspensionEnabled : 1; > bool m_fontFallbackPrefersPictographs : 1; Please don't put this in the middle of a bitfield (though the bitfield itself is silly and could be removed).
Comment on attachment 272004 [details] Fix View in context: https://bugs.webkit.org/attachment.cgi?id=272004&action=review > Source/WebCore/page/Page.cpp:1177 > + m_timerThrottlingEnabledTime = enabled ? monotonicallyIncreasingTime() : Optional<double>(); m_timerThrottlingEnabledTime = enabled ? monotonicallyIncreasingTime() : NullOpt; did not build? > Source/WebCore/page/Page.cpp:1194 > + if (frame->document()) Would be nice to store the returned document in a variable: if (auto* document = frame->document()) document->didChangeTimerAlignmentInterval();
(In reply to comment #3) > Comment on attachment 272004 [details] > Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=272004&action=review > > > Source/WebCore/page/Page.cpp:1177 > > + m_timerThrottlingEnabledTime = enabled ? monotonicallyIncreasingTime() : Optional<double>(); > > m_timerThrottlingEnabledTime = enabled ? monotonicallyIncreasingTime() : > NullOpt; did not build? > > > Source/WebCore/page/Page.cpp:1194 > > + if (frame->document()) > > Would be nice to store the returned document in a variable: > if (auto* document = frame->document()) > document->didChangeTimerAlignmentInterval(); (In reply to comment #2) > Comment on attachment 272004 [details] > Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=272004&action=review > > > Source/WebCore/page/Page.h:620 > > + Optional<double> m_timerThrottlingEnabledTime; > > It would be more stylish to use std::chrono::steady_clock::time_point and > pals. Will do - I'm going to land as is (since everything is currently using double), and make a patch to push std::chrono types through all the timer alignment code. > > Source/WebCore/page/Settings.cpp:215 > > + , m_hiddenPageDOMTimerThrottlingAutoIncreaseLimit(0.0) > > You could use { } initialization in the header. Done. > > Source/WebCore/page/Settings.h:345 > > bool m_hiddenPageDOMTimerThrottlingEnabled : 1; > > #endif > > + double m_hiddenPageDOMTimerThrottlingAutoIncreaseLimit; > > + > > bool m_hiddenPageCSSAnimationSuspensionEnabled : 1; > > bool m_fontFallbackPrefersPictographs : 1; > > Please don't put this in the middle of a bitfield (though the bitfield > itself is silly and could be removed). Fixed. (In reply to comment #3) > Comment on attachment 272004 [details] > Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=272004&action=review > > > Source/WebCore/page/Page.cpp:1177 > > + m_timerThrottlingEnabledTime = enabled ? monotonicallyIncreasingTime() : Optional<double>(); > > m_timerThrottlingEnabledTime = enabled ? monotonicallyIncreasingTime() : > NullOpt; did not build? I can't :'-( - one side of the ?: needs to explicitly be the optional type. > > Source/WebCore/page/Page.cpp:1194 > > + if (frame->document()) > > Would be nice to store the returned document in a variable: > if (auto* document = frame->document()) > document->didChangeTimerAlignmentInterval(); Done. Based on feedback from Sam going to move the limit to ChromeClient (since we've going to vary this dynamically). For now going to just remove the limit (am landing disabled anyway), and will reintroduce this in another patch.
Created attachment 272059 [details] Update with review fixes, setting -> bool (temporarily removed limit)
Attachment 272059 [details] did not pass style-queue: ERROR: Source/WebCore/page/Page.cpp:1202: One line control clauses should not use braces. [whitespace/braces] [4] Total errors found: 1 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 272059 [details] Update with review fixes, setting -> bool (temporarily removed limit) View in context: https://bugs.webkit.org/attachment.cgi?id=272059&action=review r=me > Source/WebCore/ChangeLog:10 > + in exponential steps, spaced exponentially far apart, until a limit is The "until a limit is reached" part is no longer true.
Transmitting file data ..... Committed revision 197006.