Bug 204713 – Throttling requestAnimationFrame should be controlled by RenderingUpdateScheduler

• Status: RESOLVED
• Resolution: FIXED
• Component: Animations

[Said Abou-Hallawa]

There should be two types of throttling:

1. Page throttling: This throttling will slow down all the RenderingUpdate steps.
2. Document throttling: This throttling will slow down the requestAnimationFrame step of a specific document.

[Said Abou-Hallawa]

Created attachment 384535 [details]
Patch

[Said Abou-Hallawa]

Created attachment 384557 [details]
Patch

[Said Abou-Hallawa]

Created attachment 384565 [details]
Patch

[Said Abou-Hallawa]

Created attachment 384582 [details]
Patch

[Said Abou-Hallawa]

Created attachment 384585 [details]
Patch

[Said Abou-Hallawa]

Created attachment 384597 [details]
Patch

[Said Abou-Hallawa]

<rdar://problem/55745055>

[Simon Fraser (smfr)]

Comment on attachment 384597 [details]
Patch

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

> Source/WebCore/ChangeLog:15
> +        1) Page throttling which slows down all the steps of RenderingUpdate.
> +        2) Document throttling which only slows down the rAF of the document.

Does this mean that a rendering update might happen but rAF does not fire because of Document throttling? I don't think we ever want this. I think if a rendering update happens, rAF has to fire. That implies that throttling only impacts rendering updates, which in turns means that we need to be able to throttle rendering updates in subframes independently from the Page.

It might be tricky to decide whether a subframe gets layout so that it can be painted by its parent, even if it didn't have a rendering update.

[Said Abou-Hallawa]

Comment on attachment 384597 [details]
Patch

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

>> Source/WebCore/ChangeLog:15
>> +        2) Document throttling which only slows down the rAF of the document.
> 
> Does this mean that a rendering update might happen but rAF does not fire because of Document throttling? I don't think we ever want this. I think if a rendering update happens, rAF has to fire. That implies that throttling only impacts rendering updates, which in turns means that we need to be able to throttle rendering updates in subframes independently from the Page.
> 
> It might be tricky to decide whether a subframe gets layout so that it can be painted by its parent, even if it didn't have a rendering update.

This patch fixes the bug which causes the rAF to run every 11ms instead of 30ms when a low power mode is turned on on an iOS device. Also it will not change any behavior. I think what you are asking is an enhancement which can be done in a separate patch. And I think the change should be small. For throttled iframes, we will need to move the decision to skip the RenderingUpdate from the ScriptedAnimationController to the Document or the Page. All we need is to skip more from the rendering update steps rather than the rAF only which we used to do before introducing the RenderingUpdate r244182.

[Simon Fraser (smfr)]

Comment on attachment 384597 [details]
Patch

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

> Source/WebCore/dom/ScriptedAnimationController.h:54
> +    int registerCallback(Ref<RequestAnimationFrameCallback>&&);

Why did the typedef go away?

> Source/WebCore/dom/ScriptedAnimationController.h:63
> -    enum class ThrottlingReason {
> -        VisuallyIdle                    = 1 << 0,
> -        OutsideViewport                 = 1 << 1,
> -        LowPowerMode                    = 1 << 2,
> -        NonInteractedCrossOriginFrame   = 1 << 3,
> -    };
> -    void addThrottlingReason(ThrottlingReason);
> -    void removeThrottlingReason(ThrottlingReason);
> -
> -    WEBCORE_EXPORT bool isThrottled() const;
> -    WEBCORE_EXPORT Seconds interval() const;
> +    
> +    void setOutsideViewport(bool outsideViewport) { m_outsideViewport = outsideViewport; }
> +    void setNonInteractedCrossOriginFrame(bool nonInteractedCrossOriginFrame) { m_nonInteractedCrossOriginFrame = nonInteractedCrossOriginFrame; }
> +    bool isThrottled() const { return interval() > fullSpeedAnimationInterval; }

I prefer the old version, which should use an OptionSet<> of reasons.

> Source/WebCore/page/Page.h:714
> +    Seconds preferredRenderingUpdateInterval() const;
> +    unsigned preferredFramesPerSecond() const;

It's really confusing to have both of these.

> Source/WebCore/page/RenderingUpdateScheduler.cpp:73
> +    if (interval > 1_s || !DisplayRefreshMonitorManager::sharedManager().scheduleAnimation(*this, m_page.preferredFramesPerSecond()))

Why the 1s cutoff here?

> Source/WebCore/page/RenderingUpdateScheduler.h:50
> +    void updateRenderingUpdateRate();

Let's call this adjustRenderingUpdateFrequency()

> Source/WebCore/page/RenderingUpdateScheduler.h:58
> +    void updatePreferredFramesPerSecond();

Confusing to have this and updateRenderingUpdateRate()

> Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.h:48
> +    void setPreferredFramesPerSecond(DisplayRefreshMonitorClient&, unsigned);
> +    bool scheduleAnimation(DisplayRefreshMonitorClient&, unsigned = MaxFramesPerSecond);

That unsigned arg needs to have a name. And it's odd to have both setPreferredFramesPerSecond and this argument to scheduleAnimation().

[Said Abou-Hallawa]

Created attachment 385056 [details]
Patch

[Said Abou-Hallawa]

Created attachment 385070 [details]
Patch

[Said Abou-Hallawa]

Created attachment 385114 [details]
Patch

[Said Abou-Hallawa]

Comment on attachment 384597 [details]
Patch

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

>> Source/WebCore/dom/ScriptedAnimationController.h:54
>> +    int registerCallback(Ref<RequestAnimationFrameCallback>&&);
> 
> Why did the typedef go away?

Because RequestAnimationFrameCallback::m_id is of type int and not CallbackId. But I put it back.

>> Source/WebCore/dom/ScriptedAnimationController.h:63
>> +    bool isThrottled() const { return interval() > fullSpeedAnimationInterval; }
> 
> I prefer the old version, which should use an OptionSet<> of reasons.

I put it back.

>> Source/WebCore/page/Page.h:714
>> +    unsigned preferredFramesPerSecond() const;
> 
> It's really confusing to have both of these.

The reason for having both of these functions is there is no mathematical formula that can convert from a frame interval to fps accurately. DisplayRefreshMonitor needs a fps while the timer needs an interval. So I removed preferredFramesPerSecond() from here added another function (not in Page.h) which converts an interval to fps.

>> Source/WebCore/page/RenderingUpdateScheduler.cpp:73
>> +    if (interval > 1_s || !DisplayRefreshMonitorManager::sharedManager().scheduleAnimation(*this, m_page.preferredFramesPerSecond()))
> 
> Why the 1s cutoff here?

If the interval > 1_s, the fps will be < 1 and the preferredFramesPerSecond has to be integer..

>> Source/WebCore/page/RenderingUpdateScheduler.h:50
>> +    void updateRenderingUpdateRate();
> 
> Let's call this adjustRenderingUpdateFrequency()

Done.

>> Source/WebCore/page/RenderingUpdateScheduler.h:58
>> +    void updatePreferredFramesPerSecond();
> 
> Confusing to have this and updateRenderingUpdateRate()

This function did not have an implementation. It was left in the header file by mistake.

>> Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.h:48
>> +    bool scheduleAnimation(DisplayRefreshMonitorClient&, unsigned = MaxFramesPerSecond);
> 
> That unsigned arg needs to have a name. And it's odd to have both setPreferredFramesPerSecond and this argument to scheduleAnimation().

A new type called FramesPerSecond was added. The reason for having this argument passed to scheduleAnimation() was I needed to set preferredFramesPerSecond when the DisplayRefreshMonitor is created.

I removed this argument and I made setting the initial preferredFramesPerSecond in RenderingUpdateScheduler::scheduleTimedRenderingUpdate().

[Simon Fraser (smfr)]

Comment on attachment 385114 [details]
Patch

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

> Source/WebCore/dom/ScriptedAnimationController.h:66
>      WEBCORE_EXPORT bool isThrottled() const;

Does anyone still call isThrottled()?

> Source/WebCore/page/RenderingUpdateScheduler.cpp:45
> +#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR) && PLATFORM(IOS)

PLATFORM(IOS_FAMILY)

> Source/WebCore/page/RenderingUpdateScheduler.cpp:49
> +    if (interval < 1_s)

Why is 1_s special? Doesn't DisplayRefreshMonitorManager handle any refresh rate?

> Source/WebCore/page/RenderingUpdateScheduler.cpp:56
> +#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR) && PLATFORM(IOS)

PLATFORM(IOS_FAMILY)

> Source/WebCore/page/RenderingUpdateScheduler.cpp:81
> +    if (interval < 1_s) {

Same weird cutoff.

> Source/WebCore/page/RenderingUpdateScheduler.cpp:82
> +#if PLATFORM(IOS)

PLATFORM(IOS_FAMILY)

> Source/WebCore/page/RenderingUpdateScheduler.h:57
> +#if PLATFORM(IOS)

PLATFORM(IOS_FAMILY)

> Source/WebCore/page/RenderingUpdateScheduler.h:74
> +#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR) && PLATFORM(IOS)

PLATFORM(IOS_FAMILY)

> Source/WebCore/page/RenderingUpdateScheduler.h:75
> +    std::once_flag m_createdMonitor;

These are always static.

> Source/WebCore/platform/graphics/AnimationFrameRate.h:48
> +constexpr const FramesPerSecond ZeroFramesPerSecond = 60;

This is very confusing.

> Source/WebCore/platform/graphics/AnimationFrameRate.h:50
> +constexpr const FramesPerSecond MaxFramesPerSecond = 60;

Maybe FullSpeedFramesPerSecond.

> Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.cpp:60
> -    LOG(RequestAnimationFrame, "DisplayRefreshMonitorManager::createMonitorForClient() - created monitor %p", monitor.get());
> +    LOG(RequestAnimationFrame, "DisplayRefreshMonitorManager::monitorForClient() - created monitor %p", monitor.get());

This is createMonitorForClient

[Said Abou-Hallawa]

Created attachment 385174 [details]
Patch

[Said Abou-Hallawa]

Comment on attachment 385114 [details]
Patch

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

>> Source/WebCore/dom/ScriptedAnimationController.h:66
>>      WEBCORE_EXPORT bool isThrottled() const;
> 
> Does anyone still call isThrottled()?

Yes Internals::isRequestAnimationFrameThrottled() which is used by:

LayoutTests/fast/animation/request-animation-frame-throttle-inside-overflow-scroll.html:
LayoutTests/fast/animation/request-animation-frame-throttle-subframe-display-none.html
LayoutTests/fast/animation/request-animation-frame-throttle-subframe-zero-size.html
LayoutTests/fast/animation/request-animation-frame-throttle-subframe.html
LayoutTests/fast/animation/request-animation-frame-throttling-detached-iframe.html

>> Source/WebCore/page/RenderingUpdateScheduler.cpp:45
>> +#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR) && PLATFORM(IOS)
> 
> PLATFORM(IOS_FAMILY)

Fixed.

>> Source/WebCore/page/RenderingUpdateScheduler.cpp:49
>> +    if (interval < 1_s)
> 
> Why is 1_s special? Doesn't DisplayRefreshMonitorManager handle any refresh rate?

No. CADisplayLink.preferredFramesPerSecond is an integer. So a fraction PreferredFramesPerSecond can't be set. RenderingUpdateScheduler has to fall back to use the timer in this case.

>> Source/WebCore/page/RenderingUpdateScheduler.cpp:56
>> +#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR) && PLATFORM(IOS)
> 
> PLATFORM(IOS_FAMILY)

Fixed.

>> Source/WebCore/page/RenderingUpdateScheduler.cpp:81
>> +    if (interval < 1_s) {
> 
> Same weird cutoff.

A comment is added.

>> Source/WebCore/page/RenderingUpdateScheduler.cpp:82
>> +#if PLATFORM(IOS)
> 
> PLATFORM(IOS_FAMILY)

Fixed.

>> Source/WebCore/page/RenderingUpdateScheduler.h:57
>> +#if PLATFORM(IOS)
> 
> PLATFORM(IOS_FAMILY)

Fixed.

>> Source/WebCore/page/RenderingUpdateScheduler.h:74
>> +#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR) && PLATFORM(IOS)
> 
> PLATFORM(IOS_FAMILY)

Fixed.

>> Source/WebCore/page/RenderingUpdateScheduler.h:75
>> +    std::once_flag m_createdMonitor;
> 
> These are always static.

A member bool is used instead since it has to be called once per RenderingUpdateScheduler.

>> Source/WebCore/platform/graphics/AnimationFrameRate.h:48
>> +constexpr const FramesPerSecond ZeroFramesPerSecond = 60;
> 
> This is very confusing.

Yes. This was a mistake. It has to be = 0;

>> Source/WebCore/platform/graphics/AnimationFrameRate.h:50
>> +constexpr const FramesPerSecond MaxFramesPerSecond = 60;
> 
> Maybe FullSpeedFramesPerSecond.

Fixed.

[WebKit Commit Bot]

Comment on attachment 385174 [details]
Patch

Clearing flags on attachment: 385174

Committed r253299: <https://trac.webkit.org/changeset/253299>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Truitt Savell]

The changes in https://trac.webkit.org/changeset/253299/webkit

appear to have caused 60+ imported/ test failures on Catalina Release wk2 and 35 on other Mac platforms.

build:
https://build.webkit.org/builders/Apple-Catalina-Release-WK2-Tests/builds/1365

It looks like EWS did catch this.

[Said Abou-Hallawa]

(In reply to Truitt Savell from comment #20)
> The changes in https://trac.webkit.org/changeset/253299/webkit
> 
> appear to have caused 60+ imported/ test failures on Catalina Release wk2
> and 35 on other Mac platforms.
> 
> build:
> https://build.webkit.org/builders/Apple-Catalina-Release-WK2-Tests/builds/
> 1365
> 
> It looks like EWS did catch this.

But EWS did not catch it with attachment 385114 [details]. The difference between attachment  385114 [details] and attachment 385174 [details] was just adding few comments and changing the directive PLATFORM(IOS) to PLATFORM(IOS_FAMILY).

[Truitt Savell]

Reverted r253299 for reason:

Casued 30+ imported/ test failures on Mac wk2

Committed r253308: <https://trac.webkit.org/changeset/253308>

[Said Abou-Hallawa]

Created attachment 388657 [details]
Patch

[Said Abou-Hallawa]

Created attachment 388698 [details]
Patch

[Said Abou-Hallawa]

I uploaded a pa(In reply to Said Abou-Hallawa from comment #24)
> Created attachment 388698 [details]
> Patch

This patch and the previous one are the same as r253299 but with some logging. I was expecting to see failures on EWS and from the logging I can understand the nature of the problem. But EWS did not fail. The failures that caused this patch to be rolled out was on Catalina Release wk2. But the Catalina is used only on post commit testing which means I need to land this patch with the logging and wait for the failures on Catalina.

[Said Abou-Hallawa]

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

[Said Abou-Hallawa]

Created attachment 388754 [details]
Patch

[Said Abou-Hallawa]

Created attachment 388757 [details]
Patch

[Tim Horton]

Comment on attachment 388757 [details]
Patch

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

> Tools/DumpRenderTree/mac/DumpRenderTree.mm:997
> +    [preferences setRenderingUpdateThrottlingEnabled:NO];

Do you need to turn it off for Windows DRT too? That might explain the failures.

> LayoutTests/fast/animation/request-animation-frame-throttling-outside-viewport.html:18
> +                ++ifarmeFarmesPerSecond;

ifarme, farme, etc.

[Said Abou-Hallawa]

Created attachment 388764 [details]
Patch

[Said Abou-Hallawa]

Created attachment 388783 [details]
Patch

[Said Abou-Hallawa]

Created attachment 388784 [details]
Patch

[Said Abou-Hallawa]

Created attachment 388795 [details]
Patch

[Said Abou-Hallawa]

Created attachment 388800 [details]
Patch

[Said Abou-Hallawa]

The Windows EWS seems to fail because it needs a clean build. I think the change I did in Settings.ymal did not force generating new derived sources for Settings.h and Settings.cpp.

https://bugs.webkit.org/show_bug.cgi?id=201641 was changing Settings.ymal and the Womndows EWS failed processing the batch and it gave exactly the same error.

[WebKit Commit Bot]

Comment on attachment 388800 [details]
Patch

Rejecting attachment 388800 [details] from commit-queue.

New failing tests:
editing/spelling/spellcheck-async-remove-frame.html
Full output: https://webkit-queues.webkit.org/results/13311845

[WebKit Commit Bot]

Created attachment 388809 [details]
Archive of layout-test-results from webkit-cq-03 for mac-mojave

The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: webkit-cq-03  Port: mac-mojave  Platform: Mac OS X 10.14.6

[Said Abou-Hallawa]

Created attachment 388811 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 388811 [details]
Patch

Clearing flags on attachment: 388811

Committed r255131: <https://trac.webkit.org/changeset/255131>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Truitt Savell]

It looks like the changes in https://trac.webkit.org/changeset/255131/webkit

caused testing to exit early on iOS Debug with 50+ crashes
https://build.webkit.org/builders/Apple%20iOS%2013%20Simulator%20Debug%20WK2%20%28Tests%29/builds/1874

[Ryan Haddad]

Reverted r255131 for reason:

Caused assertion failures on iOS debug bots.

Committed r255150: <https://trac.webkit.org/changeset/255150>

[Said Abou-Hallawa]

Created attachment 388871 [details]
Patch

[Said Abou-Hallawa]

(In reply to Truitt Savell from comment #41)
> It looks like the changes in https://trac.webkit.org/changeset/255131/webkit
> 
> caused testing to exit early on iOS Debug with 50+ crashes
> https://build.webkit.org/builders/
> Apple%20iOS%2013%20Simulator%20Debug%20WK2%20%28Tests%29/builds/1874

This was caused by the change I did in [WebLowPowerModeObserver initWithNotifier] where I was trying to set the initial state of the low power mode. Without this change, rAF will not be throttling if MobileSafari starts and the device is in low power mode. It will throttle only when the low power mode is turned off and then it is turned on.

Since this change broke the iOS layout tests, I am going to undo it. I am going to file a new bug for setting the initial state of the low power mode.

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 388871 [details]:

editing/spelling/spellcheck-async-remove-frame.html bug 158401 (authors: [e-mail], [e-mail], and [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 388871 [details]
Patch

Clearing flags on attachment: 388871

Committed r255158: <https://trac.webkit.org/changeset/255158>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Said Abou-Hallawa]

Reverted r255158, 255405 and r255486 because they cause test flakiness and PLT regression.

Committed r256512: <https://trac.webkit.org/changeset/256512>

[Said Abou-Hallawa]

Created attachment 398026 [details]
Patch

[Said Abou-Hallawa]

Created attachment 398097 [details]
Patch

[Said Abou-Hallawa]

(In reply to Said Abou-Hallawa from comment #50)
> Created attachment 398097 [details]
> Patch

The goal is to fix the layout tests failures and flakiness without the internal setting 'RenderingUpdateThrottling'. This will ensure the page load will not be affected by ThrottlingReason::VisuallyIdle.

In this patch I replaced isRequestAnimationFrameThrottled() by requestAnimationFrameThrottlingReasons() which returns a String with the throttling reasons.

[Said Abou-Hallawa]

Created attachment 398115 [details]
Patch

[Said Abou-Hallawa]

Created attachment 398116 [details]
Patch

[Said Abou-Hallawa]

Created attachment 398147 [details]
Patch

[Said Abou-Hallawa]

I added the following log message to Page::setIsVisuallyIdleInternal():

WTFLogAlways("In Page::setIsVisuallyIdleInternal(): isVisuallyIdle = %d, m_updateRenderingCount = %ld", isVisuallyIdle, m_updateRenderingCount);

And from the failed tests, we got the following message:

In Page::setIsVisuallyIdleInternal(): isVisuallyIdle = 1, m_updateRenderingCount = 0

[Said Abou-Hallawa]

Created attachment 398162 [details]
Patch

[Said Abou-Hallawa]

Created attachment 398167 [details]
Patch

[Said Abou-Hallawa]

Created attachment 398169 [details]
Patch

[Said Abou-Hallawa]

Created attachment 398195 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 398195 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        requestAnimationFrame is throttled by a timer although its callback are

callbacks

> Source/WebCore/ChangeLog:60
> +        is greater than the preferred rAF interval .

extra space

> Source/WebCore/page/Page.cpp:1497
> +    WTFLogAlways("In Page::setIsVisuallyIdleInternal(): isVisuallyIdle = %d", isVisuallyIdle);

Log left here.

> Source/WebCore/platform/graphics/AnimationFrameRate.h:63
> +//    if (reasons.containsAny({ ThrottlingReason::VisuallyIdle, ThrottlingReason::OutsideViewport }))
> +        return AggressiveThrottlingAnimationInterval;
> +
> +    if (reasons.containsAny({ ThrottlingReason::LowPowerMode, ThrottlingReason::NonInteractedCrossOriginFrame }))
> +        return HalfSpeedThrottlingAnimationInterval;
> +
> +//    ASSERT(reasons.isEmpty());
> +    return FullSpeedAnimationInterval;

Commented out code.

> Source/WebCore/platform/graphics/AnimationFrameRate.h:78
> +inline const char* throttlingReasonToString(ThrottlingReason reason)

Maybe do WTF::TextStream& operator<<(WTF::TextStream&, ThrottlingReason) instead, assuming throttlingReasonsToString is just used for testing.

[Said Abou-Hallawa]

Created attachment 398253 [details]
Patch

[Said Abou-Hallawa]

Created attachment 398395 [details]
Patch

[Said Abou-Hallawa]

Comment on attachment 398395 [details]
Patch

This is scoped implementation for the Page and Document throttling. Only the "Half speed throttling" is implemented by this patch. The "Aggressive throttling" will be done in following patches.

[EWS]

Committed r261113: <https://trac.webkit.org/changeset/261113>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 398395 [details].

[Michael Catanzaro]

Said, this introduced an unused variable warning:

[436/1151] Building CXX object Source/...ources/UnifiedSource-be65d27a-17.cpp.o
In file included from DerivedSources/WebCore/unified-sources/UnifiedSource-be65d27a-17.cpp:2:
/home/mcatanzaro/Projects/WebKit/Source/WebCore/dom/ScriptedAnimationController.cpp: In member function ‘WTF::Seconds WebCore::ScriptedAnimationController::preferredScriptedAnimationInterval() const’:
/home/mcatanzaro/Projects/WebKit/Source/WebCore/dom/ScriptedAnimationController.cpp:82:15: warning: unused variable ‘page’ [-Wunused-variable]
   82 |     if (auto* page = this->page())
      |               ^~~~

Right here:

Seconds ScriptedAnimationController::preferredScriptedAnimationInterval() const
{
    if (auto* page = this->page())
        return preferredFrameInterval(m_throttlingReasons);
    return FullSpeedAnimationInterval;
}

What is the desired behavior there?

[Said Abou-Hallawa]

Thanks Michael for pointing out this issue. I fixed it in <https://trac.webkit.org/changeset/261732>.

It happened because of a last minute change. This function should be like this after the fix of https://bugs.webkit.org/show_bug.cgi?id=211657:

Seconds ScriptedAnimationController::preferredScriptedAnimationInterval() const
{
    if (auto* page = this->page())
        return preferredFrameInterval(page->throttlingReasons() | m_throttlingReasons);
    return FullSpeedAnimationInterval;
}

Before submitting r261113, I took out the part "page->throttlingReasons()" but did not fix the unused variable warning.