Summary: | Some timer alignment cleanup. | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Gavin Barraclough <barraclough> | ||||
Component: | WebKit Misc. | Assignee: | Gavin Barraclough <barraclough> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | cdumez, commit-queue, esprehn+autocc, kangil.han | ||||
Priority: | P2 | ||||||
Version: | Other | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Gavin Barraclough
2016-02-22 14:57:51 PST
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. |