[webkit-changes] [WebKit/WebKit] 51b275: Simplify promise and completion handler management...

[Alex Christensen]

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 51b275a9492afcba8ee8f146ca80bbbd9ecaac77
      https://github.com/WebKit/WebKit/commit/51b275a9492afcba8ee8f146ca80bbbd9ecaac77
  Author: Alex Christensen <achristensen at apple.com>
  Date:   2025-02-12 (Wed, 12 Feb 2025)

  Changed paths:
    M Source/WebCore/dom/DocumentFullscreen.cpp
    M Source/WebCore/dom/Element.cpp
    M Source/WebCore/dom/FullscreenManager.cpp
    M Source/WebCore/dom/FullscreenManager.h
    M Source/WebCore/html/HTMLMediaElement.cpp
    M Source/WebCore/page/ChromeClient.h
    M Source/WebKit/WebProcess/FullScreen/WebFullScreenManager.cpp
    M Source/WebKit/WebProcess/FullScreen/WebFullScreenManager.h
    M Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp
    M Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h
    M Source/WebKitLegacy/mac/WebCoreSupport/WebChromeClient.h
    M Source/WebKitLegacy/mac/WebCoreSupport/WebChromeClient.mm

  Log Message:
  -----------
  Simplify promise and completion handler management in FullscreenManager
https://bugs.webkit.org/show_bug.cgi?id=287531
rdar://144654454

Reviewed by Jer Noble.

This simplifies logic flow in several tangled pieces of code.

The first simplification is in FullscreenManager::requestFullscreenForElement which used to
take an optional DeferredPromise and an optional CompletionHandler<void(bool)>.
To fix this, I moved the DeferredPromise handling to the one call site that
provided it, Element::requestFullscreen. I changed the CompletionHandler<void(bool)>
to take an ExceptionOr<void> instead of a bool to have enough information to reject
the promise with the right exception when that happens.  To retain the current
behavior of immediately rejecting the promise when another request or exit
fullscreen call happens, I continue to store a promise wrapper on the FullscreenManager,
but I store it as a CompletionHandler instead of a DeferredPromise.  Because of this,
I needed to do a similar transformation to DocumentFullscreen::exitFullscreen.
Now, they both store the CompletionHandler in m_pendingPromise and keep a WeakPtr
that will resolve the promise if it hasn't been resolved by the time the async
task is complete.  I introduced FullscreenPromise for this purpose.

In order to do this first simplification, I needed to change the time at which
the completion handler is called in the success case.  It used to be called immediately
after the call to page->chrome().client().enterFullScreenForElement(element, mode)
and now it is called after the reply to Messages::WebFullScreenManagerProxy::EnterFullScreen
is received.  This is not a change in behavior because the only callers of
FullscreenManager::requestFullscreenForElement which passed in a CompletionHandler were
HTMLMediaElement::enterFullscreen which only did anything in the failure case and
WebFullScreenManager::requestRestoreFullScreen which was only called from IPC from
WKFullScreenWindowController.requestRestoreFullScreen: which also only did anything
in the failure case, so changing the time at which the success case called its completion
handler doesn't change any behavior.

The second simplification is that FullscreenManager::willEnterFullscreen used to
return a bool and resolve or reject the stored promise.  To make this responsibility
more clear and avoid mistakes of forgetting one or the other, I pass in the completion
handler and that will take care of both when called.

* Source/WebCore/dom/DocumentFullscreen.cpp:
(WebCore::DocumentFullscreen::exitFullscreen):
* Source/WebCore/dom/Element.cpp:
(WebCore::Element::requestFullscreen):
* Source/WebCore/dom/FullscreenManager.cpp:
(WebCore::FullscreenManager::~FullscreenManager):
(WebCore::FullscreenManager::requestFullscreenForElement):
(WebCore::FullscreenManager::cancelFullscreen):
(WebCore::FullscreenManager::exitFullscreen):
(WebCore::FullscreenManager::willEnterFullscreen):
(WebCore::FullscreenManager::resolvePendingPromise):
(WebCore::FullscreenManager::rejectPendingPromise):
(WebCore::FullscreenManager::clear):
* Source/WebCore/dom/FullscreenManager.h:
* Source/WebCore/html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::enterFullscreen):
* Source/WebCore/page/ChromeClient.h:
(WebCore::ChromeClient::enterFullScreenForElement):
* Source/WebKit/WebProcess/FullScreen/WebFullScreenManager.cpp:
(WebKit::WebFullScreenManager::enterFullScreenForElement):
(WebKit::WebFullScreenManager::willEnterFullScreen):
(WebKit::WebFullScreenManager::requestRestoreFullScreen):
* Source/WebKit/WebProcess/FullScreen/WebFullScreenManager.h:
* Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp:
(WebKit::WebChromeClient::enterFullScreenForElement):
* Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h:
* Source/WebKitLegacy/mac/WebCoreSupport/WebChromeClient.h:
* Source/WebKitLegacy/mac/WebCoreSupport/WebChromeClient.mm:
(WebChromeClient::enterFullScreenForElement):

Canonical link: https://commits.webkit.org/290289@main



To unsubscribe from these emails, change your notification settings at https://github.com/WebKit/WebKit/settings/notifications