[Modern Media Controls] HTMLMediaElement is never destroyed when show…

…ing media controls

https://bugs.webkit.org/show_bug.cgi?id=270571

Reviewed by Xabier Rodriguez-Calvar.

At least in GStreamer-based ports (WPE and WebKitGTK, I haven't checked
on Mac ports because I don't have the proper environment easily
available), the media element is leaked after explicit deinitialization
and detaching from the HTML document, even after manually triggering
garbage collection (GC) using the web inspector.

See: #1285

After some debugging, we've detected that 2 extra references to
HTMLMediaElement appear when using the controls (3 refs in total), while
in a scenario where the controls are hidden on purpose only 1 reference
remains, which is released as soon as the GC kicks in.

Those references are held (or transitively held) by the MediaController,
the shadowRoot referenced by the MediaController and by the IconService,
and the <div> element that MediaController adds to the shadowRoot as a
container for the controls.

This commit adds code to use WeakRefs to the ShadowRoot and to the
MediaJSWrapper (media) on MediaController, and to the ShadowRoot on
the iconService, instead of the original objects. This allows the
garbage collector to kick in when needed and have those objects freed
automatically. The WeakRefs are transparently exposed as regular objects
by using properties (get/set), to avoid the need to change a lot of code
that expects regular objects.

There's still the <div> element added to the ShadowRoot, which
transitively holds a reference that prevents GC. There's no good place
to remove that element, so I removed it in the "less bad place", which
is HTMLMediaElement::pauseAfterDetachedTask(). A new deinitialize() JS
function takes care of that. Unfortunately, the element can still be
used after deinitialization, so there's also a method to reinitialize()
it if needed and an extra ControlsState to mark the element as
PartiallyDeinitialized in order to know when to recover from that state.

* Source/WebCore/Modules/modern-media-controls/controls/icon-service.js: Store shadowRoot as a WeakRef.
(const.iconService.new.IconService.prototype.get shadowRoot): Expose the shadowRootWeakRef as a regular shadowRoot object by binding it to the shadowRoot property.
(const.iconService.new.IconService.prototype.set shadowRoot): Ditto, but for the setter.
* Source/WebCore/Modules/modern-media-controls/media/media-controller.js:
(MediaController): Store shadowRoot and media as WeakRefs.
(MediaController.prototype.get media): Expose the mediaWeakRef as a regular media object by binding it to the media property.
(MediaController.prototype.get shadowRoot): Expose the shadowRootWeakRef as a regular shadowRoot object by binding it to the shadowRoot property.
(MediaController.prototype.get isFullscreen): Take into account the case where media can be null.
(MediaController.prototype.deinitialize): Function called from HTMLMediaElement to deinitialize the MediaController. Just removes the shadowRoot child.
(MediaController.prototype.reinitialize): Function called from HTMLMediaElement to reinitialize the MediaController. Readds the shadowRoot child and sets again the WeakRefs that might have become by lack of use of the main objects.
* Source/WebCore/html/HTMLMediaElement.cpp:
(WebCore::convertEnumerationToString): Utility function to get a printable version of ControlsState. Useful for debugging.
(WebCore::HTMLMediaElement::pauseAfterDetachedTask): Deinitialize the MediaController by calling its deinitialize() JS method.
(WebCore::HTMLMediaElement::ensureMediaControls): Support the case of reinitialization. Call the reinitialize() JS method in MediaController in that case.
* Source/WebCore/html/HTMLMediaElement.h: Added new PartiallyDeinitialized state to ControlsState. Give friend access to convertEnumerationToString() so it can do its job.

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

Source/WebCore/Modules/modern-media-controls/controls/icon-service.js

@@ -74,6 +74,15 @@ const iconService = new class IconService {
     }
 
     // Public
+    get shadowRoot()
+    {
+        return this.shadowRootWeakRef ? this.shadowRootWeakRef.deref() : null;
+    }
+
+    set shadowRoot(shadowRoot)
+    {
+        this.shadowRootWeakRef = new WeakRef(shadowRoot);
+    }
 
     imageForIconAndLayoutTraits(icon, layoutTraits)
     {

Source/WebCore/Modules/modern-media-controls/media/media-controller.js

@@ -25,11 +25,10 @@
 
 class MediaController
 {
-
     constructor(shadowRoot, media, host)
     {
-        this.shadowRoot = shadowRoot;
-        this.media = media;
+        this.shadowRootWeakRef = new WeakRef(shadowRoot);
+        this.mediaWeakRef = new WeakRef(media);
         this.host = host;
 
         this.fullscreenChangeEventType = media.webkitSupportsPresentationMode ? "webkitpresentationmodechanged" : "webkitfullscreenchange";
@@ -65,6 +64,16 @@ class MediaController
     }
 
     // Public
+    get media()
+    {
+        return this.mediaWeakRef ? this.mediaWeakRef.deref() : null;
+    }
+
+    get shadowRoot()
+    {
+
+        return this.shadowRootWeakRef ? this.shadowRootWeakRef.deref() : null;
+    }
 
     get isAudio()
     {
@@ -91,6 +100,9 @@ class MediaController
 
     get isFullscreen()
     {
+        if (!this.media)
+            return false;
+
         return this.media.webkitSupportsPresentationMode ? this.media.webkitPresentationMode === "fullscreen" : this.media.webkitDisplayingFullscreen;
     }
 
@@ -205,6 +217,24 @@ class MediaController
         }
     }
 
+    // HTMLMediaElement
+
+    deinitialize()
+    {
+        this.shadowRoot.removeChild(this.container);
+        return true;
+    }
+
+    reinitialize(shadowRoot, media, host)
+    {
+        iconService.shadowRoot = shadowRoot;
+        this.shadowRootWeakRef = new WeakRef(shadowRoot);
+        this.mediaWeakRef = new WeakRef(media);
+        this.host = host;
+        shadowRoot.appendChild(this.container);
+        return true;
+    }
+
     // Private
 
     _supportingObjectClasses()

Source/WebCore/html/HTMLMediaElement.cpp

@@ -294,6 +294,8 @@ String convertEnumerationToString(HTMLMediaElement::TextTrackVisibilityCheckType
     return values[static_cast<size_t>(enumerationValue)];
 }
 
+static JSC::JSValue controllerJSValue(JSC::JSGlobalObject& lexicalGlobalObject, JSDOMGlobalObject&, HTMLMediaElement&);
+
 class TrackDisplayUpdateScope {
 public:
     TrackDisplayUpdateScope(HTMLMediaElement& element)
@@ -444,6 +446,7 @@ HTMLMediaElement::HTMLMediaElement(const QualifiedName& tagName, Document& docum
     , m_parsingInProgress(createdByParser)
     , m_elementIsHidden(document.hidden())
     , m_creatingControls(false)
+    , m_partiallyDeinitialized(false)
     , m_receivedLayoutSizeChanged(false)
     , m_hasEverNotifiedAboutPlaying(false)
     , m_hasEverHadAudio(false)
@@ -877,6 +880,37 @@ void HTMLMediaElement::pauseAfterDetachedTask()
     if (m_videoFullscreenMode == VideoFullscreenModeStandard)
         exitFullscreen();
 
+#if ENABLE(MODERN_MEDIA_CONTROLS)
+    if (!m_creatingControls && !m_partiallyDeinitialized && m_mediaControlsHost) {
+        // Call MediaController.deinitialize() to get rid of circular references.
+        m_partiallyDeinitialized = setupAndCallJS([this](JSDOMGlobalObject& globalObject, JSC::JSGlobalObject& lexicalGlobalObject, ScriptController&, DOMWrapperWorld&) {
+            auto& vm = globalObject.vm();
+            auto scope = DECLARE_THROW_SCOPE(vm);
+
+            auto controllerValue = controllerJSValue(lexicalGlobalObject, globalObject, *this);
+            RETURN_IF_EXCEPTION(scope, false);
+            auto* controllerObject = controllerValue.toObject(&lexicalGlobalObject);
+            RETURN_IF_EXCEPTION(scope, false);
+
+            auto functionValue = controllerObject->get(&lexicalGlobalObject, JSC::Identifier::fromString(vm, "deinitialize"_s));
+            if (UNLIKELY(scope.exception()) || functionValue.isUndefinedOrNull())
+                return false;
+
+            auto* function = functionValue.toObject(&lexicalGlobalObject);
+            RETURN_IF_EXCEPTION(scope, false);
+
+            auto callData = JSC::getCallData(function);
+            if (callData.type == JSC::CallData::Type::None)
+                return false;
+
+            auto resultValue = JSC::call(&lexicalGlobalObject, function, callData, controllerObject, JSC::MarkedArgumentBuffer());
+            RETURN_IF_EXCEPTION(scope, false);
+
+            return resultValue.toBoolean(&lexicalGlobalObject);
+        });
+    }
+#endif // ENABLE(MODERN_MEDIA_CONTROLS)
+
     if (!m_player)
         return;
 
@@ -4642,6 +4676,46 @@ void HTMLMediaElement::ensureMediaControlsShadowRoot()
     if (m_creatingControls)
         return;
 
+    if (m_partiallyDeinitialized) {
+        m_partiallyDeinitialized = !setupAndCallJS([this](JSDOMGlobalObject& globalObject, JSC::JSGlobalObject& lexicalGlobalObject, ScriptController&, DOMWrapperWorld&) {
+            auto& vm = globalObject.vm();
+            auto scope = DECLARE_THROW_SCOPE(vm);
+
+            auto controllerValue = controllerJSValue(lexicalGlobalObject, globalObject, *this);
+            RETURN_IF_EXCEPTION(scope, false);
+            auto* controllerObject = controllerValue.toObject(&lexicalGlobalObject);
+            RETURN_IF_EXCEPTION(scope, false);
+
+            auto functionValue = controllerObject->get(&lexicalGlobalObject, JSC::Identifier::fromString(vm, "reinitialize"_s));
+            if (UNLIKELY(scope.exception()) || functionValue.isUndefinedOrNull())
+                return false;
+
+            if (!m_mediaControlsHost)
+                m_mediaControlsHost = MediaControlsHost::create(*this);
+
+            auto mediaJSWrapper = toJS(&lexicalGlobalObject, &globalObject, *this);
+            auto mediaControlsHostJSWrapper = toJS(&lexicalGlobalObject, &globalObject, *m_mediaControlsHost.copyRef());
+
+            JSC::MarkedArgumentBuffer argList;
+            argList.append(toJS(&lexicalGlobalObject, &globalObject, Ref { ensureUserAgentShadowRoot() }));
+            argList.append(mediaJSWrapper);
+            argList.append(mediaControlsHostJSWrapper);
+            ASSERT(!argList.hasOverflowed());
+
+            auto* function = functionValue.toObject(&lexicalGlobalObject);
+            RETURN_IF_EXCEPTION(scope, false);
+
+            auto callData = JSC::getCallData(function);
+            if (callData.type == JSC::CallData::Type::None)
+                return false;
+
+            auto resultValue = JSC::call(&lexicalGlobalObject, function, callData, controllerObject, argList);
+            RETURN_IF_EXCEPTION(scope, false);
+
+            return resultValue.toBoolean(&lexicalGlobalObject);
+        });
+    }
+
     m_creatingControls = true;
     ensureUserAgentShadowRoot();
     m_creatingControls = false;
@@ -7668,6 +7742,9 @@ bool HTMLMediaElement::ensureMediaControlsInjectedScript()
     if (mediaControlsScripts.isEmpty())
         return false;
 
+    if (m_partiallyDeinitialized)
+        return true;
+
     return setupAndCallJS([mediaControlsScripts = WTFMove(mediaControlsScripts)](JSDOMGlobalObject& globalObject, JSC::JSGlobalObject& lexicalGlobalObject, ScriptController& scriptController, DOMWrapperWorld& world) {
         auto& vm = globalObject.vm();
         auto scope = DECLARE_CATCH_SCOPE(vm);

Source/WebCore/html/HTMLMediaElement.h

@@ -1141,6 +1141,7 @@ class HTMLMediaElement
     bool m_elementIsHidden : 1;
     bool m_elementWasRemovedFromDOM : 1;
     bool m_creatingControls : 1;
+    bool m_partiallyDeinitialized : 1;
     bool m_receivedLayoutSizeChanged : 1;
     bool m_hasEverNotifiedAboutPlaying : 1;