Title: [292955] trunk
Revision
292955
Author
wenson_hs...@apple.com
Date
2022-04-18 09:09:17 -0700 (Mon, 18 Apr 2022)

Log Message

Make the main video heuristic robust when video elements are added after entering fullscreen
https://bugs.webkit.org/show_bug.cgi?id=239438
rdar://91867187

Reviewed by Eric Carlson.

Source/WebKit:

Adjust the heuristic for identifying the most prominent video element in an element fullscreen container, such
that it can detect video elements that are inserted into the document after we've already entered fullscreen
mode. See below for more details.

Exercised by a new internal API test.

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

Listen for several events (play, pause, loadedmetadata) that are bubbled up to the fullscreen container element,
from any media element underneath the fullscreen container; in response to these events, we re-run the main
fullscreen video element heuristic to update `m_mainVideoElement` (and cancel and reschedule the video
extraction timer, if this main video element changed).

(WebKit::WebFullScreenManager::enterFullScreenForElement):
(WebKit::WebFullScreenManager::didEnterFullScreen):

Pull existing logic for walking through the element fullscreen DOM in search of video elements out into a new
helper method, `updateMainVideoElement`, that's invoked both when we finish entering fullscreen mode, and when
observing any of the new events that bubble up to the fullscreen container.

(WebKit::WebFullScreenManager::updateMainVideoElement):
(WebKit::WebFullScreenManager::handleEvent):
(WebKit::WebFullScreenManager::setMainVideoElement):
* WebProcess/FullScreen/WebFullScreenManager.h:

Tools:

Add a helper method to a test page used in API tests; see the radar for more information.

* TestWebKitAPI/Tests/WebKitCocoa/element-fullscreen.html:

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (292954 => 292955)


--- trunk/Source/WebKit/ChangeLog	2022-04-18 14:33:57 UTC (rev 292954)
+++ trunk/Source/WebKit/ChangeLog	2022-04-18 16:09:17 UTC (rev 292955)
@@ -1,3 +1,37 @@
+2022-04-18  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        Make the main video heuristic robust when video elements are added after entering fullscreen
+        https://bugs.webkit.org/show_bug.cgi?id=239438
+        rdar://91867187
+
+        Reviewed by Eric Carlson.
+
+        Adjust the heuristic for identifying the most prominent video element in an element fullscreen container, such
+        that it can detect video elements that are inserted into the document after we've already entered fullscreen
+        mode. See below for more details.
+
+        Exercised by a new internal API test.
+
+        * WebProcess/FullScreen/WebFullScreenManager.cpp:
+        (WebKit::WebFullScreenManager::setElement):
+
+        Listen for several events (play, pause, loadedmetadata) that are bubbled up to the fullscreen container element,
+        from any media element underneath the fullscreen container; in response to these events, we re-run the main
+        fullscreen video element heuristic to update `m_mainVideoElement` (and cancel and reschedule the video
+        extraction timer, if this main video element changed).
+
+        (WebKit::WebFullScreenManager::enterFullScreenForElement):
+        (WebKit::WebFullScreenManager::didEnterFullScreen):
+
+        Pull existing logic for walking through the element fullscreen DOM in search of video elements out into a new
+        helper method, `updateMainVideoElement`, that's invoked both when we finish entering fullscreen mode, and when
+        observing any of the new events that bubble up to the fullscreen container.
+
+        (WebKit::WebFullScreenManager::updateMainVideoElement):
+        (WebKit::WebFullScreenManager::handleEvent):
+        (WebKit::WebFullScreenManager::setMainVideoElement):
+        * WebProcess/FullScreen/WebFullScreenManager.h:
+
 2022-04-17  Chris Dumez  <cdu...@apple.com>
 
         Leverage StringView in more places

Modified: trunk/Source/WebKit/WebProcess/FullScreen/WebFullScreenManager.cpp (292954 => 292955)


--- trunk/Source/WebKit/WebProcess/FullScreen/WebFullScreenManager.cpp	2022-04-18 14:33:57 UTC (rev 292954)
+++ trunk/Source/WebKit/WebProcess/FullScreen/WebFullScreenManager.cpp	2022-04-18 16:09:17 UTC (rev 292955)
@@ -137,6 +137,28 @@
     return m_page->injectedBundleFullScreenClient().supportsFullScreen(m_page.get(), withKeyboard);
 }
 
+void WebFullScreenManager::setElement(WebCore::Element& element)
+{
+    if (m_element == &element)
+        return;
+
+    static NeverDestroyed eventsToObserve = std::array {
+        WebCore::eventNames().playEvent,
+        WebCore::eventNames().pauseEvent,
+        WebCore::eventNames().loadedmetadataEvent,
+    };
+
+    if (m_element) {
+        for (auto& eventName : eventsToObserve.get())
+            m_element->removeEventListener(eventName, *this, { true });
+    }
+
+    m_element = &element;
+
+    for (auto& eventName : eventsToObserve.get())
+        m_element->addEventListener(eventName, *this, { true });
+}
+
 void WebFullScreenManager::enterFullScreenForElement(WebCore::Element* element)
 {
     LOG(Fullscreen, "WebFullScreenManager %p enterFullScreenForElement(%p)", this, element);
@@ -144,8 +166,9 @@
     ASSERT(element);
     if (!element)
         return;
-    m_element = element;
 
+    setElement(*element);
+
 #if PLATFORM(IOS_FAMILY) || (PLATFORM(MAC) && ENABLE(VIDEO_PRESENTATION_MODE))
     if (auto* currentPlaybackControlsElement = m_page->playbackSessionManager().currentPlaybackControlsElement())
         currentPlaybackControlsElement->prepareForVideoFullscreenStandby();
@@ -203,7 +226,18 @@
 #endif
 
 #if ENABLE(VIDEO)
+    updateMainVideoElement();
+#endif
+}
+
+#if ENABLE(VIDEO)
+
+void WebFullScreenManager::updateMainVideoElement()
+{
     setMainVideoElement([&]() -> RefPtr<WebCore::HTMLVideoElement> {
+        if (!m_element)
+            return nullptr;
+
         if (auto video = dynamicDowncast<WebCore::HTMLVideoElement>(*m_element))
             return video;
 
@@ -226,11 +260,9 @@
         }
         return mainVideo;
     }());
+}
 
-    if (m_mainVideoElement && m_mainVideoElement->paused())
-        scheduleMainVideoElementExtraction();
 #endif // ENABLE(VIDEO)
-}
 
 void WebFullScreenManager::willExitFullScreen()
 {
@@ -341,13 +373,21 @@
 void WebFullScreenManager::handleEvent(WebCore::ScriptExecutionContext& context, WebCore::Event& event)
 {
 #if ENABLE(VIDEO)
-    if (!m_mainVideoElement || event.target() != m_mainVideoElement || &context != &m_mainVideoElement->document())
+    RefPtr targetElement = dynamicDowncast<Element>(event.currentTarget());
+    if (!m_element || &context != &m_element->document() || !targetElement)
         return;
 
-    if (m_mainVideoElement->paused())
-        scheduleMainVideoElementExtraction();
-    else
-        endMainVideoElementExtractionIfNeeded();
+    if (targetElement == m_element) {
+        updateMainVideoElement();
+        return;
+    }
+
+    if (targetElement == m_mainVideoElement.get()) {
+        if (m_mainVideoElement && m_mainVideoElement->paused())
+            scheduleMainVideoElementExtraction();
+        else
+            endMainVideoElementExtractionIfNeeded();
+    }
 #else
     UNUSED_PARAM(event);
     UNUSED_PARAM(context);
@@ -388,8 +428,6 @@
     if (element == m_mainVideoElement.get())
         return;
 
-    // FIXME: We should listen for additional events (such as 'load' and 'unload') that bubble up
-    // to the fullscreen element, and recompute the main video element.
     static NeverDestroyed eventsToObserve = std::array {
         WebCore::eventNames().seekingEvent,
         WebCore::eventNames().playingEvent,
@@ -408,6 +446,9 @@
     if (m_mainVideoElement) {
         for (auto& eventName : eventsToObserve.get())
             m_mainVideoElement->addEventListener(eventName, *this, { });
+
+        if (m_mainVideoElement->paused())
+            scheduleMainVideoElementExtraction();
     }
 }
 

Modified: trunk/Source/WebKit/WebProcess/FullScreen/WebFullScreenManager.h (292954 => 292955)


--- trunk/Source/WebKit/WebProcess/FullScreen/WebFullScreenManager.h	2022-04-18 14:33:57 UTC (rev 292954)
+++ trunk/Source/WebKit/WebProcess/FullScreen/WebFullScreenManager.h	2022-04-18 16:09:17 UTC (rev 292955)
@@ -105,10 +105,13 @@
 
     void handleEvent(WebCore::ScriptExecutionContext&, WebCore::Event&) final;
 
+    void setElement(WebCore::Element&);
+
 #if ENABLE(VIDEO)
     void scheduleMainVideoElementExtraction();
     void endMainVideoElementExtractionIfNeeded();
     void mainVideoElementExtractionTimerFired();
+    void updateMainVideoElement();
     void setMainVideoElement(RefPtr<WebCore::HTMLVideoElement>&&);
 
     WeakPtr<WebCore::HTMLVideoElement> m_mainVideoElement;

Modified: trunk/Tools/ChangeLog (292954 => 292955)


--- trunk/Tools/ChangeLog	2022-04-18 14:33:57 UTC (rev 292954)
+++ trunk/Tools/ChangeLog	2022-04-18 16:09:17 UTC (rev 292955)
@@ -1,3 +1,15 @@
+2022-04-18  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        Make the main video heuristic robust when video elements are added after entering fullscreen
+        https://bugs.webkit.org/show_bug.cgi?id=239438
+        rdar://91867187
+
+        Reviewed by Eric Carlson.
+
+        Add a helper method to a test page used in API tests; see the radar for more information.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/element-fullscreen.html:
+
 2022-04-16  Chris Dumez  <cdu...@apple.com>
 
         Drop String::truncate() and use String::left() instead

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/element-fullscreen.html (292954 => 292955)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/element-fullscreen.html	2022-04-18 14:33:57 UTC (rev 292954)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/element-fullscreen.html	2022-04-18 16:09:17 UTC (rev 292955)
@@ -39,6 +39,15 @@
     });
 }
 
+async function waitForVideoFrame()
+{
+    return new Promise(resolve => {
+        video.requestVideoFrameCallback((timestamp, metadata) => {
+            resolve(timestamp);
+        });
+    });
+}
+
 function performWithUserGestureIfPossible(callback)
 {
     if (window.internals)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to