Title: [287803] trunk/Source
Revision
287803
Author
commit-qu...@webkit.org
Date
2022-01-07 18:20:37 -0800 (Fri, 07 Jan 2022)

Log Message

Make FullscreenManager::requestFullscreenForElement more robust
https://bugs.webkit.org/show_bug.cgi?id=234995

Patch by Alex Christensen <achristen...@webkit.org> on 2022-01-07
Reviewed by Darin Adler.

Source/WebCore:

I think this may fix the Windows crashes after bug 233963 lands, and it makes things more robust anyways.

* dom/Element.cpp:
(WebCore::Element::webkitRequestFullscreen):
* dom/FullscreenManager.cpp:
(WebCore::FullscreenManager::requestFullscreenForElement):
* dom/FullscreenManager.h:
* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::enterFullscreen):

Source/WebKit:

* WebProcess/FullScreen/WebFullScreenManager.cpp:
(WebKit::WebFullScreenManager::requestEnterFullScreen):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (287802 => 287803)


--- trunk/Source/WebCore/ChangeLog	2022-01-08 02:13:46 UTC (rev 287802)
+++ trunk/Source/WebCore/ChangeLog	2022-01-08 02:20:37 UTC (rev 287803)
@@ -1,3 +1,20 @@
+2022-01-07  Alex Christensen  <achristen...@webkit.org>
+
+        Make FullscreenManager::requestFullscreenForElement more robust
+        https://bugs.webkit.org/show_bug.cgi?id=234995
+
+        Reviewed by Darin Adler.
+
+        I think this may fix the Windows crashes after bug 233963 lands, and it makes things more robust anyways.
+
+        * dom/Element.cpp:
+        (WebCore::Element::webkitRequestFullscreen):
+        * dom/FullscreenManager.cpp:
+        (WebCore::FullscreenManager::requestFullscreenForElement):
+        * dom/FullscreenManager.h:
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::enterFullscreen):
+
 2022-01-07  Alexey Shvayka  <ashva...@apple.com>
 
         Don't dispatch "focusin" / "focusout" events if there are no listeners

Modified: trunk/Source/WebCore/dom/Element.cpp (287802 => 287803)


--- trunk/Source/WebCore/dom/Element.cpp	2022-01-08 02:13:46 UTC (rev 287802)
+++ trunk/Source/WebCore/dom/Element.cpp	2022-01-08 02:20:37 UTC (rev 287803)
@@ -3869,7 +3869,7 @@
 
 void Element::webkitRequestFullscreen()
 {
-    document().fullscreenManager().requestFullscreenForElement(this, FullscreenManager::EnforceIFrameAllowFullscreenRequirement);
+    document().fullscreenManager().requestFullscreenForElement(*this, FullscreenManager::EnforceIFrameAllowFullscreenRequirement);
 }
 
 void Element::setContainsFullScreenElement(bool flag)

Modified: trunk/Source/WebCore/dom/FullscreenManager.cpp (287802 => 287803)


--- trunk/Source/WebCore/dom/FullscreenManager.cpp	2022-01-08 02:13:46 UTC (rev 287802)
+++ trunk/Source/WebCore/dom/FullscreenManager.cpp	2022-01-08 02:20:37 UTC (rev 287803)
@@ -58,12 +58,11 @@
 
 FullscreenManager::~FullscreenManager() = default;
 
-void FullscreenManager::requestFullscreenForElement(Element* element, FullscreenCheckType checkType)
+void FullscreenManager::requestFullscreenForElement(Ref<Element>&& element, FullscreenCheckType checkType)
 {
-    if (!element)
-        element = documentElement();
-
-    auto failedPreflights = [this, weakThis = WeakPtr { *this }](auto element) mutable {
+    auto failedPreflights = [this, weakThis = WeakPtr { *this }](Ref<Element>&& element) mutable {
+        if (!weakThis)
+            return;
         m_fullscreenErrorEventTargetQueue.append(WTFMove(element));
         m_document.eventLoop().queueTask(TaskSource::MediaElement, [weakThis = WTFMove(weakThis)]() mutable {
             if (weakThis)
@@ -102,12 +101,12 @@
     }
 
     bool hasKeyboardAccess = true;
-    if (!page()->chrome().client().supportsFullScreenForElement(*element, hasKeyboardAccess)) {
+    if (!page()->chrome().client().supportsFullScreenForElement(element, hasKeyboardAccess)) {
         // The new full screen API does not accept a "flags" parameter, so fall back to disallowing
         // keyboard input if the chrome client refuses to allow keyboard input.
         hasKeyboardAccess = false;
 
-        if (!page()->chrome().client().supportsFullScreenForElement(*element, hasKeyboardAccess)) {
+        if (!page()->chrome().client().supportsFullScreenForElement(element, hasKeyboardAccess)) {
             ERROR_LOG(LOGIDENTIFIER, "page does not support fullscreen for element; failing.");
             failedPreflights(WTFMove(element));
             return;
@@ -114,15 +113,15 @@
         }
     }
 
-    m_pendingFullscreenElement = element;
+    m_pendingFullscreenElement = RefPtr { element.ptr() };
 
-    m_document.eventLoop().queueTask(TaskSource::MediaElement, [this, weakThis = WeakPtr { *this }, element = RefPtr { element }, checkType, hasKeyboardAccess, failedPreflights, identifier = LOGIDENTIFIER] () mutable {
+    m_document.eventLoop().queueTask(TaskSource::MediaElement, [this, weakThis = WeakPtr { *this }, element = WTFMove(element), checkType, hasKeyboardAccess, failedPreflights, identifier = LOGIDENTIFIER] () mutable {
         if (!weakThis)
             return;
 
         // Don't allow fullscreen if it has been cancelled or a different fullscreen element
         // has requested fullscreen.
-        if (m_pendingFullscreenElement != element) {
+        if (m_pendingFullscreenElement != element.ptr()) {
             ERROR_LOG(identifier, "task - pending element mismatch; failing.");
             failedPreflights(WTFMove(element));
             return;
@@ -205,7 +204,7 @@
             // stack, and queue a task to fire an event named fullscreenchange with its bubbles attribute
             // set to true on the document.
             if (!followingDoc) {
-                currentDoc->fullscreenManager().pushFullscreenElementStack(*element);
+                currentDoc->fullscreenManager().pushFullscreenElementStack(element);
                 addDocumentToFullscreenChangeEventQueue(*currentDoc);
                 continue;
             }
@@ -233,13 +232,13 @@
                 return;
 
             auto page = this->page();
-            if (!page || document().hidden() || m_pendingFullscreenElement != element || !element->isConnected()) {
+            if (!page || document().hidden() || m_pendingFullscreenElement != element.ptr() || !element->isConnected()) {
                 ERROR_LOG(identifier, "task - page, document, or element mismatch; failing.");
-                failedPreflights(element);
+                failedPreflights(WTFMove(element));
                 return;
             }
             INFO_LOG(identifier, "task - success");
-            page->chrome().client().enterFullScreenForElement(*element.get());
+            page->chrome().client().enterFullScreenForElement(element);
         });
 
         // 7. Optionally, display a message indicating how the user can exit displaying the context object fullscreen.

Modified: trunk/Source/WebCore/dom/FullscreenManager.h (287802 => 287803)


--- trunk/Source/WebCore/dom/FullscreenManager.h	2022-01-08 02:13:46 UTC (rev 287802)
+++ trunk/Source/WebCore/dom/FullscreenManager.h	2022-01-08 02:20:37 UTC (rev 287803)
@@ -69,7 +69,7 @@
         EnforceIFrameAllowFullscreenRequirement,
         ExemptIFrameAllowFullscreenRequirement,
     };
-    WEBCORE_EXPORT void requestFullscreenForElement(Element*, FullscreenCheckType);
+    WEBCORE_EXPORT void requestFullscreenForElement(Ref<Element>&&, FullscreenCheckType);
 
     WEBCORE_EXPORT bool willEnterFullscreen(Element&);
     WEBCORE_EXPORT bool didEnterFullscreen();

Modified: trunk/Source/WebCore/html/HTMLMediaElement.cpp (287802 => 287803)


--- trunk/Source/WebCore/html/HTMLMediaElement.cpp	2022-01-08 02:13:46 UTC (rev 287802)
+++ trunk/Source/WebCore/html/HTMLMediaElement.cpp	2022-01-08 02:20:37 UTC (rev 287803)
@@ -6303,7 +6303,7 @@
     if (document().settings().fullScreenEnabled() && mode == VideoFullscreenModeStandard) {
         m_temporarilyAllowingInlinePlaybackAfterFullscreen = false;
         m_waitingToEnterFullscreen = true;
-        document().fullscreenManager().requestFullscreenForElement(this, FullscreenManager::ExemptIFrameAllowFullscreenRequirement);
+        document().fullscreenManager().requestFullscreenForElement(*this, FullscreenManager::ExemptIFrameAllowFullscreenRequirement);
         return;
     }
 #endif

Modified: trunk/Source/WebKit/ChangeLog (287802 => 287803)


--- trunk/Source/WebKit/ChangeLog	2022-01-08 02:13:46 UTC (rev 287802)
+++ trunk/Source/WebKit/ChangeLog	2022-01-08 02:20:37 UTC (rev 287803)
@@ -1,3 +1,13 @@
+2022-01-07  Alex Christensen  <achristen...@webkit.org>
+
+        Make FullscreenManager::requestFullscreenForElement more robust
+        https://bugs.webkit.org/show_bug.cgi?id=234995
+
+        Reviewed by Darin Adler.
+
+        * WebProcess/FullScreen/WebFullScreenManager.cpp:
+        (WebKit::WebFullScreenManager::requestEnterFullScreen):
+
 2022-01-07  Chris Lord  <cl...@igalia.com>
 
         [GTK] REGRESSION: Kinetic scrolling via touch-screen behaves oddly

Modified: trunk/Source/WebKit/WebProcess/FullScreen/WebFullScreenManager.cpp (287802 => 287803)


--- trunk/Source/WebKit/WebProcess/FullScreen/WebFullScreenManager.cpp	2022-01-08 02:13:46 UTC (rev 287802)
+++ trunk/Source/WebKit/WebProcess/FullScreen/WebFullScreenManager.cpp	2022-01-08 02:20:37 UTC (rev 287803)
@@ -242,7 +242,7 @@
         return;
 
     WebCore::UserGestureIndicator gestureIndicator(WebCore::ProcessingUserGesture);
-    m_element->document().fullscreenManager().requestFullscreenForElement(m_element.get(), WebCore::FullscreenManager::ExemptIFrameAllowFullscreenRequirement);
+    m_element->document().fullscreenManager().requestFullscreenForElement(*m_element, WebCore::FullscreenManager::ExemptIFrameAllowFullscreenRequirement);
 }
 
 void WebFullScreenManager::requestExitFullScreen()
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to