Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: 80dd62a7e3323e4c039cac722ec4c563c92a17ff
https://github.com/WebKit/WebKit/commit/80dd62a7e3323e4c039cac722ec4c563c92a17ff
Author: Enrique Ocaña González <[email protected]>
Date: 2024-04-08 (Mon, 08 Apr 2024)
Changed paths:
M Source/WebCore/Modules/modern-media-controls/controls/icon-service.js
M Source/WebCore/Modules/modern-media-controls/media/media-controller.js
M Source/WebCore/html/HTMLMediaElement.cpp
M Source/WebCore/html/HTMLMediaElement.h
Log Message:
-----------
[Modern Media Controls] HTMLMediaElement is never destroyed when showing
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: https://github.com/WebPlatformForEmbedded/WPEWebKit/issues/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
To unsubscribe from these emails, change your notification settings at
https://github.com/WebKit/WebKit/settings/notifications
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
