Bug 154935 – Disable timer throttling increases for visually idle / active pages.

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Gavin Barraclough]

Currently any page that is visually idle can timer throttle, and all are eligible for throttling to increase.

Instead, still allow any visually idle page to timer throttle, but only allow increasing in those that are fully hidden & inactive (no page loading or media activity).

[Gavin Barraclough]

Created attachment 272699 [details]
Fix

[Gavin Barraclough]

Comment on attachment 272699 [details]
Fix

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

> Source/WebCore/page/Page.cpp:1188
> +    // Timer throttling disabled is page is visually active, or disabled by setting.

Timer throttling disabled *if*

[Chris Dumez]

Comment on attachment 272699 [details]
Fix

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

r=me, makes sense.

> Source/WebCore/page/Page.cpp:1189
> +    if (!(m_viewState & ViewState::IsVisuallyIdle) || !m_settings->hiddenPageDOMTimerThrottlingEnabled()) {

nit: I think logically the !m_settings->hiddenPageDOMTimerThrottlingEnabled() should come first.

> Source/WebCore/page/Page.cpp:1196
> +    if (m_viewState & ViewState::IsVisible || m_pageThrottler.activityState() || !m_settings->hiddenPageDOMTimerThrottlingAutoIncreases()) {

nit: I think logically the !m_settings->hiddenPageDOMTimerThrottlingAutoIncreases() condition should come first.

> Source/WebCore/page/Page.cpp:1467
> +    updateTimerThrottlingState();

At this point, it may be clearer to move this one (and the one in setIsVisuallyIdleInternal()), to Page::setViewState(ViewState::Flags).
Something like:
if (changed & ViewState::IsVisible || changed & ViewState::IsVisuallyIdle)
  updateTimerThrottlingState();

What do you think?

[Gavin Barraclough]

Transmitting file data ..
Committed revision 197472.