Bug 142837 – Clean up DOMTimer related settings

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Chris Dumez]

Clean up DOMTimer related settings

[Chris Dumez]

Created attachment 248962 [details]
Patch

[Chris Dumez]

Created attachment 248974 [details]
Patch

[Chris Dumez]

Created attachment 248982 [details]
Patch

[Chris Dumez]

*** Bug 61214 has been marked as a duplicate of this bug. ***

[Darin Adler]

Comment on attachment 248982 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=248982&action=review

> Source/WebCore/page/DOMTimer.cpp:62
> +const double DOMTimer::gDefaultMinimumInterval = 0.004; // 4 milliseconds.
> +const double DOMTimer::gDefaultAlignmentInterval = 0;
> +const double DOMTimer::gHiddenPageAlignmentInterval = 1.0; // 1 second.

I now see that you just moved this, but still:

Is this "g" prefix something we use in WebKit? I don’t think it is. I would have used no prefix at all, but others, I guess, use an "s_" prefix. Even better, just put these constants into the header file inside the static function member return statements and don’t bother with data members.

> Source/WebCore/page/Settings.cpp:449
> +    for (Frame* frame = &m_page->mainFrame(); frame; frame = frame->tree().traverseNextWithWrap(false)) {

There is no reason to use traverseNextWithWrap(false) instead of traverseNext(). This seems to be a mistake in Page::setMinimumTimerInterval and Page::setTimerThrottlingEnabled that we should fix and not repeat.

> Source/WebCore/page/Settings.h:166
> +    WEBCORE_EXPORT void setMinimumDOMTimerInterval(double); // Per-page; initialized to default value.

I don’t understand what the comment “per-page” means. Aren’t all settings on Settings equally per-page or not-per-page?

[Darin Adler]

Comment on attachment 248982 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=248982&action=review

> Source/WebCore/page/DOMTimer.h:53
> +        static double hiddenPageAlignmentInterval() { return gHiddenPageAlignmentInterval; }
> +        static double defaultAlignmentInterval() { return gDefaultAlignmentInterval; }

It’s more logical to list the default alignment interval first and then the hidden page alignment interval.

[Chris Dumez]

Created attachment 249038 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 249038 [details]
Patch

Clearing flags on attachment: 249038

Committed r181753: <http://trac.webkit.org/changeset/181753>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.