Bug 188729

Summary: svg/custom/glyph-selection-bidi-mirror.svg and other SVG tests abandon their document because of references via SVGFontFaceElement
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: SVGAssignee: Nobody <webkit-unassigned>
Status: REOPENED ---    
Severity: Normal CC: ap, Lawrence.j, rackler, simon.fraser, webkit-bot-watchers-bugzilla, webkit-bug-importer, zimmermann
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Update TestExpectations none

Description Simon Fraser (smfr) 2018-08-18 17:12:06 PDT
svg/custom/glyph-selection-bidi-mirror.svg and other SVG tests abandon their document (bug 186214). Ref stacks on the C++ side seem reasonable, so this looks like an issue on the GC side.
Comment 1 Simon Fraser (smfr) 2018-08-20 21:30:55 PDT
svg/wicd/test-rightsizing-b.xhtml abandons 4 instances of svg/wicd/resources/test-svg-child-object-rightsizing.svg. They are retained through a referencing element, the <font-face> element:

<font-face font-family="Sera Sans"
    units-per-em="2048"
    panose-1="2 11 6 3 3 8 4 2 2 4"
    ascent="1901"
    descent="-483"
    alphabetic="0" />
Comment 2 Simon Fraser (smfr) 2018-08-20 21:31:05 PDT
*** Bug 188776 has been marked as a duplicate of this bug. ***
Comment 3 Simon Fraser (smfr) 2018-08-29 21:15:41 PDT
I think these tests all suffer in the same way:

svg/W3C-SVG-1.1-SE/color-prop-05-t.svg
svg/W3C-SVG-1.1-SE/painting-control-04-f.svg
svg/W3C-SVG-1.1/animate-elem-03-t.svg
svg/W3C-SVG-1.1/animate-elem-24-t.svg
svg/W3C-SVG-1.1/animate-elem-36-t.svg
svg/W3C-SVG-1.1/animate-elem-40-t.svg
svg/W3C-SVG-1.1/fonts-desc-02-t.svg
svg/W3C-SVG-1.1/fonts-elem-01-t.svg
svg/W3C-SVG-1.1/fonts-elem-02-t.svg
svg/W3C-SVG-1.1/fonts-elem-04-b.svg
svg/W3C-SVG-1.1/fonts-elem-05-t.svg
svg/W3C-SVG-1.1/fonts-elem-06-t.svg
svg/W3C-SVG-1.1/fonts-glyph-02-t.svg
svg/W3C-SVG-1.1/fonts-glyph-03-t.svg
svg/W3C-SVG-1.1/fonts-kern-01-t.svg
svg/W3C-SVG-1.1/masking-mask-01-b.svg
svg/W3C-SVG-1.1/pservers-grad-08-b.svg
svg/W3C-SVG-1.1/render-elems-06-t.svg
svg/W3C-SVG-1.1/render-elems-07-t.svg
svg/W3C-SVG-1.1/render-elems-08-t.svg
svg/W3C-SVG-1.1/render-groups-01-b.svg
svg/W3C-SVG-1.1/render-groups-03-t.svg
svg/W3C-SVG-1.1/text-align-08-b.svg
svg/W3C-SVG-1.1/text-altglyph-01-b.svg
svg/W3C-SVG-1.1/text-fonts-03-t.svg
svg/W3C-SVG-1.1/text-intro-01-t.svg
svg/W3C-SVG-1.1/text-intro-02-b.svg
svg/W3C-SVG-1.1/text-intro-03-b.svg
svg/W3C-SVG-1.1/text-intro-04-t.svg
svg/W3C-SVG-1.1/text-text-04-t.svg
svg/W3C-SVG-1.1/text-text-05-t.svg
svg/W3C-SVG-1.1/text-text-06-t.svg
Comment 4 Simon Fraser (smfr) 2018-08-29 21:22:16 PDT
In MiniBrowser WK2, we correctly destroy the two SVGFontFaceElements and one CachedSVGFont that are created for svg/W3C-SVG-1.1-SE/color-prop-05-t.svg.

However, in WTR, we only destroy one of those SVGFontFaceElements and never the CachedSVGFont, which keeps a Document alive.
Comment 5 Simon Fraser (smfr) 2018-08-29 22:16:12 PDT
To fix this we need to clear font caches, something like:

diff --git a/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundle.cpp b/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundle.cpp
index 1a32c605f5f2b713b6430309e1dee833318311ee..4d22a40e70a19ed493fb8c9d7a2760d33af3f21d 100644
--- a/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundle.cpp
+++ b/Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundle.cpp
@@ -45,6 +45,7 @@
 #include "WebPageGroupProxy.h"
 #include <WebCore/DatabaseTracker.h>
 #include <WebCore/MemoryCache.h>
+#include <WebCore/MemoryRelease.h>
 #include <WebCore/PageCache.h>
 #include <WebCore/ResourceLoadObserver.h>
 #include <WebCore/ServiceWorkerThreadProxy.h>
@@ -268,6 +269,7 @@ void WKBundleClearPageCache(WKBundleRef bundle)
 void WKBundleClearMemoryCache(WKBundleRef bundle)
 {
     MemoryCache::singleton().evictResources();
+    WebCore::releaseMemory(WTF::Critical::Yes, WTF::Synchronous::Yes); // Some redundant work here.
 }
 
 WKDataRef WKBundleCreateWKDataFromUInt8Array(WKBundleRef bundle, JSContextRef context, JSValueRef data)
Comment 8 Karl Rackler 2020-06-29 15:42:03 PDT
svg/W3C-SVG-1.1/fonts-elem-01-t.svg and svg/W3C-SVG-1.1/fonts-elem-02-t.svg and svg/W3C-SVG-1.1/fonts-elem-03-b.svg and svg/W3C-SVG-1.1/fonts-elem-07-b.svg is no longer failing - remove expectations

Current history is green: https://results.webkit.org/?suite=layout-tests&suite=layout-tests&suite=layout-tests&suite=layout-tests&test=svg%2FW3C-SVG-1.1%2Ffonts-elem-01-t.svg&test=svg%2FW3C-SVG-1.1%2Ffonts-elem-02-t.svg&test=svg%2FW3C-SVG-1.1%2Ffonts-elem-03-b.svg&test=svg%2FW3C-SVG-1.1%2Ffonts-elem-07-b.svg

These tests were isolated from the others in the group as ones that are unexpectedly passing.
Comment 9 Karl Rackler 2020-06-29 15:45:53 PDT
Created attachment 403126 [details]
Update TestExpectations
Comment 10 Radar WebKit Bug Importer 2020-06-29 15:47:14 PDT
<rdar://problem/64911473>
Comment 11 EWS 2020-06-29 16:31:51 PDT
Committed r263706: <https://trac.webkit.org/changeset/263706>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 403126 [details].
Comment 12 Ryan Haddad 2020-06-29 16:42:07 PDT
Reopening since some of the tests are still failing.