Bug 154935

Summary: Disable timer throttling increases for visually idle / active pages.
Product: WebKit Reporter: Gavin Barraclough <barraclough>
Component: WebKit Misc.Assignee: Gavin Barraclough <barraclough>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Fix cdumez: review+

Description Gavin Barraclough 2016-03-02 15:51:02 PST
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).
Comment 1 Gavin Barraclough 2016-03-02 15:54:45 PST
Created attachment 272699 [details]
Fix
Comment 2 Gavin Barraclough 2016-03-02 16:01:01 PST
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*
Comment 3 Chris Dumez 2016-03-02 16:07:38 PST
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?
Comment 4 Gavin Barraclough 2016-03-02 16:19:02 PST
Transmitting file data ..
Committed revision 197472.