Bug 154935 - Disable timer throttling increases for visually idle / active pages.
Summary: Disable timer throttling increases for visually idle / active pages.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Gavin Barraclough
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-03-02 15:51 PST by Gavin Barraclough
Modified: 2016-03-02 16:19 PST (History)
0 users

See Also:


Attachments
Fix (3.19 KB, patch)
2016-03-02 15:54 PST, Gavin Barraclough
cdumez: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.