[webkit-reviews] review denied: [Bug 189722] Fix crash under FontCache::purgeInactiveFontData() when a memory warning fires : [Attachment 350073] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Sep 18 23:45:48 PDT 2018
Myles C. Maxfield <mmaxfield at apple.com> has denied Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 189722: Fix crash under FontCache::purgeInactiveFontData() when a memory
warning fires
https://bugs.webkit.org/show_bug.cgi?id=189722
Attachment 350073: Patch
https://bugs.webkit.org/attachment.cgi?id=350073&action=review
--- Comment #3 from Myles C. Maxfield <mmaxfield at apple.com> ---
Comment on attachment 350073
--> https://bugs.webkit.org/attachment.cgi?id=350073
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=350073&action=review
> Source/WebCore/platform/graphics/FontCache.cpp:379
> - for (auto& font : cachedFonts().values()) {
> + for (auto font : cachedFonts().values()) {
Right, I see what's happening here. cachedFonts().remove(font->platformData());
is removing the wrong item. This null deref means that if we follow the RefPtr
in the cachedFonts() cache to a Font object, and we look up that Font’s inner
FontPlatformData, we don’t end up with the same entry in cachedFonts() than we
started with. This should be impossible because we maintain the invariant that
this cycle should hold.
Copying the RefPtr is the wrong thing to do because it means that the
cachedFonts().remove() call won't actually remove the font, which is contrary
to the whole point of this function. This is papering over the problem rather
than actually fixing it.
More information about the webkit-reviews
mailing list