Changeset 287803 in webkit

Timestamp:

Jan 7, 2022, 6:20:37 PM (5 years ago)

Author:

commit-queue@webkit.org

Message:

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

Patch by Alex Christensen <​achristensen@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):

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/dom/Element.cpp (modified) (1 diff)
• WebCore/dom/FullscreenManager.cpp (modified) (6 diffs)
• WebCore/dom/FullscreenManager.h (modified) (1 diff)
• WebCore/html/HTMLMediaElement.cpp (modified) (1 diff)
• WebKit/ChangeLog (modified) (1 diff)
• WebKit/WebProcess/FullScreen/WebFullScreenManager.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2022-01-07  Alex Christensen  <achristensen@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  <ashvayka@apple.com>
 

trunk/Source/WebCore/dom/Element.cpp

@@ -3870 +3870 @@
 void Element::webkitRequestFullscreen()
 {
-    document().fullscreenManager().requestFullscreenForElement(this, FullscreenManager::EnforceIFrameAllowFullscreenRequirement);
+    document().fullscreenManager().requestFullscreenForElement(*this, FullscreenManager::EnforceIFrameAllowFullscreenRequirement);
 }
 

trunk/Source/WebCore/dom/FullscreenManager.cpp

@@ -59 +59 @@
 FullscreenManager::~FullscreenManager() = default;
 
-void FullscreenManager::requestFullscreenForElement(Element* element, FullscreenCheckType checkType)
-{
-    if (!element)
-        element = documentElement();
-
-    auto failedPreflights = [this, weakThis = WeakPtr { *this }](auto element) mutable {
+void FullscreenManager::requestFullscreenForElement(Ref<Element>&& element, FullscreenCheckType checkType)
+{
+    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 {
@@ -103 +102 @@
 
     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));
@@ -115 +114 @@
     }
 
-    m_pendingFullscreenElement = element;
-
-    m_document.eventLoop().queueTask(TaskSource::MediaElement, [this, weakThis = WeakPtr { *this }, element = RefPtr { element }, checkType, hasKeyboardAccess, failedPreflights, identifier = LOGIDENTIFIER] () mutable {
+    m_pendingFullscreenElement = RefPtr { element.ptr() };
+
+    m_document.eventLoop().queueTask(TaskSource::MediaElement, [this, weakThis = WeakPtr { *this }, element = WTFMove(element), checkType, hasKeyboardAccess, failedPreflights, identifier = LOGIDENTIFIER] () mutable {
         if (!weakThis)
             return;
@@ -123 +122 @@
         // 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));
@@ -206 +205 @@
             // set to true on the document.
             if (!followingDoc) {
-                currentDoc->fullscreenManager().pushFullscreenElementStack(*element);
+                currentDoc->fullscreenManager().pushFullscreenElementStack(element);
                 addDocumentToFullscreenChangeEventQueue(*currentDoc);
                 continue;
@@ -234 +233 @@
 
             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);
         });
 

trunk/Source/WebCore/dom/FullscreenManager.h

@@ -70 +70 @@
         ExemptIFrameAllowFullscreenRequirement,
     };
-    WEBCORE_EXPORT void requestFullscreenForElement(Element*, FullscreenCheckType);
+    WEBCORE_EXPORT void requestFullscreenForElement(Ref<Element>&&, FullscreenCheckType);
 
     WEBCORE_EXPORT bool willEnterFullscreen(Element&);

trunk/Source/WebCore/html/HTMLMediaElement.cpp

@@ -6304 +6304 @@
         m_temporarilyAllowingInlinePlaybackAfterFullscreen = false;
         m_waitingToEnterFullscreen = true;
-        document().fullscreenManager().requestFullscreenForElement(this, FullscreenManager::ExemptIFrameAllowFullscreenRequirement);
+        document().fullscreenManager().requestFullscreenForElement(*this, FullscreenManager::ExemptIFrameAllowFullscreenRequirement);
         return;
     }

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2022-01-07  Alex Christensen  <achristensen@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  <clord@igalia.com>
 

trunk/Source/WebKit/WebProcess/FullScreen/WebFullScreenManager.cpp

@@ -243 +243 @@
 
     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);
 }