Attachment 349282 Details for Bug 189147 – Patch

[patch] Patch

bug-189147-20180908210749.patch (text/plain), 7.15 KB, created by Simon Fraser (smfr) on 2018-09-08 21:07:50 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Simon Fraser (smfr)

Created: 2018-09-08 21:07:50 PDT

Size: 7.15 KB

patch

obsolete

>Subversion Revision: 235830
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index e99fb6b1c5b9508200cd143e55b59d24777f6f1a..95f1f42128108ea0ca8c5162e8bedfdfad3d5534 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,32 @@
>+2018-09-08  Simon Fraser  <simon.fraser@apple.com>
>+
>+        svg/W3C-SVG-1.1/render-groups-03-t.svg and some other SVG tests leak documents
>+        https://bugs.webkit.org/show_bug.cgi?id=189147
>+
>+        Reviewed by NOBODY (OOPS!).
>+        
>+        Document::removedLastRef() needs to clean up m_fontSelector, because it can reference
>+        CSSFontFaceSources that keep SVGFontFaceElements alive, and they in turn will keep
>+        the Document alive.
>+        
>+        Also add the beginnings of a Fonts log channel.
>+
>+        This will be tested by world leak testing (webkit.org/b/189332).
>+
>+        * css/CSSFontSelector.cpp:
>+        (WebCore::CSSFontSelector::CSSFontSelector):
>+        (WebCore::CSSFontSelector::~CSSFontSelector):
>+        * dom/Document.cpp:
>+        (WebCore::Document::removedLastRef):
>+        * platform/Logging.h:
>+        * platform/graphics/FontCache.cpp:
>+        (WebCore::FontCache::purgeInactiveFontDataIfNeeded):
>+        (WebCore::FontCache::purgeInactiveFontData):
>+        * svg/SVGFontFaceElement.cpp:
>+        (WebCore::SVGFontFaceElement::SVGFontFaceElement):
>+        (WebCore::SVGFontFaceElement::~SVGFontFaceElement):
>+        * svg/SVGFontFaceElement.h:
>+
> 2018-09-08  Simon Fraser  <simon.fraser@apple.com>
> 
>         Make it possible to track unbalanced ref()/deref()
>diff --git a/Source/WebCore/css/CSSFontSelector.cpp b/Source/WebCore/css/CSSFontSelector.cpp
>index 9ad88a2a4df62a851786c0f34a9541d0a3cffdcc..a2e5eb2fbd7e334a53c1533cfda9a6987991642f 100644
>--- a/Source/WebCore/css/CSSFontSelector.cpp
>+++ b/Source/WebCore/css/CSSFontSelector.cpp
>@@ -45,6 +45,7 @@
> #include "FontSelectorClient.h"
> #include "Frame.h"
> #include "FrameLoader.h"
>+#include "Logging.h"
> #include "Settings.h"
> #include "StyleProperties.h"
> #include "StyleResolver.h"
>@@ -68,10 +69,13 @@ CSSFontSelector::CSSFontSelector(Document& document)
>     ASSERT(m_document);
>     FontCache::singleton().addClient(*this);
>     m_cssFontFaceSet->addClient(*this);
>+    LOG(Fonts, "CSSFontSelector %p ctor", this);
> }
> 
> CSSFontSelector::~CSSFontSelector()
> {
>+    LOG(Fonts, "CSSFontSelector %p dtor", this);
>+
>     clearDocument();
>     m_cssFontFaceSet->removeClient(*this);
>     FontCache::singleton().removeClient(*this);
>diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp
>index 55e8fdaedb93ef34400db29236dbf46887defb35..0910d69e59fc0ffbb141d9c976a9bcae28815b3c 100644
>--- a/Source/WebCore/dom/Document.cpp
>+++ b/Source/WebCore/dom/Document.cpp
>@@ -684,6 +684,9 @@ void Document::removedLastRef()
> #endif
>         m_associatedFormControls.clear();
> 
>+        m_fontSelector->clearDocument();
>+        m_fontSelector->unregisterForInvalidationCallbacks(*this);
>+
>         detachParser();
> 
>         // removeDetachedChildren() doesn't always unregister IDs,
>diff --git a/Source/WebCore/platform/Logging.h b/Source/WebCore/platform/Logging.h
>index b47cbf993991a2acad935c37fd2c55369ed656e4..390e375f7ffd68f4d7b74f01f59ef1afdcec38f1 100644
>--- a/Source/WebCore/platform/Logging.h
>+++ b/Source/WebCore/platform/Logging.h
>@@ -49,6 +49,7 @@ namespace WebCore {
>     M(Events) \
>     M(FileAPI) \
>     M(Filters) \
>+    M(Fonts) \
>     M(Frames) \
>     M(FTP) \
>     M(Fullscreen) \
>diff --git a/Source/WebCore/platform/graphics/FontCache.cpp b/Source/WebCore/platform/graphics/FontCache.cpp
>index d248f2ff8eb000843bf8876add292e9c402d2cc3..58773be73c26c3791398090bd540b3a920ad56bf 100644
>--- a/Source/WebCore/platform/graphics/FontCache.cpp
>+++ b/Source/WebCore/platform/graphics/FontCache.cpp
>@@ -33,6 +33,7 @@
> #include "FontCascade.h"
> #include "FontPlatformData.h"
> #include "FontSelector.h"
>+#include "Logging.h"
> #include "WebKitFontFamilyNames.h"
> #include <wtf/HashMap.h>
> #include <wtf/MemoryPressureHandler.h>
>@@ -350,6 +351,8 @@ void FontCache::purgeInactiveFontDataIfNeeded()
>     bool underMemoryPressure = MemoryPressureHandler::singleton().isUnderMemoryPressure();
>     unsigned inactiveFontDataLimit = underMemoryPressure ? cMaxUnderMemoryPressureInactiveFontData : cMaxInactiveFontData;
> 
>+    LOG(Fonts, "FontCache::purgeInactiveFontDataIfNeeded() - underMemoryPressure %d, inactiveFontDataLimit %u", underMemoryPressure, inactiveFontDataLimit);
>+
>     if (cachedFonts().size() < inactiveFontDataLimit)
>         return;
>     unsigned inactiveCount = inactiveFontCount();
>@@ -362,6 +365,8 @@ void FontCache::purgeInactiveFontDataIfNeeded()
> 
> void FontCache::purgeInactiveFontData(unsigned purgeCount)
> {
>+    LOG(Fonts, "FontCache::purgeInactiveFontData(%u)", purgeCount);
>+
>     pruneUnreferencedEntriesFromFontCascadeCache();
>     pruneSystemFallbackFonts();
> 
>@@ -372,6 +377,7 @@ void FontCache::purgeInactiveFontData(unsigned purgeCount)
>     while (purgeCount) {
>         Vector<RefPtr<Font>, 20> fontsToDelete;
>         for (auto& font : cachedFonts().values()) {
>+            LOG(Fonts, " trying to purge font %s (has one ref %d)", font->platformData().description().utf8().data(), font->hasOneRef());
>             if (!font->hasOneRef())
>                 continue;
>             fontsToDelete.append(WTFMove(font));
>@@ -396,6 +402,9 @@ void FontCache::purgeInactiveFontData(unsigned purgeCount)
>         if (entry.value && !cachedFonts().contains(*entry.value))
>             keysToRemove.uncheckedAppend(entry.key);
>     }
>+
>+    LOG(Fonts, " removing %lu keys", keysToRemove.size());
>+
>     for (auto& key : keysToRemove)
>         fontPlatformDataCache().remove(key);
> 
>diff --git a/Source/WebCore/svg/SVGFontFaceElement.cpp b/Source/WebCore/svg/SVGFontFaceElement.cpp
>index 90ec2697c774ec3ac9900fd3b6049b82828079df..fe3b619530d510b5572ae25a0a14cbd80b8b69f7 100644
>--- a/Source/WebCore/svg/SVGFontFaceElement.cpp
>+++ b/Source/WebCore/svg/SVGFontFaceElement.cpp
>@@ -33,6 +33,7 @@
> #include "Document.h"
> #include "ElementIterator.h"
> #include "FontCascade.h"
>+#include "Logging.h"
> #include "SVGDocumentExtensions.h"
> #include "SVGFontElement.h"
> #include "SVGFontFaceSrcElement.h"
>@@ -56,9 +57,15 @@ inline SVGFontFaceElement::SVGFontFaceElement(const QualifiedName& tagName, Docu
>     , m_fontFaceRule(StyleRuleFontFace::create(MutableStyleProperties::create(HTMLStandardMode)))
>     , m_fontElement(nullptr)
> {
>+    LOG(Fonts, "SVGFontFaceElement %p ctor", this);
>     ASSERT(hasTagName(font_faceTag));
> }
> 
>+SVGFontFaceElement::~SVGFontFaceElement()
>+{
>+    LOG(Fonts, "SVGFontFaceElement %p dtor", this);
>+}
>+
> Ref<SVGFontFaceElement> SVGFontFaceElement::create(const QualifiedName& tagName, Document& document)
> {
>     return adoptRef(*new SVGFontFaceElement(tagName, document));
>diff --git a/Source/WebCore/svg/SVGFontFaceElement.h b/Source/WebCore/svg/SVGFontFaceElement.h
>index 4d5365345b5060ecd765fd17d3bf225d008c3e3a..8f8ba5892be280798ee41dc00bd1b3cb7d3098a9 100644
>--- a/Source/WebCore/svg/SVGFontFaceElement.h
>+++ b/Source/WebCore/svg/SVGFontFaceElement.h
>@@ -55,6 +55,7 @@ public:
> 
> private:
>     SVGFontFaceElement(const QualifiedName&, Document&);
>+    ~SVGFontFaceElement();
> 
>     void parseAttribute(const QualifiedName&, const AtomicString&) final;
>

Actions: View | Formatted Diff | Diff

Attachments on bug 189147: 349276 | 349282