Document shouldn't override Page's timer alignment policy with a lower alignment, and m_domTimerAlignmentInterval isn't really a Settings, it's just the current state for the page.
Created attachment 271963 [details] Fix
Comment on attachment 271963 [details] Fix View in context: https://bugs.webkit.org/attachment.cgi?id=271963&action=review You should change this to use std::chrono::milliseconds! > Source/WebCore/page/Page.h:620 > - bool m_timerThrottlingEnabled; > + bool m_timerThrottlingEnabledTime; > + double m_timerAlignmentInterval; Can this use Optional instead?
(In reply to comment #2) > Comment on attachment 271963 [details] > Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=271963&action=review > > You should change this to use std::chrono::milliseconds! Heh, probably! - sure I'll look into this. (But as a separate change - these intervals are all currently doubles & I should probably switch double -> chrono::milliseconds in a bunch of other places at the same time). > > Source/WebCore/page/Page.h:620 > > - bool m_timerThrottlingEnabled; > > + bool m_timerThrottlingEnabledTime; > > + double m_timerAlignmentInterval; > > Can this use Optional instead? Interesting. I don't think this should be an Optional - when timer throttling is disabled the timer alignment property does still hold a value (DOMTimer::defaultAlignmentInterval() - which theoretically could be non-zero). Also I plan on replacing m_timerThrottlingEnabled with the timestamp at which time throttling is enabled, so we know how long it has been - come to think of it, that probably should be an Optional.
Comment on attachment 271963 [details] Fix View in context: https://bugs.webkit.org/attachment.cgi?id=271963&action=review r=me % nits > Source/WebCore/dom/Document.cpp:2934 > + if (Page* page = this->page()) auto* > Source/WebCore/page/Page.cpp:208 > + , m_timerThrottlingEnabledTime(false) I prefer the previous name since this is a boolean.
> > Source/WebCore/page/Page.cpp:208 > > + , m_timerThrottlingEnabledTime(false) > > I prefer the previous name since this is a boolean. Oops, yes!
Transmitting file data ...... Committed revision 196971.