Changeset 238322 in webkit


Ignore:
Timestamp:
Nov 16, 2018 4:15:55 PM (5 years ago)
Author:
jer.noble@apple.com
Message:

Regression(r233865): Causes synchronous IPC in the middle of layout
https://bugs.webkit.org/show_bug.cgi?id=188307
<rdar://problem/42807306>

Reviewed by Eric Carlson.

Revert the changes added in r233865. Rather than make a syncronous call to the UIProcess to
query whether the view has been backgrounded while (e.g.) JS has been spinning, perform the
steps of the requestFullscreen() method on the next run loop, allowing messages from the
UIProcess about page visibilty to be delivered first.

  • dom/Document.cpp:

(WebCore::Document::requestFullScreenForElement):

  • html/HTMLMediaElement.cpp:

(WebCore::HTMLMediaElement::enterFullscreen):

  • html/HTMLMediaElement.h:
  • page/ChromeClient.h:
Location:
trunk/Source
Files:
9 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r238321 r238322  
     12018-11-16  Jer Noble  <jer.noble@apple.com>
     2
     3        Regression(r233865): Causes synchronous IPC in the middle of layout
     4        https://bugs.webkit.org/show_bug.cgi?id=188307
     5        <rdar://problem/42807306>
     6
     7        Reviewed by Eric Carlson.
     8
     9        Revert the changes added in r233865. Rather than make a syncronous call to the UIProcess to
     10        query whether the view has been backgrounded while (e.g.) JS has been spinning, perform the
     11        steps of the requestFullscreen() method on the next run loop, allowing messages from the
     12        UIProcess about page visibilty to be delivered first.
     13
     14        * dom/Document.cpp:
     15        (WebCore::Document::requestFullScreenForElement):
     16        * html/HTMLMediaElement.cpp:
     17        (WebCore::HTMLMediaElement::enterFullscreen):
     18        * html/HTMLMediaElement.h:
     19        * page/ChromeClient.h:
     20
    1212018-11-16  Ross Kirsling  <ross.kirsling@sony.com>
    222
  • trunk/Source/WebCore/dom/Document.cpp

    r238223 r238322  
    62126212void Document::requestFullScreenForElement(Element* element, FullScreenCheckType checkType)
    62136213{
    6214     do {
    6215         if (!element)
    6216             element = documentElement();
    6217  
    6218         // 1. If any of the following conditions are true, terminate these steps and queue a task to fire
    6219         // an event named fullscreenerror with its bubbles attribute set to true on the context object's
    6220         // node document:
    6221 
     6214    if (!element)
     6215        element = documentElement();
     6216
     6217    auto failedPreflights = [this](auto element) mutable {
     6218        m_fullScreenErrorEventTargetQueue.append(WTFMove(element));
     6219        m_fullScreenTaskQueue.enqueueTask([this] {
     6220            dispatchFullScreenChangeEvents();
     6221        });
     6222    };
     6223
     6224    // 1. If any of the following conditions are true, terminate these steps and queue a task to fire
     6225    // an event named fullscreenerror with its bubbles attribute set to true on the context object's
     6226    // node document:
     6227
     6228    // This algorithm is not allowed to show a pop-up:
     6229    //   An algorithm is allowed to show a pop-up if, in the task in which the algorithm is running, either:
     6230    //   - an activation behavior is currently being processed whose click event was trusted, or
     6231    //   - the event listener for a trusted click event is being handled.
     6232    if (!UserGestureIndicator::processingUserGesture()) {
     6233        failedPreflights(WTFMove(element));
     6234        return;
     6235    }
     6236
     6237    // We do not allow pressing the Escape key as a user gesture to enter fullscreen since this is the key
     6238    // to exit fullscreen.
     6239    if (UserGestureIndicator::currentUserGesture()->gestureType() == UserGestureType::EscapeKey) {
     6240        addConsoleMessage(MessageSource::Security, MessageLevel::Error, "The Escape key may not be used as a user gesture to enter fullscreen"_s);
     6241        failedPreflights(WTFMove(element));
     6242        return;
     6243    }
     6244
     6245    // There is a previously-established user preference, security risk, or platform limitation.
     6246    if (!page() || !page()->settings().fullScreenEnabled()) {
     6247        failedPreflights(WTFMove(element));
     6248        return;
     6249    }
     6250
     6251    bool hasKeyboardAccess = true;
     6252    if (!page()->chrome().client().supportsFullScreenForElement(*element, hasKeyboardAccess)) {
     6253        // The new full screen API does not accept a "flags" parameter, so fall back to disallowing
     6254        // keyboard input if the chrome client refuses to allow keyboard input.
     6255        hasKeyboardAccess = false;
     6256
     6257        if (!page()->chrome().client().supportsFullScreenForElement(*element, hasKeyboardAccess)) {
     6258            failedPreflights(WTFMove(element));
     6259            return;
     6260        }
     6261    }
     6262
     6263    m_fullScreenTaskQueue.enqueueTask([this, element = makeRefPtr(element), checkType, hasKeyboardAccess, failedPreflights] () mutable {
    62226264        // Don't allow fullscreen if document is hidden.
    6223         if (!page() || !page()->chrome().client().isViewVisible())
    6224             break;
     6265        if (hidden()) {
     6266            failedPreflights(WTFMove(element));
     6267            return;
     6268        }
    62256269
    62266270        // The context object is not in a document.
    6227         if (!element->isConnected())
    6228             break;
     6271        if (!element->isConnected()) {
     6272            failedPreflights(WTFMove(element));
     6273            return;
     6274        }
    62296275
    62306276        // The context object's node document, or an ancestor browsing context's document does not have
    62316277        // the fullscreen enabled flag set.
    6232         if (checkType == EnforceIFrameAllowFullScreenRequirement && !fullScreenIsAllowedForElement(element))
    6233             break;
     6278        if (checkType == EnforceIFrameAllowFullScreenRequirement && !fullScreenIsAllowedForElement(element.get())) {
     6279            failedPreflights(WTFMove(element));
     6280            return;
     6281        }
    62346282
    62356283        // The context object's node document fullscreen element stack is not empty and its top element
    62366284        // is not an ancestor of the context object.
    6237         if (!m_fullScreenElementStack.isEmpty() && !m_fullScreenElementStack.last()->contains(element))
    6238             break;
     6285        if (!m_fullScreenElementStack.isEmpty() && !m_fullScreenElementStack.last()->contains(element.get())) {
     6286            failedPreflights(WTFMove(element));
     6287            return;
     6288        }
    62396289
    62406290        // A descendant browsing context's document has a non-empty fullscreen element stack.
     
    62466296            }
    62476297        }
    6248         if (descendentHasNonEmptyStack)
    6249             break;
    6250 
    6251         // This algorithm is not allowed to show a pop-up:
    6252         //   An algorithm is allowed to show a pop-up if, in the task in which the algorithm is running, either:
    6253         //   - an activation behavior is currently being processed whose click event was trusted, or
    6254         //   - the event listener for a trusted click event is being handled.
    6255         if (!UserGestureIndicator::processingUserGesture())
    6256             break;
    6257 
    6258         // We do not allow pressing the Escape key as a user gesture to enter fullscreen since this is the key
    6259         // to exit fullscreen.
    6260         if (UserGestureIndicator::currentUserGesture()->gestureType() == UserGestureType::EscapeKey) {
    6261             addConsoleMessage(MessageSource::Security, MessageLevel::Error, "The Escape key may not be used as a user gesture to enter fullscreen"_s);
    6262             break;
    6263         }
    6264 
    6265         // There is a previously-established user preference, security risk, or platform limitation.
    6266         if (!page() || !page()->settings().fullScreenEnabled())
    6267             break;
    6268 
    6269         bool hasKeyboardAccess = true;
    6270         if (!page()->chrome().client().supportsFullScreenForElement(*element, hasKeyboardAccess)) {
    6271             // The new full screen API does not accept a "flags" parameter, so fall back to disallowing
    6272             // keyboard input if the chrome client refuses to allow keyboard input.
    6273             hasKeyboardAccess = false;
    6274 
    6275             if (!page()->chrome().client().supportsFullScreenForElement(*element, hasKeyboardAccess))
    6276                 break;
     6298        if (descendentHasNonEmptyStack) {
     6299            failedPreflights(WTFMove(element));
     6300            return;
    62776301        }
    62786302
     
    63036327            // set to true on the document.
    63046328            if (!followingDoc) {
    6305                 currentDoc->pushFullscreenElementStack(element);
     6329                currentDoc->pushFullscreenElementStack(element.get());
    63066330                addDocumentToFullScreenChangeEventQueue(currentDoc);
    63076331                continue;
     
    63266350        // 6. Optionally, perform some animation.
    63276351        m_areKeysEnabledInFullScreen = hasKeyboardAccess;
    6328         m_fullScreenTaskQueue.enqueueTask([this, element = makeRefPtr(element)] {
     6352        m_fullScreenTaskQueue.enqueueTask([this, element = WTFMove(element)] {
    63296353            if (auto page = this->page())
    6330                 page->chrome().client().enterFullScreenForElement(*element);
     6354                page->chrome().client().enterFullScreenForElement(*element.get());
    63316355        });
    63326356
    63336357        // 7. Optionally, display a message indicating how the user can exit displaying the context object fullscreen.
    6334         return;
    6335     } while (0);
    6336 
    6337     m_fullScreenErrorEventTargetQueue.append(element ? element : documentElement());
    6338     m_fullScreenTaskQueue.enqueueTask([this] {
    6339         dispatchFullScreenChangeEvents();
    63406358    });
    63416359}
  • trunk/Source/WebCore/html/HTMLMediaElement.cpp

    r237376 r238322  
    59795979        return;
    59805980
    5981     if (!document().page() || !document().page()->chrome().client().isViewVisible()) {
    5982         ALWAYS_LOG(LOGIDENTIFIER, "  returning because document is hidden");
    5983         return;
    5984     }
    5985 
    59865981    m_temporarilyAllowingInlinePlaybackAfterFullscreen = false;
    59875982    m_waitingToEnterFullscreen = true;
     
    59945989#endif
    59955990
    5996     fullscreenModeChanged(mode);
    5997     configureMediaControls();
    5998     if (hasMediaControls())
    5999         mediaControls()->enteredFullscreen();
    6000     if (is<HTMLVideoElement>(*this)) {
    6001         HTMLVideoElement& asVideo = downcast<HTMLVideoElement>(*this);
    6002         if (document().page()->chrome().client().supportsVideoFullscreen(m_videoFullscreenMode)) {
    6003             document().page()->chrome().client().enterVideoFullscreenForVideoElement(asVideo, m_videoFullscreenMode, m_videoFullscreenStandby);
    6004             scheduleEvent(eventNames().webkitbeginfullscreenEvent);
     5991    m_fullscreenTaskQueue.enqueueTask([this, mode] {
     5992        if (document().hidden()) {
     5993            ALWAYS_LOG(LOGIDENTIFIER, "  returning because document is hidden");
     5994            return;
    60055995        }
    6006     }
     5996
     5997        fullscreenModeChanged(mode);
     5998        configureMediaControls();
     5999        if (hasMediaControls())
     6000            mediaControls()->enteredFullscreen();
     6001        if (is<HTMLVideoElement>(*this)) {
     6002            HTMLVideoElement& asVideo = downcast<HTMLVideoElement>(*this);
     6003            if (document().page()->chrome().client().supportsVideoFullscreen(m_videoFullscreenMode)) {
     6004                document().page()->chrome().client().enterVideoFullscreenForVideoElement(asVideo, m_videoFullscreenMode, m_videoFullscreenStandby);
     6005                scheduleEvent(eventNames().webkitbeginfullscreenEvent);
     6006            }
     6007        }
     6008    });
    60076009}
    60086010
  • trunk/Source/WebCore/html/HTMLMediaElement.h

    r237266 r238322  
    956956    GenericTaskQueue<Timer> m_resourceSelectionTaskQueue;
    957957    GenericTaskQueue<Timer> m_visibilityChangeTaskQueue;
     958    GenericTaskQueue<Timer> m_fullscreenTaskQueue;
    958959    RefPtr<TimeRanges> m_playedTimeRanges;
    959960    GenericEventQueue m_asyncEventQueue;
  • trunk/Source/WebCore/page/ChromeClient.h

    r238064 r238322  
    490490    virtual String signedPublicKeyAndChallengeString(unsigned, const String&, const URL&) const { return emptyString(); }
    491491
    492     virtual bool isViewVisible() { return true; }
    493 
    494492protected:
    495493    virtual ~ChromeClient() = default;
  • trunk/Source/WebKit/UIProcess/WebPageProxy.messages.in

    r238193 r238322  
    6262    ScreenToRootView(WebCore::IntPoint screenPoint) -> (WebCore::IntPoint windowPoint) Delayed
    6363    RootViewToScreen(WebCore::IntRect rect) -> (WebCore::IntRect screenFrame) Delayed
    64     GetIsViewVisible() -> (bool result) LegacySync
    6564
    6665#if PLATFORM(COCOA)
  • trunk/Source/WebKit/UIProcess/mac/ViewSnapshotStore.h

    r230030 r238322  
    4242OBJC_CLASS CAContext;
    4343
     44#if HAVE(IOSURFACE)
    4445namespace WebCore {
    4546class IOSurface;
    4647}
     48#endif
    4749
    4850namespace WebKit {
  • trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp

    r238064 r238322  
    13171317#endif
    13181318
    1319 bool WebChromeClient::isViewVisible()
    1320 {
    1321     bool isVisible = false;
    1322     WebProcess::singleton().parentProcessConnection()->sendSync(Messages::WebPageProxy::GetIsViewVisible(), Messages::WebPageProxy::GetIsViewVisible::Reply(isVisible), m_page.pageID());
    1323     return isVisible;
    1324 }
    1325 
    13261319} // namespace WebKit
  • trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h

    r238064 r238322  
    368368#endif
    369369
    370     bool isViewVisible() final;
    371 
    372370    String m_cachedToolTip;
    373371    mutable RefPtr<WebFrame> m_cachedFrameSetLargestFrame;
Note: See TracChangeset for help on using the changeset viewer.