Changeset 255338 in webkit


Ignore:
Timestamp:
Jan 28, 2020 10:53:59 PM (4 years ago)
Author:
commit-queue@webkit.org
Message:

REGRESSION (r255158): http/tests/frame-throttling/raf-throttle-in-cross-origin-subframe.html is a flaky failure
​https://bugs.webkit.org/show_bug.cgi?id=206839

Patch by Said Abou-Hallawa <Said Abou-Hallawa> on 2020-01-28
Reviewed by Simon Fraser.

Logging on EWS has shown that in some cases ThrottlingReason::VisuallyIdle
was added to the Page::m_throttlingReasons but it was never removed. If
this happens renderingUpdate and requestAnimationFrame will be throttled
to 10 seconds which leads to test flakiness and timeout.

Currently the throttling reasons of Page or ScriptedAnimationController
can be changed by natural reasons (visibility, layout or device state)
unless Setting::renderingUpdateThrottlingEnabled is disabled. Low power
mode also can be overridden by a specific value set by the test.

To make things clearer, a new OptionSet<ThrottlingReason> will be added
to Page. Its name is m_throttlingReasonsOverrideForTestingMask. The
purpose of adding it is to allow disabling and overriding specific
ThrottlingReasons. The bits of m_throttlingReasonsOverrideForTestingMask
are not actually reasons. They prevent changing the corresponding bits in
Page::m_throttlingReasons by natural reasons. Here are the rules for
setting the bits of m_throttlingReasonsOverrideForTestingMask and
m_throttlingReasons:

-- Settings::renderingUpdateThrottlingEnabled is enabled: All the bits in

m_throttlingReasonsOverrideForTestingMask will be turned off expect
the bit of ThrottlingReason::VisuallyIdle. We need to disable it always
for testing. All the bits in m_throttlingReasons will be cleared.
ThrottlingReason::LowPowerMode will be added to m_throttlingReasons
if the device is already in low power mode.

-- Settings::renderingUpdateThrottlingEnabled is disabled: All the bits

in m_throttlingReasonsOverrideForTestingMask will be turned on. All the
bits in m_throttlingReasons will be cleared.

-- Low power mode is overridden: ThrottlingReason::LowPowerMode is added

to m_throttlingReasonsOverrideForTestingMask. The new overriding value
is set in m_throttlingReasons.

-- Low power mode is cleared: ThrottlingReason::LowPowerMode is removed

from m_throttlingReasonsOverrideForTestingMask. If the device is in
low power mode. ThrottlingReason::LowPowerMode will be added to
m_throttlingReasons

  • dom/ScriptedAnimationController.cpp:

(WebCore::ScriptedAnimationController::addThrottlingReason):
(WebCore::ScriptedAnimationController::removeThrottlingReason):
Adding and removing ThrottlingReasons to ScriptedAnimationController will
be controlled by Page::m_throttlingReasonsOverrideForTestingMask. If the
bits of the corresponding reasons are on, no change will be allowed.

(WebCore::ScriptedAnimationController::clearThrottlingReasons):
(WebCore::ScriptedAnimationController::isThrottled const):
The bits in the m_throttlingReasons of Page and ScriptedAnimationController
reflect the state of the throttling. No need to check for the Settings.

(WebCore::ScriptedAnimationController::preferredScriptedAnimationInterval const): Deleted.

  • dom/ScriptedAnimationController.h:

(WebCore::ScriptedAnimationController::preferredScriptedAnimationInterval const):
(WebCore::ScriptedAnimationController::addThrottlingReason): Deleted.
(WebCore::ScriptedAnimationController::removeThrottlingReason): Deleted.

  • page/Page.cpp:

(WebCore::Page::Page):
Set the initial state of the low power mode throttling.

(WebCore::Page::setLowPowerModeEnabledOverrideForTesting):
This will override the low power mode state or clear it. If it overrides
it, no subsequent change will be allowed.

(WebCore::Page::renderingUpdateThrottlingEnabledChangedForTesting):
This called through changing the Settings from the tests only.

(WebCore::Page::setIsVisuallyIdleInternal):
(WebCore::Page::handleLowModePowerChange):
Prevent changing m_throttlingReasons if the throttling reasons can't be
altered.

(WebCore::Page::isLowPowerModeEnabled const): Deleted.
(WebCore::Page::renderingUpdateThrottlingEnabled const): Deleted.
(WebCore::Page::renderingUpdateThrottlingEnabledChanged): Deleted.
(WebCore::Page::isRenderingUpdateThrottled const): Deleted.
(WebCore::Page::preferredRenderingUpdateInterval const): Deleted.

  • page/Page.h:

(WebCore::Page::isLowPowerModeEnabled const):
(WebCore::Page::canUpdateThrottlingReason const):
(WebCore::Page::isRenderingUpdateThrottled const):
(WebCore::Page::preferredRenderingUpdateInterval const):

  • page/Settings.yaml:
  • page/SettingsBase.cpp:

(WebCore::SettingsBase::renderingUpdateThrottlingEnabledChangedForTesting):
(WebCore::SettingsBase::renderingUpdateThrottlingEnabledChanged): Deleted.

  • page/SettingsBase.h:
Location:
trunk/Source/WebCore
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r255335 r255338  
     12020-01-28  Said Abou-Hallawa  <said@apple.com>
     2
     3        REGRESSION (r255158): http/tests/frame-throttling/raf-throttle-in-cross-origin-subframe.html is a flaky failure
     4        https://bugs.webkit.org/show_bug.cgi?id=206839
     5
     6        Reviewed by Simon Fraser.
     7
     8        Logging on EWS has shown that in some cases ThrottlingReason::VisuallyIdle
     9        was added to the Page::m_throttlingReasons but it was never removed. If
     10        this happens renderingUpdate and requestAnimationFrame will be throttled
     11        to 10 seconds which leads to test flakiness and timeout.
     12
     13        Currently the throttling reasons of Page or ScriptedAnimationController
     14        can be changed by natural reasons (visibility, layout or device state)
     15        unless Setting::renderingUpdateThrottlingEnabled is disabled. Low power
     16        mode also can be overridden by a specific value set by the test.
     17
     18        To make things clearer, a new OptionSet<ThrottlingReason> will be added
     19        to Page. Its name is m_throttlingReasonsOverrideForTestingMask. The
     20        purpose of adding it is to allow disabling and overriding specific
     21        ThrottlingReasons. The bits of m_throttlingReasonsOverrideForTestingMask
     22        are not actually reasons. They prevent changing the corresponding bits in
     23        Page::m_throttlingReasons by natural reasons. Here are the rules for
     24        setting the bits of m_throttlingReasonsOverrideForTestingMask and
     25        m_throttlingReasons:
     26
     27        -- Settings::renderingUpdateThrottlingEnabled is enabled: All the bits in
     28           m_throttlingReasonsOverrideForTestingMask will be turned off expect
     29           the bit of ThrottlingReason::VisuallyIdle. We need to disable it always
     30           for testing. All the bits in m_throttlingReasons will be cleared.
     31           ThrottlingReason::LowPowerMode will be added to m_throttlingReasons
     32           if the device is already in low power mode.
     33
     34        -- Settings::renderingUpdateThrottlingEnabled is disabled: All the bits
     35           in m_throttlingReasonsOverrideForTestingMask will be turned on. All the
     36           bits in m_throttlingReasons will be cleared.
     37
     38        -- Low power mode is overridden: ThrottlingReason::LowPowerMode is added
     39           to m_throttlingReasonsOverrideForTestingMask. The new overriding value
     40           is set in m_throttlingReasons.
     41
     42        -- Low power mode is cleared: ThrottlingReason::LowPowerMode is removed
     43           from m_throttlingReasonsOverrideForTestingMask. If the device is in
     44           low power mode. ThrottlingReason::LowPowerMode will be added to
     45           m_throttlingReasons
     46
     47        * dom/ScriptedAnimationController.cpp:
     48        (WebCore::ScriptedAnimationController::addThrottlingReason):
     49        (WebCore::ScriptedAnimationController::removeThrottlingReason):
     50        Adding and removing ThrottlingReasons to ScriptedAnimationController will
     51        be controlled by Page::m_throttlingReasonsOverrideForTestingMask. If the
     52        bits of the corresponding reasons are on, no change will be allowed.
     53
     54        (WebCore::ScriptedAnimationController::clearThrottlingReasons):
     55        (WebCore::ScriptedAnimationController::isThrottled const):
     56        The bits in the m_throttlingReasons of Page and ScriptedAnimationController
     57        reflect the state of the throttling. No need to check for the Settings.
     58
     59        (WebCore::ScriptedAnimationController::preferredScriptedAnimationInterval const): Deleted.
     60        * dom/ScriptedAnimationController.h:
     61        (WebCore::ScriptedAnimationController::preferredScriptedAnimationInterval const):
     62        (WebCore::ScriptedAnimationController::addThrottlingReason): Deleted.
     63        (WebCore::ScriptedAnimationController::removeThrottlingReason): Deleted.
     64
     65        * page/Page.cpp:
     66        (WebCore::Page::Page):
     67        Set the initial state of the low power mode throttling.
     68
     69        (WebCore::Page::setLowPowerModeEnabledOverrideForTesting):
     70        This will override the low power mode state or clear it. If it overrides
     71        it, no subsequent change will be allowed.
     72
     73        (WebCore::Page::renderingUpdateThrottlingEnabledChangedForTesting):
     74        This called through changing the Settings from the tests only.
     75
     76        (WebCore::Page::setIsVisuallyIdleInternal):
     77        (WebCore::Page::handleLowModePowerChange):
     78        Prevent changing m_throttlingReasons if the throttling reasons can't be
     79        altered.
     80
     81        (WebCore::Page::isLowPowerModeEnabled const): Deleted.
     82        (WebCore::Page::renderingUpdateThrottlingEnabled const): Deleted.
     83        (WebCore::Page::renderingUpdateThrottlingEnabledChanged): Deleted.
     84        (WebCore::Page::isRenderingUpdateThrottled const): Deleted.
     85        (WebCore::Page::preferredRenderingUpdateInterval const): Deleted.
     86        * page/Page.h:
     87        (WebCore::Page::isLowPowerModeEnabled const):
     88        (WebCore::Page::canUpdateThrottlingReason const):
     89        (WebCore::Page::isRenderingUpdateThrottled const):
     90        (WebCore::Page::preferredRenderingUpdateInterval const):
     91        * page/Settings.yaml:
     92        * page/SettingsBase.cpp:
     93        (WebCore::SettingsBase::renderingUpdateThrottlingEnabledChangedForTesting):
     94        (WebCore::SettingsBase::renderingUpdateThrottlingEnabledChanged): Deleted.
     95        * page/SettingsBase.h:
     96
    1972020-01-28  Simon Fraser  <simon.fraser@apple.com>
    298

  • trunk/Source/WebCore/dom/ScriptedAnimationController.cpp

    r255158 r255338  
    5353}
    5454
    55 Seconds ScriptedAnimationController::preferredScriptedAnimationInterval() const
    56 {
    57     if (auto* page = this->page())
    58         return page->renderingUpdateThrottlingEnabled() ? preferredFrameInterval(m_throttlingReasons) : FullSpeedAnimationInterval;
    59     return FullSpeedAnimationInterval;
    60 }
    61 
    6255Seconds ScriptedAnimationController::interval() const
    6356{
    … …  
    8376}
    8477
     78void ScriptedAnimationController::addThrottlingReason(ThrottlingReason reason)
     79{
     80    if (auto* page = this->page()) {
     81        if (!page->canUpdateThrottlingReason(reason))
     82            return;
     83    }
     84    m_throttlingReasons.add(reason);
     85}
     86
     87void ScriptedAnimationController::removeThrottlingReason(ThrottlingReason reason)
     88{
     89    if (auto* page = this->page()) {
     90        if (!page->canUpdateThrottlingReason(reason))
     91            return;
     92    }
     93    m_throttlingReasons.remove(reason);
     94}
     95
     96void ScriptedAnimationController::clearThrottlingReasons()
     97{
     98    m_throttlingReasons = { };
     99}
     100
    85101bool ScriptedAnimationController::isThrottled() const
    86102{
    87     if (auto* page = this->page())
    88         return page->renderingUpdateThrottlingEnabled() && (page->isRenderingUpdateThrottled() || !m_throttlingReasons.isEmpty());
     103    if (!m_throttlingReasons.isEmpty())
     104        return true;
     105    if (auto* page = this->page())
     106        return page->isRenderingUpdateThrottled();
    89107    return false;
    90108}

  • trunk/Source/WebCore/dom/ScriptedAnimationController.h

    r255158 r255338  
    6262    void resume();
    6363   
    64     void addThrottlingReason(ThrottlingReason reason) { m_throttlingReasons.add(reason); }
    65     void removeThrottlingReason(ThrottlingReason reason) { m_throttlingReasons.remove(reason); }
     64    void addThrottlingReason(ThrottlingReason);
     65    void removeThrottlingReason(ThrottlingReason);
     66    void clearThrottlingReasons();
    6667    WEBCORE_EXPORT bool isThrottled() const;
    6768
    … …  
    7071
    7172    Page* page() const;
    72     Seconds preferredScriptedAnimationInterval() const;
     73    Seconds preferredScriptedAnimationInterval() const { return preferredFrameInterval(m_throttlingReasons); }
    7374    bool isThrottledRelativeToPage() const;
    7475    bool shouldRescheduleRequestAnimationFrame(DOMHighResTimeStamp) const;

  • trunk/Source/WebCore/page/Page.cpp

    r255226 r255338  
    336336    }
    337337    m_corsDisablingPatterns.shrinkToFit();
     338
     339    if (m_lowPowerModeNotifier->isLowPowerModeEnabled())
     340        m_throttlingReasons.add(ThrottlingReason::LowPowerMode);
    338341}
    339342
    … …  
    11611164}
    11621165
    1163 bool Page::isLowPowerModeEnabled() const
    1164 {
    1165     if (m_lowPowerModeEnabledOverrideForTesting)
    1166         return m_lowPowerModeEnabledOverrideForTesting.value();
    1167 
    1168     return m_lowPowerModeNotifier->isLowPowerModeEnabled();
    1169 }
    1170 
    11711166void Page::setLowPowerModeEnabledOverrideForTesting(Optional<bool> isEnabled)
    11721167{
    1173     m_lowPowerModeEnabledOverrideForTesting = isEnabled;
    1174     handleLowModePowerChange(m_lowPowerModeEnabledOverrideForTesting.valueOr(false));
     1168    // Remove ThrottlingReason::LowPowerMode so handleLowModePowerChange() can do its work.
     1169    m_throttlingReasonsOverridenForTesting.remove(ThrottlingReason::LowPowerMode);
     1170
     1171    if (isEnabled) {
     1172        handleLowModePowerChange(isEnabled.value());
     1173        m_throttlingReasonsOverridenForTesting.add(ThrottlingReason::LowPowerMode);
     1174    } else
     1175        handleLowModePowerChange(m_lowPowerModeNotifier->isLowPowerModeEnabled());
    11751176}
    11761177
    … …  
    13781379}
    13791380
    1380 bool Page::renderingUpdateThrottlingEnabled() const
    1381 {
    1382     return m_settings->renderingUpdateThrottlingEnabled();
    1383 }
    1384 
    1385 void Page::renderingUpdateThrottlingEnabledChanged()
    1386 {
     1381void Page::renderingUpdateThrottlingEnabledChangedForTesting()
     1382{
     1383    m_throttlingReasons = { };
     1384
     1385    // This function can only be called through changing the Settings from the layout tests.
     1386    // So disable ThrottlingReason::VisuallyIdle always.
     1387    m_throttlingReasonsOverridenForTesting = ThrottlingReason::VisuallyIdle;
     1388
     1389    if (m_settings->renderingUpdateThrottlingEnabled()) {
     1390        if (m_lowPowerModeNotifier->isLowPowerModeEnabled())
     1391            m_throttlingReasons.add(ThrottlingReason::LowPowerMode);
     1392    } else {
     1393        m_throttlingReasonsOverridenForTesting.add({ ThrottlingReason::OutsideViewport, ThrottlingReason::LowPowerMode, ThrottlingReason::NonInteractedCrossOriginFrame });
     1394
     1395        forEachDocument([] (Document& document) {
     1396            if (auto* scriptedAnimationController = document.scriptedAnimationController())
     1397                scriptedAnimationController->clearThrottlingReasons();
     1398        });
     1399    }
     1400
    13871401    renderingUpdateScheduler().adjustRenderingUpdateFrequency();
    13881402}
    13891403
    1390 bool Page::isRenderingUpdateThrottled() const
    1391 {
    1392     return renderingUpdateThrottlingEnabled() && !m_throttlingReasons.isEmpty();
    1393 }
    1394 
    1395 Seconds Page::preferredRenderingUpdateInterval() const
    1396 {
    1397     return renderingUpdateThrottlingEnabled() ? preferredFrameInterval(m_throttlingReasons) : FullSpeedAnimationInterval;
    1398 }
    1399 
    14001404void Page::setIsVisuallyIdleInternal(bool isVisuallyIdle)
    14011405{
     1406    if (!canUpdateThrottlingReason(ThrottlingReason::VisuallyIdle))
     1407        return;
     1408
    14021409    if (isVisuallyIdle == m_throttlingReasons.contains(ThrottlingReason::VisuallyIdle))
    14031410        return;
    14041411
    14051412    m_throttlingReasons = m_throttlingReasons ^ ThrottlingReason::VisuallyIdle;
    1406 
    1407     if (renderingUpdateThrottlingEnabled())
    1408         renderingUpdateScheduler().adjustRenderingUpdateFrequency();
     1413    renderingUpdateScheduler().adjustRenderingUpdateFrequency();
    14091414}
    14101415
    14111416void Page::handleLowModePowerChange(bool isLowPowerModeEnabled)
    14121417{
     1418    if (!canUpdateThrottlingReason(ThrottlingReason::LowPowerMode))
     1419        return;
     1420
    14131421    if (isLowPowerModeEnabled == m_throttlingReasons.contains(ThrottlingReason::LowPowerMode))
    14141422        return;
    14151423
    14161424    m_throttlingReasons = m_throttlingReasons ^ ThrottlingReason::LowPowerMode;
    1417 
    1418     if (renderingUpdateThrottlingEnabled())
    1419         renderingUpdateScheduler().adjustRenderingUpdateFrequency();
     1425    renderingUpdateScheduler().adjustRenderingUpdateFrequency();
    14201426
    14211427    if (!RuntimeEnabledFeatures::sharedFeatures().webAnimationsCSSIntegrationEnabled())

  • trunk/Source/WebCore/page/Page.h

    r255226 r255338  
    699699    bool isUtilityPage() const { return m_isUtilityPage; }
    700700
    701     bool isLowPowerModeEnabled() const;
     701    bool isLowPowerModeEnabled() const { return m_throttlingReasons.contains(ThrottlingReason::LowPowerMode); }
    702702    WEBCORE_EXPORT void setLowPowerModeEnabledOverrideForTesting(Optional<bool>);
    703703
    704     bool renderingUpdateThrottlingEnabled() const;
    705     void renderingUpdateThrottlingEnabledChanged();
    706     bool isRenderingUpdateThrottled() const;
    707     Seconds preferredRenderingUpdateInterval() const;
     704    void renderingUpdateThrottlingEnabledChangedForTesting();
     705    bool canUpdateThrottlingReason(ThrottlingReason reason) const { return !m_throttlingReasonsOverridenForTesting.contains(reason); }
     706    bool isRenderingUpdateThrottled() const { return !m_throttlingReasons.isEmpty(); }
     707    Seconds preferredRenderingUpdateInterval() const { return preferredFrameInterval(m_throttlingReasons); }
    708708
    709709    WEBCORE_EXPORT void applicationWillResignActive();
    … …  
    970970    std::unique_ptr<PerformanceMonitor> m_performanceMonitor;
    971971    std::unique_ptr<LowPowerModeNotifier> m_lowPowerModeNotifier;
    972     Optional<bool> m_lowPowerModeEnabledOverrideForTesting;
    973972
    974973    Optional<Navigation> m_navigationToLogWhenVisible;
    … …  
    10051004    Vector<UserContentURLPattern> m_corsDisablingPatterns;
    10061005    OptionSet<ThrottlingReason> m_throttlingReasons;
     1006    OptionSet<ThrottlingReason> m_throttlingReasonsOverridenForTesting;
    10071007};
    10081008

  • trunk/Source/WebCore/page/Settings.yaml

    r255158 r255338  
    768768renderingUpdateThrottlingEnabled:
    769769  initial: true
    770   onChange: renderingUpdateThrottlingEnabledChanged
     770  onChange: renderingUpdateThrottlingEnabledChangedForTesting
    771771
    772772storageBlockingPolicy:

  • trunk/Source/WebCore/page/SettingsBase.cpp

    r255158 r255338  
    408408}
    409409
    410 void SettingsBase::renderingUpdateThrottlingEnabledChanged()
    411 {
    412     if (m_page)
    413         m_page->renderingUpdateThrottlingEnabledChanged();
     410void SettingsBase::renderingUpdateThrottlingEnabledChangedForTesting()
     411{
     412    if (m_page)
     413        m_page->renderingUpdateThrottlingEnabledChangedForTesting();
    414414}
    415415

  • trunk/Source/WebCore/page/SettingsBase.h

    r255158 r255338  
    195195    void hiddenPageDOMTimerThrottlingStateChanged();
    196196    void hiddenPageCSSAnimationSuspensionEnabledChanged();
    197     void renderingUpdateThrottlingEnabledChanged();
     197    void renderingUpdateThrottlingEnabledChangedForTesting();
    198198    void resourceUsageOverlayVisibleChanged();
    199199    void iceCandidateFilteringEnabledChanged();
Note: See TracChangeset for help on using the changeset viewer.