[Webkit-unassigned] [Bug 206839] REGRESSION (r255158): http/tests/frame-throttling/raf-throttle-in-cross-origin-subframe.html is a flaky failure

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=206839

--- Comment #3 from Said Abou-Hallawa <sabouhallawa at apple.com> ---
Looking at the diff above, I can see that rAF of each of sub-frames is throttled regardless whether it is cross origin frame or not or whether it is interacted or not.

The throttling reasons are the following:

1. VisuallyIdle
2. OutsideViewport
3. LowPowerMode
4. NonInteractedCrossOriginFrame

The OutsideViewport and LowPowerMode should not be involved in this test. The page can show the two sub-frames. And the low power mode has to be turned on explicitly from the test which is not happening.

NonInteractedCrossOriginFrame should be on for the "cross" frame at the beginning of the test. Then it should be turned off when UIHelper activates it at the end of the test. But it should be off for the "same" frame all the times.

This leaves VisuallyIdle as the cause of the throttling. And this can happen only if the page activity state is set to VisuallyIdle. But it is never reset back.

So we can fix this issue by forcing the visibility of the page at the beginning of the test by calling Internals::setPageVisibility(). But looking at the code I can see the following:

void Internals::setPageVisibility(bool isVisible)
{
    auto* document = contextDocument();
    if (!document || !document->page())
        return;
    auto& page = *document->page();
    auto state = page.activityState();

    if (!isVisible)
        state.remove(ActivityState::IsVisible);
    else
        state.add(ActivityState::IsVisible);

    page.setActivityState(state);
}

void Page::setIsVisible(bool isVisible)
{
    auto state = m_activityState;

    if (isVisible) {
        state.remove(ActivityState::IsVisuallyIdle);
        state.add({ ActivityState::IsVisible, ActivityState::IsVisibleOrOccluded });
    } else {
        state.add(ActivityState::IsVisuallyIdle);
        state.remove({ ActivityState::IsVisible, ActivityState::IsVisibleOrOccluded });
    }
    setActivityState(state);
}

So the Internals API does not take ActivityState::IsVisuallyIdle into consideration when the visibility state changes. But the Page API does take it into consideration which I think is correct. 

I think Page::setIsVisible() should be called from Internals::setPageVisibility() to remove this discrepancy and the test itself should call Internals::setPageVisibility() to fix this issue.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20200128/2c697ada/attachment.htm>