Diff
Modified: trunk/LayoutTests/ChangeLog (249146 => 249147)
--- trunk/LayoutTests/ChangeLog 2019-08-27 16:19:37 UTC (rev 249146)
+++ trunk/LayoutTests/ChangeLog 2019-08-27 16:48:21 UTC (rev 249147)
@@ -1,3 +1,14 @@
+2019-08-26 Jer Noble <jer.no...@apple.com>
+
+ 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.
+
+ * fullscreen/full-screen-request-removed-with-raf-expected.txt: Added.
+ * fullscreen/full-screen-request-removed-with-raf.html: Added.
+
2019-08-27 Peng Liu <peng.l...@apple.com>
webkitpresentationmodechanged is fired twice when exiting picture in picture
Added: trunk/LayoutTests/fullscreen/full-screen-request-removed-with-raf-expected.txt (0 => 249147)
--- trunk/LayoutTests/fullscreen/full-screen-request-removed-with-raf-expected.txt (rev 0)
+++ trunk/LayoutTests/fullscreen/full-screen-request-removed-with-raf-expected.txt 2019-08-27 16:48:21 UTC (rev 249147)
@@ -0,0 +1,7 @@
+Tests that fullscreen is not entered if the fullscreen element ancestor is removed with rAF while entering fullscreen.
+
+Removed child element.
+
+SUCCESS
+END OF TEST
+
Added: trunk/LayoutTests/fullscreen/full-screen-request-removed-with-raf.html (0 => 249147)
--- trunk/LayoutTests/fullscreen/full-screen-request-removed-with-raf.html (rev 0)
+++ trunk/LayoutTests/fullscreen/full-screen-request-removed-with-raf.html 2019-08-27 16:48:21 UTC (rev 249147)
@@ -0,0 +1,43 @@
+
+<script src=""
+<script>
+
+ window.addEventListener('load', (ev) => {
+ var target = document.querySelector('#target');
+
+ document.addEventListener('webkitfullscreenchange', (ev) => {
+ if (document.webkitCurrentFullScreenElement && !document.webkitCurrentFullScreenElement.ownerDocument)
+ consoleWrite("FAIL: document.webkitCurrentFullScreenElement.ownerDocument is NULL!");
+ else
+ consoleWrite("SUCCESS");
+
+ document.webkitExitFullscreen();
+ endTest();
+ }, {once: true});
+
+ document.addEventListener('webkitfullscreenerror', (ev) => {
+ consoleWrite("SUCCESS");
+ endTest();
+ }, {once: true});
+
+ function test()
+ {
+ target.webkitRequestFullScreen();
+ window.requestAnimationFrame(() => {
+ consoleWrite("Removed child element.");
+ target.parentNode.removeChild(target);
+ consoleWrite("");
+ });
+ }
+
+ runWithKeyDown(test);
+ });
+
+</script>
+
+<p>Tests that fullscreen is not entered if the fullscreen element ancestor is removed with rAF while entering fullscreen.</p>
+
+<div id=ancestor>
+ <span id=target>Fullscreen target!</span>
+</div>
+
Modified: trunk/Source/WebCore/ChangeLog (249146 => 249147)
--- trunk/Source/WebCore/ChangeLog 2019-08-27 16:19:37 UTC (rev 249146)
+++ trunk/Source/WebCore/ChangeLog 2019-08-27 16:48:21 UTC (rev 249147)
@@ -1,3 +1,29 @@
+2019-08-26 Jer Noble <jer.no...@apple.com>
+
+ 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.
+
+ 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:
+
2019-08-27 Peng Liu <peng.l...@apple.com>
webkitpresentationmodechanged is fired twice when exiting picture in picture
Modified: trunk/Source/WebCore/dom/FullscreenManager.cpp (249146 => 249147)
--- trunk/Source/WebCore/dom/FullscreenManager.cpp 2019-08-27 16:19:37 UTC (rev 249146)
+++ trunk/Source/WebCore/dom/FullscreenManager.cpp 2019-08-27 16:48:21 UTC (rev 249147)
@@ -120,7 +120,16 @@
}
}
+ m_pendingFullscreenElement = element;
+
m_fullscreenTaskQueue.enqueueTask([this, element = makeRefPtr(element), checkType, hasKeyboardAccess, failedPreflights] () mutable {
+ // Don't allow fullscreen if it has been cancelled or a different fullscreen element
+ // has requested fullscreen.
+ if (m_pendingFullscreenElement != element) {
+ failedPreflights(WTFMove(element));
+ return;
+ }
+
// Don't allow fullscreen if document is hidden.
if (document().hidden()) {
failedPreflights(WTFMove(element));
@@ -209,9 +218,13 @@
// 5. Return, and run the remaining steps asynchronously.
// 6. Optionally, perform some animation.
m_areKeysEnabledInFullscreen = hasKeyboardAccess;
- m_fullscreenTaskQueue.enqueueTask([this, element = WTFMove(element)] {
- if (auto page = this->page())
- page->chrome().client().enterFullScreenForElement(*element.get());
+ m_fullscreenTaskQueue.enqueueTask([this, element = WTFMove(element), failedPreflights = WTFMove(failedPreflights)] () mutable {
+ auto page = this->page();
+ if (!page || document().hidden() || m_pendingFullscreenElement != element || !element->isConnected()) {
+ failedPreflights(element);
+ return;
+ }
+ page->chrome().client().enterFullScreenForElement(*element.get());
});
// 7. Optionally, display a message indicating how the user can exit displaying the context object fullscreen.
@@ -225,8 +238,13 @@
// "To fully exit fullscreen act as if the exitFullscreen() method was invoked on the top-level browsing
// context's document and subsequently empty that document's fullscreen element stack."
Document& topDocument = document().topDocument();
- if (!topDocument.fullscreenManager().fullscreenElement())
+ if (!topDocument.fullscreenManager().fullscreenElement()) {
+ // If there is a pending fullscreen element but no top document fullscreen element,
+ // there is a pending task in enterFullscreen(). Cause it to cancel and fire an error
+ // by clearing the pending fullscreen element.
+ m_pendingFullscreenElement = nullptr;
return;
+ }
// To achieve that aim, remove all the elements from the top document's stack except for the first before
// calling webkitExitFullscreen():
@@ -245,8 +263,13 @@
Document* currentDoc = &document();
// 2. If doc's fullscreen element stack is empty, terminate these steps.
- if (m_fullscreenElementStack.isEmpty())
+ if (m_fullscreenElementStack.isEmpty()) {
+ // If there is a pending fullscreen element but an empty fullscreen element stack,
+ // there is a pending task in requestFullscreenForElement(). Cause it to cancel and fire an error
+ // by clearing the pending fullscreen element.
+ m_pendingFullscreenElement = nullptr;
return;
+ }
// 3. Let descendants be all the doc's descendant browsing context's documents with a non-empty fullscreen
// element stack (if any), ordered so that the child of the doc is last and the document furthest
@@ -298,6 +321,14 @@
if (!page)
return;
+ // If there is a pending fullscreen element but no fullscreen element
+ // there is a pending task in requestFullscreenForElement(). Cause it to cancel and fire an error
+ // by clearing the pending fullscreen element.
+ if (!fullscreenElement && m_pendingFullscreenElement) {
+ m_pendingFullscreenElement = nullptr;
+ return;
+ }
+
// Only exit out of full screen window mode if there are no remaining elements in the
// full screen stack.
if (!newTop) {
@@ -339,6 +370,13 @@
if (!page())
return;
+ // If pending fullscreen element is unset or another element's was requested,
+ // issue a cancel fullscreen request to the client
+ if (m_pendingFullscreenElement != &element) {
+ page()->chrome().client().exitFullScreenForElement(&element);
+ return;
+ }
+
ASSERT(page()->settings().fullScreenEnabled());
unwrapFullscreenRenderer(m_fullscreenRenderer.get(), m_fullscreenElement.get());
@@ -345,6 +383,8 @@
element.willBecomeFullscreenElement();
+ ASSERT(&element == m_pendingFullscreenElement);
+ m_pendingFullscreenElement = nullptr;
m_fullscreenElement = &element;
#if USE(NATIVE_FULLSCREEN_VIDEO)
@@ -385,30 +425,32 @@
void FullscreenManager::willExitFullscreen()
{
- if (!m_fullscreenElement)
+ auto fullscreenElement = fullscreenOrPendingElement();
+ if (!fullscreenElement)
return;
if (!hasLivingRenderTree() || pageCacheState() != Document::NotInPageCache)
return;
- m_fullscreenElement->willStopBeingFullscreenElement();
+ fullscreenElement->willStopBeingFullscreenElement();
}
void FullscreenManager::didExitFullscreen()
{
- if (!m_fullscreenElement)
+ auto fullscreenElement = fullscreenOrPendingElement();
+ if (!fullscreenElement)
return;
if (!hasLivingRenderTree() || pageCacheState() != Document::NotInPageCache)
return;
+ fullscreenElement->setContainsFullScreenElementOnAncestorsCrossingFrameBoundaries(false);
- m_fullscreenElement->setContainsFullScreenElementOnAncestorsCrossingFrameBoundaries(false);
-
m_areKeysEnabledInFullscreen = false;
unwrapFullscreenRenderer(m_fullscreenRenderer.get(), m_fullscreenElement.get());
m_fullscreenElement = nullptr;
+ m_pendingFullscreenElement = nullptr;
scheduleFullStyleRebuild();
// When webkitCancelFullscreen is called, we call webkitExitFullscreen on the topDocument(). That
@@ -483,25 +525,22 @@
}
}
-void FullscreenManager::fullscreenElementRemoved()
-{
- m_fullscreenElement->setContainsFullScreenElementOnAncestorsCrossingFrameBoundaries(false);
- cancelFullscreen();
-}
-
void FullscreenManager::adjustFullscreenElementOnNodeRemoval(Node& node, Document::NodeRemoval nodeRemoval)
{
- if (!m_fullscreenElement)
+ auto fullscreenElement = fullscreenOrPendingElement();
+ if (!fullscreenElement)
return;
bool elementInSubtree = false;
if (nodeRemoval == Document::NodeRemoval::ChildrenOfNode)
- elementInSubtree = m_fullscreenElement->isDescendantOf(node);
+ elementInSubtree = fullscreenElement->isDescendantOf(node);
else
- elementInSubtree = (m_fullscreenElement == &node) || m_fullscreenElement->isDescendantOf(node);
+ elementInSubtree = (fullscreenElement == &node) || fullscreenElement->isDescendantOf(node);
- if (elementInSubtree)
- fullscreenElementRemoved();
+ if (elementInSubtree) {
+ fullscreenElement->setContainsFullScreenElementOnAncestorsCrossingFrameBoundaries(false);
+ cancelFullscreen();
+ }
}
bool FullscreenManager::isAnimatingFullscreen() const
@@ -541,6 +580,7 @@
void FullscreenManager::clear()
{
m_fullscreenElement = nullptr;
+ m_pendingFullscreenElement = nullptr;
m_fullscreenElementStack.clear();
}
Modified: trunk/Source/WebCore/dom/FullscreenManager.h (249146 => 249147)
--- trunk/Source/WebCore/dom/FullscreenManager.h 2019-08-27 16:19:37 UTC (rev 249146)
+++ trunk/Source/WebCore/dom/FullscreenManager.h 2019-08-27 16:48:21 UTC (rev 249147)
@@ -109,6 +109,9 @@
private:
Document& m_document;
+ RefPtr<Element> fullscreenOrPendingElement() const { return m_fullscreenElement ? m_fullscreenElement : m_pendingFullscreenElement; }
+
+ RefPtr<Element> m_pendingFullscreenElement;
RefPtr<Element> m_fullscreenElement;
Vector<RefPtr<Element>> m_fullscreenElementStack;
WeakPtr<RenderFullScreen> m_fullscreenRenderer { nullptr };
Modified: trunk/Source/WebKit/ChangeLog (249146 => 249147)
--- trunk/Source/WebKit/ChangeLog 2019-08-27 16:19:37 UTC (rev 249146)
+++ trunk/Source/WebKit/ChangeLog 2019-08-27 16:48:21 UTC (rev 249147)
@@ -1,3 +1,30 @@
+2019-08-26 Jer Noble <jer.no...@apple.com>
+
+ 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.
+
+ 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):
+
2019-08-27 Carlos Garcia Campos <cgar...@igalia.com>
Do not clear the pending api request when there's no navigation ID
Modified: trunk/Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.mm (249146 => 249147)
--- trunk/Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.mm 2019-08-27 16:19:37 UTC (rev 249146)
+++ trunk/Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.mm 2019-08-27 16:48:21 UTC (rev 249147)
@@ -447,6 +447,7 @@
RetainPtr<NSString> _EVOrganizationName;
BOOL _EVOrganizationNameIsValid;
BOOL _inInteractiveDismiss;
+ BOOL _exitRequested;
RetainPtr<id> _notificationListener;
}
@@ -597,6 +598,13 @@
_repaintCallback = WebKit::VoidCallback::create([protectedSelf = retainPtr(self), self](WebKit::CallbackBase::Error) {
_repaintCallback = nullptr;
+
+ if (_exitRequested) {
+ _exitRequested = NO;
+ [self _exitFullscreenImmediately];
+ return;
+ }
+
if (auto* manager = [protectedSelf _manager]) {
manager->willEnterFullScreen();
return;
@@ -640,6 +648,12 @@
[_rootViewController presentViewController:_fullscreenViewController.get() animated:YES completion:^{
_fullScreenState = WebKit::InFullScreen;
+ if (_exitRequested) {
+ _exitRequested = NO;
+ [self _exitFullscreenImmediately];
+ return;
+ }
+
auto* page = [self._webView _page];
auto* manager = self._manager;
if (page && manager) {
@@ -657,6 +671,11 @@
- (void)requestExitFullScreen
{
+ if (_fullScreenState != WebKit::InFullScreen) {
+ _exitRequested = YES;
+ return;
+ }
+
if (auto* manager = self._manager) {
manager->requestExitFullScreen();
return;
@@ -668,8 +687,10 @@
- (void)exitFullScreen
{
- if (!self.isFullScreen)
+ if (_fullScreenState < WebKit::InFullScreen) {
+ _exitRequested = YES;
return;
+ }
_fullScreenState = WebKit::WaitingToExitFullScreen;
if (auto* manager = self._manager) {
Modified: trunk/Source/WebKit/WebProcess/cocoa/VideoFullscreenManager.h (249146 => 249147)
--- trunk/Source/WebKit/WebProcess/cocoa/VideoFullscreenManager.h 2019-08-27 16:19:37 UTC (rev 249146)
+++ trunk/Source/WebKit/WebProcess/cocoa/VideoFullscreenManager.h 2019-08-27 16:48:21 UTC (rev 249147)
@@ -73,8 +73,9 @@
LayerHostingContext* layerHostingContext() { return m_layerHostingContext.get(); }
void setLayerHostingContext(std::unique_ptr<LayerHostingContext>&&);
- bool isAnimating() const { return m_isAnimating; }
- void setIsAnimating(bool flag) { m_isAnimating = flag; }
+ enum class AnimationType { None, IntoFullscreen, FromFullscreen };
+ AnimationType animationState() const { return m_animationType; }
+ void setAnimationState(AnimationType flag) { m_animationType = flag; }
bool targetIsFullscreen() const { return m_targetIsFullscreen; }
void setTargetIsFullscreen(bool flag) { m_targetIsFullscreen = flag; }
@@ -98,7 +99,7 @@
VideoFullscreenManager* m_manager;
uint64_t m_contextId;
std::unique_ptr<LayerHostingContext> m_layerHostingContext;
- bool m_isAnimating { false };
+ AnimationType m_animationType { false };
bool m_targetIsFullscreen { false };
WebCore::HTMLMediaElementEnums::VideoFullscreenMode m_fullscreenMode { WebCore::HTMLMediaElementEnums::VideoFullscreenModeNone };
bool m_fullscreenStandby { false };
Modified: trunk/Source/WebKit/WebProcess/cocoa/VideoFullscreenManager.mm (249146 => 249147)
--- trunk/Source/WebKit/WebProcess/cocoa/VideoFullscreenManager.mm 2019-08-27 16:19:37 UTC (rev 249146)
+++ trunk/Source/WebKit/WebProcess/cocoa/VideoFullscreenManager.mm 2019-08-27 16:48:21 UTC (rev 249147)
@@ -260,9 +260,9 @@
if (oldMode == HTMLMediaElementEnums::VideoFullscreenModeNone && mode != HTMLMediaElementEnums::VideoFullscreenModeNone)
model->setVideoLayerFrame(videoLayerFrame);
- if (interface->isAnimating())
+ if (interface->animationState() != VideoFullscreenInterfaceContext::AnimationType::None)
return;
- interface->setIsAnimating(true);
+ interface->setAnimationState(VideoFullscreenInterfaceContext::AnimationType::IntoFullscreen);
bool allowsPictureInPicture = videoElement.webkitSupportsPresentationMode(HTMLVideoElement::VideoPresentationMode::PictureInPicture);
@@ -296,10 +296,9 @@
interface.setTargetIsFullscreen(false);
- if (interface.isAnimating())
+ if (interface.animationState() == VideoFullscreenInterfaceContext::AnimationType::FromFullscreen)
return;
-
- interface.setIsAnimating(true);
+ interface.setAnimationState(VideoFullscreenInterfaceContext::AnimationType::FromFullscreen);
m_page->send(Messages::VideoFullscreenManagerProxy::ExitFullscreen(contextId, inlineVideoFrame(videoElement)));
}
@@ -438,7 +437,7 @@
auto [model, interface] = ensureModelAndInterface(contextId);
- interface->setIsAnimating(false);
+ interface->setAnimationState(VideoFullscreenInterfaceContext::AnimationType::None);
interface->setIsFullscreen(false);
RefPtr<HTMLVideoElement> videoElement = model->videoElement();
@@ -501,7 +500,7 @@
interface->setLayerHostingContext(nullptr);
}
- interface->setIsAnimating(false);
+ interface->setAnimationState(VideoFullscreenInterfaceContext::AnimationType::None);
interface->setIsFullscreen(false);
HTMLMediaElementEnums::VideoFullscreenMode mode = interface->fullscreenMode();
bool standby = interface->fullscreenStandby();
