Diff
Modified: trunk/LayoutTests/ChangeLog (127919 => 127920)
--- trunk/LayoutTests/ChangeLog 2012-09-07 21:16:12 UTC (rev 127919)
+++ trunk/LayoutTests/ChangeLog 2012-09-07 21:17:50 UTC (rev 127920)
@@ -1,3 +1,19 @@
+2012-09-07 Stephen Chenney <schen...@chromium.org>
+
+ Font data is purged while fonts are still using it
+ https://bugs.webkit.org/show_bug.cgi?id=93640
+
+ Reviewed by Eric Seidel.
+
+ Tests for font purging. The seamless-custom-font-pruning-crash test
+ was only failing in Chromium Asan, while the seamless-nested-crash case
+ was only failing in Asan DumpRenderTree.
+
+ * fast/frames/seamless/seamless-custom-font-pruning-crash-expected.txt: Added.
+ * fast/frames/seamless/seamless-custom-font-pruning-crash.html: Added.
+ * fast/frames/seamless/seamless-nested-crash-expected.txt: Added.
+ * fast/frames/seamless/seamless-nested-crash.html: Added.
+
2012-09-07 Ojan Vafai <o...@chromium.org>
Fix linter errors. The deleted lines are all tests that are passing
@@ -1372,7 +1388,7 @@
Use the CSS mix function with the updated color-fill.fs.
* css3/filters/custom/effect-custom-combined-missing-expected.png:
* css3/filters/custom/effect-custom-combined-missing.html:
- Use the CSS mix function with the updated color-offset.fs.
+ Use the CSS mix function with the updated color-offset.fs.
* css3/filters/custom/effect-custom-expected.png:
* css3/filters/custom/effect-custom-parameters-expected.png:
* css3/filters/custom/effect-custom-parameters.html:
Added: trunk/LayoutTests/fast/frames/seamless/seamless-custom-font-pruning-crash-expected.txt (0 => 127920)
--- trunk/LayoutTests/fast/frames/seamless/seamless-custom-font-pruning-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/frames/seamless/seamless-custom-font-pruning-crash-expected.txt 2012-09-07 21:17:50 UTC (rev 127920)
@@ -0,0 +1 @@
+This test passes if it does not crash. This test passes if it does not crash.
Added: trunk/LayoutTests/fast/frames/seamless/seamless-custom-font-pruning-crash.html (0 => 127920)
--- trunk/LayoutTests/fast/frames/seamless/seamless-custom-font-pruning-crash.html (rev 0)
+++ trunk/LayoutTests/fast/frames/seamless/seamless-custom-font-pruning-crash.html 2012-09-07 21:17:50 UTC (rev 127920)
@@ -0,0 +1,33 @@
+<html>
+ This test passes if it does not crash.
+ <style>@font-face {
+ font-family: "Times New Roman";
+ src: local("Arial")
+ </style>
+ <script>
+ if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+ }
+ var docElement = document.body ? document.body : document.documentElement;
+
+ function initTest() {
+ iframeElement = document.createElementNS("http://www.w3.org/1999/xhtml", "iframe");
+ iframeElement.setAttribute("seamless", "");
+ iframeElement.setAttribute("srcdoc", "This test passes if it does not crash. <iframe seamless sandbox='allow-scripts' srcdoc='This test passes if it does not crash.'");
+ docElement.appendChild(iframeElement);
+
+ textElement = document.createTextNode("This test passes if it does not crash.");
+ docElement.appendChild(textElement);
+ setTimeout("crash()", 0);
+ }
+
+ document.addEventListener("DOMContentLoaded", initTest, false);
+
+ function crash() {
+ docElement.appendChild(iframeElement);
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }
+ </script>
+</html>
\ No newline at end of file
Added: trunk/LayoutTests/fast/frames/seamless/seamless-nested-crash-expected.txt (0 => 127920)
--- trunk/LayoutTests/fast/frames/seamless/seamless-nested-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/frames/seamless/seamless-nested-crash-expected.txt 2012-09-07 21:17:50 UTC (rev 127920)
@@ -0,0 +1 @@
+This test passes if it does not crash.
Added: trunk/LayoutTests/fast/frames/seamless/seamless-nested-crash.html (0 => 127920)
--- trunk/LayoutTests/fast/frames/seamless/seamless-nested-crash.html (rev 0)
+++ trunk/LayoutTests/fast/frames/seamless/seamless-nested-crash.html 2012-09-07 21:17:50 UTC (rev 127920)
@@ -0,0 +1,30 @@
+<html>
+ <head>
+ <script>
+ if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+ }
+ if (window.eventSender)
+ for (i=0; i<10; i++)
+ setTimeout('eventSender.mouseMoveTo(' + i + ', ' + i + ');', i);
+ setTimeout(function() {
+ document.body.innerHTML = "This test passes if it does not crash.";
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }, 10);
+ </script>
+ <style>
+ @font-face { font-family: "A"; src: url(); }
+ * { font-family: A; }
+ </style>
+ </head>
+ <body>
+ <!-- Multiple lines of text are needed to crash -->
+ This is a really long line. This is a really long line. This is a really long line. This is a really long line. This is a really long line. This is a really long line. This is a really long line. This is a really long line. This is a really long line. This is a really long line. This is a really long line. This is a really long line. This is a really long line.
+ <iframe seamless src=""
+ <!-- This lonely <style> is needed to crash -->
+ <style>
+ </style>
+ </body>
+</html>
\ No newline at end of file
Modified: trunk/Source/WebCore/ChangeLog (127919 => 127920)
--- trunk/Source/WebCore/ChangeLog 2012-09-07 21:16:12 UTC (rev 127919)
+++ trunk/Source/WebCore/ChangeLog 2012-09-07 21:17:50 UTC (rev 127920)
@@ -1,3 +1,63 @@
+2012-09-07 Stephen Chenney <schen...@chromium.org>
+
+ Font data is purged while fonts are still using it
+ https://bugs.webkit.org/show_bug.cgi?id=93640
+
+ Reviewed by Eric Seidel.
+
+ Move the handling of custom font pruning from Document to FontFallbackList.
+ The previous inplementation allowed fonts to be removed before all their
+ clients were done. This change moves handling of custom font purging to the
+ FontFallbackList class, which is the shared object that is only removed
+ when all clients of a font are done with it. This fixes a crash in Angry
+ Birds due to a seamless iframe and some failing tests in fast/frames/seamless.
+
+ The specific element that causes problems is:
+ <iframe id="ingame_frame0" name="ingame_frame0" frameborder="0" seamless="true"
+ src=""
+ _onload_="this.style.opacity = 1; parent.adLoaded();" scrolling="no"
+ style="opacity: 1; -webkit-transition: opacity 1s ease-in-out 0s;
+ position: absolute; border: 0px; width: 312px; height: 320px; z-index:
+ 300; overflow: hidden; visibility: visible;"></iframe>
+ The source document uses the same font as the embedding document.
+
+ Tests: fast/frames/seamless/seamless-custom-font-pruning-crash.html
+ fast/frames/seamless/seamless-nested-crash.html
+
+ * css/CSSFontFaceSource.cpp:
+ (WebCore::CSSFontFaceSource::getFontData): Remove code to register the
+ font with the document.
+ * css/CSSSegmentedFontFace.cpp:
+ (WebCore::CSSSegmentedFontFace::getFontData): Remove code to register
+ the font with the document.
+ * dom/Document.cpp:
+ (WebCore::Document::~Document): Remove code that records and purges
+ custom fonts.
+ (WebCore):
+ (WebCore::Document::reportMemoryUsage): Remove reference to removed
+ object.
+ * dom/Document.h:
+ (Document): Remove method declarations for custom font handling.
+ * platform/graphics/FontData.h: Add RefCounted to FontData. Previously, FontData
+ for custom fonts was owned by a Vector<OwnPtr<FontData>> in Document. Now it
+ is owned by FontFallbackList, and ref counting is required. A future patch will
+ add RefPtr code for all FontData use cases.
+ * platform/graphics/FontFallbackList.cpp:
+ (WebCore): Add global custom font data cache, which owns the FontData pointers
+ used as keys.
+ (WebCore::FontFallbackList::appendFontData): Helper method increments
+ the ref count on custom FontData.
+ (WebCore::FontFallbackList::releaseFontData): Helper method decrements
+ the ref count on custom FontData.
+ (WebCore::FontFallbackList::fontDataAt): Add calls to register the
+ FontFallbackList as a client of custom fonts.
+ * platform/graphics/FontFallbackList.h:
+ (WebCore::FontFallbackList::setGlyphPageZero): Moved this declaration.
+ (WebCore::FontFallbackList::setGlyphPages): Moved this declaration.
+ (FontFallbackList):
+ * platform/graphics/GlyphPageTreeNode.cpp:
+ (WebCore::GlyphPageTreeNode::pruneFontData): Removed unnecessary null check.
+
2012-09-07 Sheriff Bot <webkit.review....@gmail.com>
Unreviewed, rolling out r127911.
Modified: trunk/Source/WebCore/css/CSSFontFaceSource.cpp (127919 => 127920)
--- trunk/Source/WebCore/css/CSSFontFaceSource.cpp 2012-09-07 21:16:12 UTC (rev 127919)
+++ trunk/Source/WebCore/css/CSSFontFaceSource.cpp 2012-09-07 21:17:50 UTC (rev 127920)
@@ -167,7 +167,7 @@
return 0;
fontData = adoptPtr(new SimpleFontData(m_font->platformDataFromCustomData(fontDescription.computedPixelSize(), syntheticBold, syntheticItalic, fontDescription.orientation(),
- fontDescription.textOrientation(), fontDescription.widthVariant(), fontDescription.renderingMode()), true, false));
+ fontDescription.textOrientation(), fontDescription.widthVariant(), fontDescription.renderingMode()), true, false));
}
} else {
#if ENABLE(SVG_FONTS)
@@ -187,12 +187,7 @@
fontData = adoptPtr(new SimpleFontData(temporaryFont->platformData(), true, true));
}
- if (Document* document = fontSelector->document()) {
- cachedData = fontData.get();
- document->registerCustomFont(fontData.release());
- }
-
- return cachedData;
+ return fontData.leakPtr();
}
#if ENABLE(SVG_FONTS)
Modified: trunk/Source/WebCore/css/CSSSegmentedFontFace.cpp (127919 => 127920)
--- trunk/Source/WebCore/css/CSSSegmentedFontFace.cpp 2012-09-07 21:16:12 UTC (rev 127919)
+++ trunk/Source/WebCore/css/CSSSegmentedFontFace.cpp 2012-09-07 21:17:50 UTC (rev 127920)
@@ -124,14 +124,10 @@
appendFontDataWithInvalidUnicodeRangeIfLoading(newFontData.get(), faceFontData, m_fontFaces[i]->ranges());
}
}
- if (newFontData->numRanges()) {
- if (Document* document = m_fontSelector->document()) {
- fontData = newFontData.get();
- document->registerCustomFont(newFontData.release());
- }
- }
+ if (newFontData->numRanges())
+ return newFontData.leakPtr();
- return fontData;
+ return 0;
}
}
Modified: trunk/Source/WebCore/dom/Document.cpp (127919 => 127920)
--- trunk/Source/WebCore/dom/Document.cpp 2012-09-07 21:16:12 UTC (rev 127919)
+++ trunk/Source/WebCore/dom/Document.cpp 2012-09-07 21:17:50 UTC (rev 127920)
@@ -658,8 +658,6 @@
(*m_userSheets)[i]->clearOwnerNode();
}
- deleteCustomFonts();
-
m_weakReference->clear();
if (m_mediaQueryMatcher)
@@ -1978,20 +1976,6 @@
return style.release();
}
-void Document::registerCustomFont(PassOwnPtr<FontData> fontData)
-{
- m_customFonts.append(fontData);
-}
-
-void Document::deleteCustomFonts()
-{
- size_t size = m_customFonts.size();
- for (size_t i = 0; i < size; ++i)
- GlyphPageTreeNode::pruneTreeCustomFontData(m_customFonts[i].get());
-
- m_customFonts.clear();
-}
-
bool Document::isPageBoxVisible(int pageIndex)
{
RefPtr<RenderStyle> style = styleForPage(pageIndex);
@@ -6118,7 +6102,6 @@
MemoryClassInfo info(memoryObjectInfo, this, WebCoreMemoryTypes::DOM);
info.addInstrumentedMember(m_styleResolver);
ContainerNode::reportMemoryUsage(memoryObjectInfo);
- info.addVector(m_customFonts);
info.addInstrumentedMember(m_url);
info.addInstrumentedMember(m_baseURL);
info.addInstrumentedMember(m_baseURLOverride);
Modified: trunk/Source/WebCore/dom/Document.h (127919 => 127920)
--- trunk/Source/WebCore/dom/Document.h 2012-09-07 21:16:12 UTC (rev 127919)
+++ trunk/Source/WebCore/dom/Document.h 2012-09-07 21:17:50 UTC (rev 127920)
@@ -90,7 +90,6 @@
class EventListener;
class FloatRect;
class FloatQuad;
-class FontData;
class FormController;
class Frame;
class FrameView;
@@ -569,8 +568,6 @@
PassRefPtr<RenderStyle> styleForElementIgnoringPendingStylesheets(Element*);
PassRefPtr<RenderStyle> styleForPage(int pageIndex);
- void registerCustomFont(PassOwnPtr<FontData>);
-
// Returns true if page box (margin boxes and page borders) is visible.
bool isPageBoxVisible(int pageIndex);
@@ -1248,8 +1245,6 @@
void seamlessParentUpdatedStylesheets();
void notifySeamlessChildDocumentsOfStylesheetUpdate() const;
- void deleteCustomFonts();
-
PassRefPtr<NodeList> handleZeroPadding(const HitTestRequest&, HitTestResult&) const;
void loadEventDelayTimerFired(Timer<Document>*);
@@ -1283,7 +1278,6 @@
OwnPtr<StyleResolver> m_styleResolver;
bool m_didCalculateStyleResolver;
bool m_hasDirtyStyleResolver;
- Vector<OwnPtr<FontData> > m_customFonts;
Frame* m_frame;
RefPtr<DOMWindow> m_domWindow;
Modified: trunk/Source/WebCore/platform/graphics/FontData.h (127919 => 127920)
--- trunk/Source/WebCore/platform/graphics/FontData.h 2012-09-07 21:16:12 UTC (rev 127919)
+++ trunk/Source/WebCore/platform/graphics/FontData.h 2012-09-07 21:17:50 UTC (rev 127920)
@@ -29,13 +29,17 @@
#include <wtf/FastAllocBase.h>
#include <wtf/Forward.h>
#include <wtf/Noncopyable.h>
+#include <wtf/RefCounted.h>
#include <wtf/unicode/Unicode.h>
namespace WebCore {
class SimpleFontData;
-class FontData {
+// FIXME: The RefCounted functionality is only used by FontFallbackList for custom FontData.
+// The FontCache explicitly deletes FontData for non-custom fonts. All users of FontData should
+// be refactored to use RefPtr. https://bugs.webkit.org/show_bug.cgi?id=95866
+class FontData : public RefCounted<FontData> {
WTF_MAKE_NONCOPYABLE(FontData); WTF_MAKE_FAST_ALLOCATED;
public:
FontData()
Modified: trunk/Source/WebCore/platform/graphics/FontFallbackList.cpp (127919 => 127920)
--- trunk/Source/WebCore/platform/graphics/FontFallbackList.cpp 2012-09-07 21:16:12 UTC (rev 127919)
+++ trunk/Source/WebCore/platform/graphics/FontFallbackList.cpp 2012-09-07 21:17:50 UTC (rev 127920)
@@ -32,6 +32,7 @@
#include "Font.h"
#include "FontCache.h"
#include "SegmentedFontData.h"
+#include <wtf/ListHashSet.h>
namespace WebCore {
@@ -62,11 +63,26 @@
m_generation = fontCache()->generation();
}
-void FontFallbackList::releaseFontData()
+void FontFallbackList::appendFontData(const FontData* fontData) const
{
+ bool isCustomFont = fontData->isCustomFont();
+ m_fontList.append(pair<const FontData*, IsCustomFontOrNot>(fontData, isCustomFont ? IsCustomFont : IsNotCustomFont));
+ if (isCustomFont) {
+ // This FontFallbackList is using a FontData object that we wish to live while in use
+ // by any FontFallbackList, so increment its ref count.
+ const_cast<FontData*>(fontData)->ref();
+ }
+}
+
+void FontFallbackList::releaseFontData() const
+{
unsigned numFonts = m_fontList.size();
for (unsigned i = 0; i < numFonts; ++i) {
- if (!m_fontList[i].second) {
+ if (m_fontList[i].second == IsCustomFont) {
+ // This FontFallbackList no longer requires the custom FontData, so deref. If we are the
+ // last FontFallbackList using this custom FontData, it will be deleted.
+ const_cast<FontData*>(m_fontList[i].first)->deref();
+ } else {
ASSERT(!m_fontList[i].first->isSegmented());
fontCache()->releaseFontData(static_cast<const SimpleFontData*>(m_fontList[i].first));
}
@@ -106,7 +122,7 @@
ASSERT(fontCache()->generation() == m_generation);
const FontData* result = fontCache()->getFontData(*font, m_familyIndex, m_fontSelector.get());
if (result) {
- m_fontList.append(pair<const FontData*, bool>(result, result->isCustomFont()));
+ appendFontData(result);
if (result->isLoading())
m_loadingCustomFonts = true;
}
@@ -118,7 +134,7 @@
m_familyIndex = cAllFamiliesScanned;
ASSERT(fontCache()->generation() == m_generation);
const FontData* fontData = fontCache()->getCachedFontData(&platformData);
- m_fontList.append(pair<const FontData*, bool>(fontData, fontData->isCustomFont()));
+ appendFontData(fontData);
}
}
Modified: trunk/Source/WebCore/platform/graphics/FontFallbackList.h (127919 => 127920)
--- trunk/Source/WebCore/platform/graphics/FontFallbackList.h 2012-09-07 21:16:12 UTC (rev 127919)
+++ trunk/Source/WebCore/platform/graphics/FontFallbackList.h 2012-09-07 21:17:50 UTC (rev 127920)
@@ -39,6 +39,7 @@
const int cAllFamiliesScanned = -1;
class FontFallbackList : public RefCounted<FontFallbackList> {
+ WTF_MAKE_NONCOPYABLE(FontFallbackList);
public:
static PassRefPtr<FontFallbackList> create() { return adoptRef(new FontFallbackList()); }
@@ -63,9 +64,7 @@
const GlyphPages& glyphPages() const { return m_pages; }
private:
- friend class SVGTextRunRenderingContext;
- void setGlyphPageZero(GlyphPageTreeNode* pageZero) { m_pageZero = pageZero; }
- void setGlyphPages(const GlyphPages& pages) { m_pages = pages; }
+ enum IsCustomFontOrNot { IsNotCustomFont, IsCustomFont };
FontFallbackList();
@@ -82,9 +81,18 @@
void setPlatformFont(const FontPlatformData&);
- void releaseFontData();
+ void setGlyphPageZero(GlyphPageTreeNode* pageZero) { m_pageZero = pageZero; }
+ void setGlyphPages(const GlyphPages& pages) { m_pages = pages; }
- mutable Vector<pair<const FontData*, bool>, 1> m_fontList;
+ void appendFontData(const FontData*) const;
+ void releaseFontData() const;
+
+ // A custom FontData pointer in this Vector is owned by FontFallbackList and
+ // deleted when no FontFallbackList is using it. A non-custom FontData pointer
+ // is owned by the FontCache and is deleted when the font is purged.
+ // FIXME: Shift to RefPtr for all FontData objects and modify this pair to be a custom class.
+ // https://bugs.webkit.org/show_bug.cgi?id=95866
+ mutable Vector<pair<const FontData*, IsCustomFontOrNot>, 1> m_fontList;
mutable GlyphPages m_pages;
mutable GlyphPageTreeNode* m_pageZero;
mutable const SimpleFontData* m_cachedPrimarySimpleFontData;
@@ -96,6 +104,7 @@
mutable bool m_loadingCustomFonts : 1;
friend class Font;
+ friend class SVGTextRunRenderingContext;
};
}
Modified: trunk/Source/WebCore/platform/graphics/GlyphPageTreeNode.cpp (127919 => 127920)
--- trunk/Source/WebCore/platform/graphics/GlyphPageTreeNode.cpp 2012-09-07 21:16:12 UTC (rev 127919)
+++ trunk/Source/WebCore/platform/graphics/GlyphPageTreeNode.cpp 2012-09-07 21:17:50 UTC (rev 127920)
@@ -374,8 +374,6 @@
void GlyphPageTreeNode::pruneFontData(const SimpleFontData* fontData, unsigned level)
{
ASSERT(fontData);
- if (!fontData)
- return;
// Prune fall back child (if any) of this font.
if (m_systemFallbackChild && m_systemFallbackChild->m_page)
