WebPlatformForEmbedded / WPEWebKit

WPE WebKit port (downstream)
210 stars 135 forks source link

[wpe-2.38] <video> element is never destoyed when using modern media controls #1285

Closed asurdej-comcast closed 3 months ago

asurdej-comcast commented 4 months ago

We observe increased memory consumption for wpe webkit 2.38 (comparing to 2.28) when running YT cert tests. It sometimes leads to OOM killer as WPEWebProcess uses ~550MB of memory despite much lower memory pressure settings.

YT creates separete \<video> element for each test. Initial analysis shows that HTMLMediaElement (s) are never destroyed during execution. Also manually triggering GarbageCollector from RWI doesn't help, nothing is destoryd. Together with HTMLMediaElement there are many other objects that are connected, like SourceBuffers, MediaKeys, etc. As a result memory usage is growing constantly triggering OOM eventually.

Looking into JS sources it looks like enabling video controls attribute is the reason here. When media controls are disabled then issue is not observed. With 2.38 media controls impl changed -> modern media controls was enabled that seems to be the reason here.

Here is a javascript code that reproduces the issue. I use "collect garbage" button from RWI to trigger GC and check if ~HTMLMediaElement() is called:

var vid;
var ms;

function test() {
    vid = document.createElement("video");
    document.body.appendChild(vid);

    vid.controls = true;

    ms = new MediaSource;
    ms.addEventListener('sourceopen', function(e) {
        vid.pause();
        URL.revokeObjectURL(vid.src);
        vid.src = '';
        vid.removeAttribute('src');
        vid.load();
        vid.parentNode.removeChild(vid);

        vid = null;
        ms = null;
    });
    vid.src = URL.createObjectURL(ms);
}

Note that skipping video.controls(true) or not attaching video element into document's body works perfectly fine.

eocanha commented 4 months ago

Fix patch submitted upstream for review as https://bugs.webkit.org/show_bug.cgi?id=270571 / https://github.com/WebKit/WebKit/pull/25625.

eocanha commented 3 months ago

The patch landed upstream as https://github.com/WebKit/WebKit/commit/80dd62a7e3323e4c039cac722ec4c563c92a17ff and was backported to wpe-2.38 as https://github.com/WebPlatformForEmbedded/WPEWebKit/commit/78779020f015db20ed65df9bf0a178d1c3cfefc0.

eocanha commented 3 months ago

...and to the calvaris/wpe-2.42/upstream branch as https://github.com/WebPlatformForEmbedded/WPEWebKit/commit/6de0ae15e6e7418410660ecf4da5d29269d57668. Closing Issue.

asurdej-comcast commented 3 months ago

Tested with the latest webkit build and all my cases work fine now. HTMLMediaElement is correctly destroyed. Also YT cert tests look much better now. Thanks @eocanha