Bug 112308 – Add runtime setting for hidden page DOM timer throttling and CSS animation suspension

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Kiran Muppala]

https://bugs.webkit.org/show_bug.cgi?id=98474 added a feature define, HIDDEN_PAGE_DOM_TIMER_THROTTLING, to enable throttling of Javascript timers when the visibility state of a page is set to "Hidden".  Later https://bugs.webkit.org/show_bug.cgi?id=106940 added logic to suspend CSS animations for "Hidden" pages.  These two features can break rendering of off screen web views, particularly Javascript timer throttling.  Hence it is preferable to add a setting for these features.

[Kiran Muppala]

<rdar://problem/13337564>

[Kiran Muppala]

The settings should be disabled by default to minimize compatibility issues.

[Kiran Muppala]

Created attachment 193040 [details]
Patch

[Early Warning System Bot]

Comment on attachment 193040 [details]
Patch

Attachment 193040 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17166388

[EFL EWS Bot]

Comment on attachment 193040 [details]
Patch

Attachment 193040 [details] did not pass efl-ews (efl):
Output: http://webkit-commit-queue.appspot.com/results/17133130

[Kiran Muppala]

Created attachment 193051 [details]
Patch

[Kiran Muppala]

On further discussion with Maciej, it was decided that this setting should be enabled by default in WebKit2, since compatibility is less of a issue there.

[Kiran Muppala]

Created attachment 193185 [details]
Patch

[Kiran Muppala]

(In reply to comment #8)
> Created an attachment (id=193185) [details]
> Patch

Updated patch to make the settings default to false in WebKit1 and true in WebKit2.

[Kiran Muppala]

Created attachment 193422 [details]
Patch

[Kiran Muppala]

(In reply to comment #10)
> Created an attachment (id=193422) [details]
> Patch

Modified patch to only enable by default on Mac.  Since WebKit2 might be a public API on other platforms.

[Gavin Barraclough]

Comment on attachment 193422 [details]
Patch

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

> Source/WebCore/page/Page.cpp:1453
> +#if ENABLE(PAGE_VISIBILITY_API)

Why this #ifdef? – setVisibilityState is compiled in if ENABLE(HIDDEN_PAGE_DOM_TIMER_THROTTLING) and not ENABLE(PAGE_VISIBILITY_API) is defined, "setTimerAlignmentInterval(Settings::hiddenPageDOMTimerAlignmentInterval());" may have been called (line 1239/1240).

> Source/WebCore/page/Page.cpp:1458
> +        setTimerAlignmentInterval(Settings::defaultDOMTimerAlignmentInterval());

This will unnecessarily reset the timer interval where m_visibilityState != WebCore::PageVisibilityStateHidden.  Could this not be more like hiddenPageCSSAnimationSuspensionStateChanged, below?

#if ENABLE(HIDDEN_PAGE_DOM_TIMER_THROTTLING)
void Page::hiddenPageDOMTimerThrottlingStateChanged()
{
    if (m_visibilityState == WebCore::PageVisibilityStateHidden) {
        if (m_settings->hiddenPageDOMTimerThrottlingEnabled())
            setTimerAlignmentInterval(Settings::hiddenPageDOMTimerAlignmentInterval());
        else
            setTimerAlignmentInterval(Settings::defaultDOMTimerAlignmentInterval());
    }
}
#endif

[Kiran Muppala]

(In reply to comment #12)
> (From update of attachment 193422 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=193422&action=review
> 
> > Source/WebCore/page/Page.cpp:1453
> > +#if ENABLE(PAGE_VISIBILITY_API)
> 
> Why this #ifdef? – setVisibilityState is compiled in if ENABLE(HIDDEN_PAGE_DOM_TIMER_THROTTLING) and not ENABLE(PAGE_VISIBILITY_API) is defined, "setTimerAlignmentInterval(Settings::hiddenPageDOMTimerAlignmentInterval());" may have been called (line 1239/1240).
> 
> > Source/WebCore/page/Page.cpp:1458
> > +        setTimerAlignmentInterval(Settings::defaultDOMTimerAlignmentInterval());
> 
> This will unnecessarily reset the timer interval where m_visibilityState != WebCore::PageVisibilityStateHidden.  Could this not be more like hiddenPageCSSAnimationSuspensionStateChanged, below?
> 

HIDDEN_PAGE_DOM_TIMER_THROTTLING doesn't compile in m_visibilityState needed to remember the last set value.  Only PAGE_VISIBILITY_API does that.  On Mac, this is not a concern because we enable both.  And I have a separate bug https://bugs.webkit.org/show_bug.cgi?id=112323, to remove HIDDEN_PAGE_DOM_TIMER_THROTTLING entirely and just use PAGE_VISIBILITY_API.  Then the code will match CSS animation suspension.

> #if ENABLE(HIDDEN_PAGE_DOM_TIMER_THROTTLING)
> void Page::hiddenPageDOMTimerThrottlingStateChanged()
> {
>     if (m_visibilityState == WebCore::PageVisibilityStateHidden) {
>         if (m_settings->hiddenPageDOMTimerThrottlingEnabled())
>             setTimerAlignmentInterval(Settings::hiddenPageDOMTimerAlignmentInterval());
>         else
>             setTimerAlignmentInterval(Settings::defaultDOMTimerAlignmentInterval());
>     }
> }
> #endif

[Gavin Barraclough]

r is me.

It's a bit gross that we can forget the state we're in, but I guess this is at least conservative in favouring the default timeout.

[Kiran Muppala]

Committed r146704: <http://trac.webkit.org/changeset/146704>

[David Kilzer (:ddkilzer)]

(In reply to comment #15)
> Committed r146704: <http://trac.webkit.org/changeset/146704>

Build fix in <http://trac.webkit.org/r146729>

[Kiran Muppala]

(In reply to comment #16)
> (In reply to comment #15)
> > Committed r146704: <http://trac.webkit.org/changeset/146704>
> 
> Build fix in <http://trac.webkit.org/r146729>

I missed that, EWS wasn't of help.  Thanks for the fix.