These and other media tests leak documents: media/modern-media-controls/controls-visibility-support/controls-visibility-support-controls-off-audio.html [ Leak Pass ] media/modern-media-controls/controls-visibility-support/controls-visibility-support-controls-on-audio.html [ Leak Pass ] media/modern-media-controls/controls-visibility-support/controls-visibility-support-fullscreen-on-parent-element.html [ Leak Pass ] media/modern-media-controls/css/pointer-events-none.html [ Leak ] media/modern-media-controls/css/webkit-cursor-visibility-auto-hide.html [ Leak ] media/modern-media-controls/fullscreen-support/fullscreen-support-disabled-video-with-audio-tracks-only.html [ Leak ] media/modern-media-controls/fullscreen-support/fullscreen-support-press.html [ Leak ] media/modern-media-controls/macos-fullscreen-media-controls/macos-fullscreen-media-controls-drag-is-prevented-over-button.html [ Leak ] media/modern-media-controls/macos-fullscreen-media-controls/macos-fullscreen-media-controls-drag.html [ Leak ] media/modern-media-controls/media-controller/media-controller-click-on-video-background-should-pause-fullscreen.html [ Leak Pass ] media/modern-media-controls/media-controller/media-controller-click-on-video-background-to-dismiss-tracks-panel-should-not-toggle-playback.html [ Leak Pass ] media/modern-media-controls/media-controller/media-controller-fullscreen-ltr.html [ Leak Pass ] media/modern-media-controls/media-controller/media-controller-space-bar-toggle-playback.html [ Leak ] media/modern-media-controls/placard-support/placard-support-airplay-fullscreen.html [ Leak ] media/modern-media-controls/placard-support/placard-support-pip.html [ Leak ] media/modern-media-controls/seek-forward-support/seek-forward-support.html [ Leak ] media/modern-media-controls/start-support/start-support-audio.html [ Leak Pass ] media/modern-media-controls/start-support/start-support-click-to-start.html [ Leak ] media/modern-media-controls/tracks-panel/tracks-panel-controls-bar-remains-visible-after-clicking-over-it.html [ Leak ] media/modern-media-controls/tracks-support/tracks-support-audio-tracks.html [ Leak Pass ] media/modern-media-controls/tracks-support/tracks-support-auto-text-track.html [ Leak Pass ] media/modern-media-controls/tracks-support/tracks-support-show-panel-fullscreen.html [ Leak ] media/modern-media-controls/volume-down-support/volume-down-support.html [ Leak ] media/modern-media-controls/volume-up-support/volume-up-support.html [ Leak ] media/W3C/video/events/event_canplay.html [ Leak Pass ] media/W3C/video/events/event_canplaythrough.html [ Leak Pass ] media/W3C/video/events/event_progress.html [ Leak Pass ]
<rdar://problem/44241903>
This is about SVGImages in the memory cache, which aren't cleared because they still have clients when we try to clear the cache (possibly because resources are still loading).
These seem to be about the lifetime of the HTMLImageElements, which sometimes extends to after we've cleared caches and fetched live documents. Those HTMLImageElements keep the ImageLoader alive, which is a client of the CachedImage.
There are a couple of variants of this. Sometimes, the main document has live references, which seem to come from a Strong Handled pointing to the Window object. Sometimes there are two HTMLImageElements which outlive the GC/clear caches, and they keep the document alive. They are usually one of the SVG data URI images, and modern-media-controls/images/macOS/Pause.svg
We're racing with ImageLoader::m_derefElementTimer.
Fixed one issue by waiting on a zero-delay timer before the GC. But tests still leak; some lifetime extension of CachedImages that I haven't figure out yet.
Created attachment 349851 [details] Patch to use RefTracker for tracking CachedResourceHandles on CachedResource
To fix all the SVGImage leaks, we have to: * fire a zero-delay timer * call document->cachedResourceLoader().garbageCollectDocumentResources() before the final GC and live document count.
Some fullscreen tests seems to leak media elements via WebFullScreenManager::enterFullScreenForElement(WebCore::Element*)
Here's some logging for running media/modern-media-controls/css/webkit-cursor-visibility-auto-hide.html twice in a row: WebFullScreenManager 0x6d22a3540 ctor WebFullScreenManager 0x6d22a3540 enterFullScreenForElement(0x6cb000990) WebFullScreenManager 0x6d22a3540 willEnterFullScreen() - element 0x6cb000990 Testing that control are shown in fullscreen when the controls attribute is not present. PASS media.webkitDisplayingFullscreen is false PASS media.webkitDisplayingFullscreen is true WebFullScreenManager 0x6d22a3540 enterFullScreenForElement(0x6cf000990) WebFullScreenManager 0x6d22a3540 willEnterFullScreen() - element 0x6cf000990 Testing that control are shown in fullscreen when the controls attribute is not present. PASS media.webkitDisplayingFullscreen is false PASS media.webkitDisplayingFullscreen is true Jeremy, how is it that WTR can navigate away from webkit-cursor-visibility-auto-hide.html to about:blank but not close the fullscreen?
Filed bug 189648 on the WebFullscreenManager leaks.
Created attachment 349860 [details] Patch
Attachment 349860 [details] did not pass style-queue: ERROR: Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp:639: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp:654: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 3 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 349860 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=349860&action=review > Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp:637 > + class TimerOwner { Would RunLoop::main().dispatch work for this?
(In reply to Alex Christensen from comment #14) > Comment on attachment 349860 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=349860&action=review > > > Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp:637 > > + class TimerOwner { > > Would RunLoop::main().dispatch work for this? RunLoop::main().dispatch() uses a CFRunLoopSource, and I don't think that is guaranteed to run after timesrs. I wanted a WebCore timer to ensure that it would hop the Timer used by ImageLoader.
https://trac.webkit.org/changeset/236100/webkit