Changeset 249147 in webkit


Ignore:
Timestamp:
Aug 27, 2019 9:48:21 AM (5 years ago)
Author:
jer.noble@apple.com
Message:

Removing fullscreen element in rAF() callback after requestFullscreen() can leave fullscreen in inconsistent state.
​https://bugs.webkit.org/show_bug.cgi?id=201101
<rdar://problem/54164587>

Reviewed by Eric Carlson.

Source/WebCore:

Test: fullscreen/full-screen-request-removed-with-raf.html

Add a new state variable, m_pendingFullscreenElement, to track which element is about to
become the fullscreen element, so that when elements are removed or cancelFullscreen() is
called, the state machine inside the fullscreen algorithm can cancel effectively.

  • dom/FullscreenManager.cpp:

(WebCore::FullscreenManager::requestFullscreenForElement):
(WebCore::FullscreenManager::cancelFullscreen):
(WebCore::FullscreenManager::exitFullscreen):
(WebCore::FullscreenManager::willEnterFullscreen):
(WebCore::FullscreenManager::willExitFullscreen):
(WebCore::FullscreenManager::didExitFullscreen):
(WebCore::FullscreenManager::adjustFullscreenElementOnNodeRemoval):
(WebCore::FullscreenManager::clear):
(WebCore::FullscreenManager::fullscreenElementRemoved): Deleted.

  • dom/FullscreenManager.h:

Source/WebKit:

Add more state to track in which direction the animation is flowing to allow in-process
animations to be cancelled more gracefully.

  • UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.mm:

(-[WKFullScreenWindowController enterFullScreen]):
(-[WKFullScreenWindowController beganEnterFullScreenWithInitialFrame:finalFrame:]):
(-[WKFullScreenWindowController requestExitFullScreen]):
(-[WKFullScreenWindowController exitFullScreen]):

  • WebProcess/cocoa/VideoFullscreenManager.h:

(WebKit::VideoFullscreenInterfaceContext::animationState const):
(WebKit::VideoFullscreenInterfaceContext::setAnimationState):
(WebKit::VideoFullscreenInterfaceContext::isAnimating const): Deleted.
(WebKit::VideoFullscreenInterfaceContext::setIsAnimating): Deleted.

  • WebProcess/cocoa/VideoFullscreenManager.mm:

(WebKit::VideoFullscreenManager::enterVideoFullscreenForVideoElement):
(WebKit::VideoFullscreenManager::exitVideoFullscreenForVideoElement):
(WebKit::VideoFullscreenManager::didEnterFullscreen):
(WebKit::VideoFullscreenManager::didCleanupFullscreen):

LayoutTests:

  • fullscreen/full-screen-request-removed-with-raf-expected.txt: Added.
  • fullscreen/full-screen-request-removed-with-raf.html: Added.
Location:
trunk
Files:
2 added
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r249141 r249147  
     12019-08-26  Jer Noble  <jer.noble@apple.com>
     2
     3        Removing fullscreen element in rAF() callback after requestFullscreen() can leave fullscreen in inconsistent state.
     4        https://bugs.webkit.org/show_bug.cgi?id=201101
     5        <rdar://problem/54164587>
     6
     7        Reviewed by Eric Carlson.
     8
     9        * fullscreen/full-screen-request-removed-with-raf-expected.txt: Added.
     10        * fullscreen/full-screen-request-removed-with-raf.html: Added.
     11
    1122019-08-27  Peng Liu  <peng.liu6@apple.com>
    213

  • trunk/Source/WebCore/ChangeLog

    r249141 r249147  
     12019-08-26  Jer Noble  <jer.noble@apple.com>
     2
     3        Removing fullscreen element in rAF() callback after requestFullscreen() can leave fullscreen in inconsistent state.
     4        https://bugs.webkit.org/show_bug.cgi?id=201101
     5        <rdar://problem/54164587>
     6
     7        Reviewed by Eric Carlson.
     8
     9        Test: fullscreen/full-screen-request-removed-with-raf.html
     10
     11        Add a new state variable, m_pendingFullscreenElement, to track which element is about to
     12        become the fullscreen element, so that when elements are removed or cancelFullscreen() is
     13        called, the state machine inside the fullscreen algorithm can cancel effectively.
     14
     15        * dom/FullscreenManager.cpp:
     16        (WebCore::FullscreenManager::requestFullscreenForElement):
     17        (WebCore::FullscreenManager::cancelFullscreen):
     18        (WebCore::FullscreenManager::exitFullscreen):
     19        (WebCore::FullscreenManager::willEnterFullscreen):
     20        (WebCore::FullscreenManager::willExitFullscreen):
     21        (WebCore::FullscreenManager::didExitFullscreen):
     22        (WebCore::FullscreenManager::adjustFullscreenElementOnNodeRemoval):
     23        (WebCore::FullscreenManager::clear):
     24        (WebCore::FullscreenManager::fullscreenElementRemoved): Deleted.
     25        * dom/FullscreenManager.h:
     26
    1272019-08-27  Peng Liu  <peng.liu6@apple.com>
    228

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

    r246490 r249147  
    121121    }
    122122
     123    m_pendingFullscreenElement = element;
     124
    123125    m_fullscreenTaskQueue.enqueueTask([this, element = makeRefPtr(element), checkType, hasKeyboardAccess, failedPreflights] () mutable {
     126        // Don't allow fullscreen if it has been cancelled or a different fullscreen element
     127        // has requested fullscreen.
     128        if (m_pendingFullscreenElement != element) {
     129            failedPreflights(WTFMove(element));
     130            return;
     131        }
     132
    124133        // Don't allow fullscreen if document is hidden.
    125134        if (document().hidden()) {
    … …  
    210219        // 6. Optionally, perform some animation.
    211220        m_areKeysEnabledInFullscreen = hasKeyboardAccess;
    212         m_fullscreenTaskQueue.enqueueTask([this, element = WTFMove(element)] {
    213             if (auto page = this->page())
    214                 page->chrome().client().enterFullScreenForElement(*element.get());
     221        m_fullscreenTaskQueue.enqueueTask([this, element = WTFMove(element), failedPreflights = WTFMove(failedPreflights)] () mutable {
     222            auto page = this->page();
     223            if (!page || document().hidden() || m_pendingFullscreenElement != element || !element->isConnected()) {
     224                failedPreflights(element);
     225                return;
     226            }
     227            page->chrome().client().enterFullScreenForElement(*element.get());
    215228        });
    216229
    … …  
    226239    // context's document and subsequently empty that document's fullscreen element stack."
    227240    Document& topDocument = document().topDocument();
    228     if (!topDocument.fullscreenManager().fullscreenElement())
    229         return;
     241    if (!topDocument.fullscreenManager().fullscreenElement()) {
     242        // If there is a pending fullscreen element but no top document fullscreen element,
     243        // there is a pending task in enterFullscreen(). Cause it to cancel and fire an error
     244        // by clearing the pending fullscreen element.
     245        m_pendingFullscreenElement = nullptr;
     246        return;
     247    }
    230248
    231249    // To achieve that aim, remove all the elements from the top document's stack except for the first before
    … …  
    246264
    247265    // 2. If doc's fullscreen element stack is empty, terminate these steps.
    248     if (m_fullscreenElementStack.isEmpty())
    249         return;
     266    if (m_fullscreenElementStack.isEmpty()) {
     267        // If there is a pending fullscreen element but an empty fullscreen element stack,
     268        // there is a pending task in requestFullscreenForElement(). Cause it to cancel and fire an error
     269        // by clearing the pending fullscreen element.
     270        m_pendingFullscreenElement = nullptr;
     271        return;
     272    }
    250273
    251274    // 3. Let descendants be all the doc's descendant browsing context's documents with a non-empty fullscreen
    … …  
    299322            return;
    300323
     324        // If there is a pending fullscreen element but no fullscreen element
     325        // there is a pending task in requestFullscreenForElement(). Cause it to cancel and fire an error
     326        // by clearing the pending fullscreen element.
     327        if (!fullscreenElement && m_pendingFullscreenElement) {
     328            m_pendingFullscreenElement = nullptr;
     329            return;
     330        }
     331
    301332        // Only exit out of full screen window mode if there are no remaining elements in the
    302333        // full screen stack.
    … …  
    340371        return;
    341372
     373    // If pending fullscreen element is unset or another element's was requested,
     374    // issue a cancel fullscreen request to the client
     375    if (m_pendingFullscreenElement != &element) {
     376        page()->chrome().client().exitFullScreenForElement(&element);
     377        return;
     378    }
     379
    342380    ASSERT(page()->settings().fullScreenEnabled());
    343381
    … …  
    346384    element.willBecomeFullscreenElement();
    347385
     386    ASSERT(&element == m_pendingFullscreenElement);
     387    m_pendingFullscreenElement = nullptr;
    348388    m_fullscreenElement = &element;
    349389
    … …  
    386426void FullscreenManager::willExitFullscreen()
    387427{
    388     if (!m_fullscreenElement)
     428    auto fullscreenElement = fullscreenOrPendingElement();
     429    if (!fullscreenElement)
    389430        return;
    390431
    … …  
    392433        return;
    393434
    394     m_fullscreenElement->willStopBeingFullscreenElement();
     435    fullscreenElement->willStopBeingFullscreenElement();
    395436}
    396437
    397438void FullscreenManager::didExitFullscreen()
    398439{
    399     if (!m_fullscreenElement)
     440    auto fullscreenElement = fullscreenOrPendingElement();
     441    if (!fullscreenElement)
    400442        return;
    401443
    402444    if (!hasLivingRenderTree() || pageCacheState() != Document::NotInPageCache)
    403445        return;
    404 
    405     m_fullscreenElement->setContainsFullScreenElementOnAncestorsCrossingFrameBoundaries(false);
     446    fullscreenElement->setContainsFullScreenElementOnAncestorsCrossingFrameBoundaries(false);
    406447
    407448    m_areKeysEnabledInFullscreen = false;
    … …  
    410451
    411452    m_fullscreenElement = nullptr;
     453    m_pendingFullscreenElement = nullptr;
    412454    scheduleFullStyleRebuild();
    413455
    … …  
    484526}
    485527
    486 void FullscreenManager::fullscreenElementRemoved()
    487 {
    488     m_fullscreenElement->setContainsFullScreenElementOnAncestorsCrossingFrameBoundaries(false);
    489     cancelFullscreen();
    490 }
    491 
    492528void FullscreenManager::adjustFullscreenElementOnNodeRemoval(Node& node, Document::NodeRemoval nodeRemoval)
    493529{
    494     if (!m_fullscreenElement)
     530    auto fullscreenElement = fullscreenOrPendingElement();
     531    if (!fullscreenElement)
    495532        return;
    496533
    497534    bool elementInSubtree = false;
    498535    if (nodeRemoval == Document::NodeRemoval::ChildrenOfNode)
    499         elementInSubtree = m_fullscreenElement->isDescendantOf(node);
     536        elementInSubtree = fullscreenElement->isDescendantOf(node);
    500537    else
    501         elementInSubtree = (m_fullscreenElement == &node) || m_fullscreenElement->isDescendantOf(node);
    502 
    503     if (elementInSubtree)
    504         fullscreenElementRemoved();
     538        elementInSubtree = (fullscreenElement == &node) || fullscreenElement->isDescendantOf(node);
     539
     540    if (elementInSubtree) {
     541        fullscreenElement->setContainsFullScreenElementOnAncestorsCrossingFrameBoundaries(false);
     542        cancelFullscreen();
     543    }
    505544}
    506545
    … …  
    542581{
    543582    m_fullscreenElement = nullptr;
     583    m_pendingFullscreenElement = nullptr;
    544584    m_fullscreenElementStack.clear();
    545585}

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

    r248762 r249147  
    110110    Document& m_document;
    111111
     112    RefPtr<Element> fullscreenOrPendingElement() const { return m_fullscreenElement ? m_fullscreenElement : m_pendingFullscreenElement; }
     113
     114    RefPtr<Element> m_pendingFullscreenElement;
    112115    RefPtr<Element> m_fullscreenElement;
    113116    Vector<RefPtr<Element>> m_fullscreenElementStack;

  • trunk/Source/WebKit/ChangeLog

    r249142 r249147  
     12019-08-26  Jer Noble  <jer.noble@apple.com>
     2
     3        Removing fullscreen element in rAF() callback after requestFullscreen() can leave fullscreen in inconsistent state.
     4        https://bugs.webkit.org/show_bug.cgi?id=201101
     5        <rdar://problem/54164587>
     6
     7        Reviewed by Eric Carlson.
     8
     9        Add more state to track in which direction the animation is flowing to allow in-process
     10        animations to be cancelled more gracefully.
     11
     12        * UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.mm:
     13        (-[WKFullScreenWindowController enterFullScreen]):
     14        (-[WKFullScreenWindowController beganEnterFullScreenWithInitialFrame:finalFrame:]):
     15        (-[WKFullScreenWindowController requestExitFullScreen]):
     16        (-[WKFullScreenWindowController exitFullScreen]):
     17        * WebProcess/cocoa/VideoFullscreenManager.h:
     18        (WebKit::VideoFullscreenInterfaceContext::animationState const):
     19        (WebKit::VideoFullscreenInterfaceContext::setAnimationState):
     20        (WebKit::VideoFullscreenInterfaceContext::isAnimating const): Deleted.
     21        (WebKit::VideoFullscreenInterfaceContext::setIsAnimating): Deleted.
     22        * WebProcess/cocoa/VideoFullscreenManager.mm:
     23        (WebKit::VideoFullscreenManager::enterVideoFullscreenForVideoElement):
     24        (WebKit::VideoFullscreenManager::exitVideoFullscreenForVideoElement):
     25        (WebKit::VideoFullscreenManager::didEnterFullscreen):
     26        (WebKit::VideoFullscreenManager::didCleanupFullscreen):
     27
    1282019-08-27  Carlos Garcia Campos  <cgarcia@igalia.com>
    229

  • trunk/Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.mm

    r247018 r249147  
    448448    BOOL _EVOrganizationNameIsValid;
    449449    BOOL _inInteractiveDismiss;
     450    BOOL _exitRequested;
    450451
    451452    RetainPtr<id> _notificationListener;
    … …  
    598599        _repaintCallback = WebKit::VoidCallback::create([protectedSelf = retainPtr(self), self](WebKit::CallbackBase::Error) {
    599600            _repaintCallback = nullptr;
     601
     602            if (_exitRequested) {
     603                _exitRequested = NO;
     604                [self _exitFullscreenImmediately];
     605                return;
     606            }
     607
    600608            if (auto* manager = [protectedSelf _manager]) {
    601609                manager->willEnterFullScreen();
    … …  
    640648    [_rootViewController presentViewController:_fullscreenViewController.get() animated:YES completion:^{
    641649        _fullScreenState = WebKit::InFullScreen;
     650
     651        if (_exitRequested) {
     652            _exitRequested = NO;
     653            [self _exitFullscreenImmediately];
     654            return;
     655        }
    642656
    643657        auto* page = [self._webView _page];
    … …  
    658672- (void)requestExitFullScreen
    659673{
     674    if (_fullScreenState != WebKit::InFullScreen) {
     675        _exitRequested = YES;
     676        return;
     677    }
     678
    660679    if (auto* manager = self._manager) {
    661680        manager->requestExitFullScreen();
    … …  
    669688- (void)exitFullScreen
    670689{
    671     if (!self.isFullScreen)
    672         return;
     690    if (_fullScreenState < WebKit::InFullScreen) {
     691        _exitRequested = YES;
     692        return;
     693    }
    673694    _fullScreenState = WebKit::WaitingToExitFullScreen;
    674695

  • trunk/Source/WebKit/WebProcess/cocoa/VideoFullscreenManager.h

    r238528 r249147  
    7474    void setLayerHostingContext(std::unique_ptr<LayerHostingContext>&&);
    7575
    76     bool isAnimating() const { return m_isAnimating; }
    77     void setIsAnimating(bool flag) { m_isAnimating = flag; }
     76    enum class AnimationType { None, IntoFullscreen, FromFullscreen };
     77    AnimationType animationState() const { return m_animationType; }
     78    void setAnimationState(AnimationType flag) { m_animationType = flag; }
    7879
    7980    bool targetIsFullscreen() const { return m_targetIsFullscreen; }
    … …  
    99100    uint64_t m_contextId;
    100101    std::unique_ptr<LayerHostingContext> m_layerHostingContext;
    101     bool m_isAnimating { false };
     102    AnimationType m_animationType { false };
    102103    bool m_targetIsFullscreen { false };
    103104    WebCore::HTMLMediaElementEnums::VideoFullscreenMode m_fullscreenMode { WebCore::HTMLMediaElementEnums::VideoFullscreenModeNone };

  • trunk/Source/WebKit/WebProcess/cocoa/VideoFullscreenManager.mm

    r247102 r249147  
    261261        model->setVideoLayerFrame(videoLayerFrame);
    262262
    263     if (interface->isAnimating())
    264         return;
    265     interface->setIsAnimating(true);
     263    if (interface->animationState() != VideoFullscreenInterfaceContext::AnimationType::None)
     264        return;
     265    interface->setAnimationState(VideoFullscreenInterfaceContext::AnimationType::IntoFullscreen);
    266266
    267267    bool allowsPictureInPicture = videoElement.webkitSupportsPresentationMode(HTMLVideoElement::VideoPresentationMode::PictureInPicture);
    … …  
    297297    interface.setTargetIsFullscreen(false);
    298298
    299     if (interface.isAnimating())
    300         return;
    301 
    302     interface.setIsAnimating(true);
     299    if (interface.animationState() == VideoFullscreenInterfaceContext::AnimationType::FromFullscreen)
     300        return;
     301    interface.setAnimationState(VideoFullscreenInterfaceContext::AnimationType::FromFullscreen);
    303302    m_page->send(Messages::VideoFullscreenManagerProxy::ExitFullscreen(contextId, inlineVideoFrame(videoElement)));
    304303}
    … …  
    439438    auto [model, interface] = ensureModelAndInterface(contextId);
    440439
    441     interface->setIsAnimating(false);
     440    interface->setAnimationState(VideoFullscreenInterfaceContext::AnimationType::None);
    442441    interface->setIsFullscreen(false);
    443442
    … …  
    502501    }
    503502
    504     interface->setIsAnimating(false);
     503    interface->setAnimationState(VideoFullscreenInterfaceContext::AnimationType::None);
    505504    interface->setIsFullscreen(false);
    506505    HTMLMediaElementEnums::VideoFullscreenMode mode = interface->fullscreenMode();
Note: See TracChangeset for help on using the changeset viewer.