Diff
Modified: trunk/LayoutTests/ChangeLog (130318 => 130319)
--- trunk/LayoutTests/ChangeLog 2012-10-03 21:00:29 UTC (rev 130318)
+++ trunk/LayoutTests/ChangeLog 2012-10-03 21:00:40 UTC (rev 130319)
@@ -1,3 +1,19 @@
+2012-10-03 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-10-03 Ojan Vafai <o...@chromium.org>
Fix some style violations in perparation for changing this code.
Added: trunk/LayoutTests/fast/frames/seamless/seamless-custom-font-pruning-crash-expected.txt (0 => 130319)
--- 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-10-03 21:00:40 UTC (rev 130319)
@@ -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 => 130319)
--- 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-10-03 21:00:40 UTC (rev 130319)
@@ -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 => 130319)
--- trunk/LayoutTests/fast/frames/seamless/seamless-nested-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/frames/seamless/seamless-nested-crash-expected.txt 2012-10-03 21:00:40 UTC (rev 130319)
@@ -0,0 +1 @@
+This test passes if it does not crash.
Added: trunk/LayoutTests/fast/frames/seamless/seamless-nested-crash.html (0 => 130319)
--- trunk/LayoutTests/fast/frames/seamless/seamless-nested-crash.html (rev 0)
+++ trunk/LayoutTests/fast/frames/seamless/seamless-nested-crash.html 2012-10-03 21:00:40 UTC (rev 130319)
@@ -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 (130318 => 130319)
--- trunk/Source/WebCore/ChangeLog 2012-10-03 21:00:29 UTC (rev 130318)
+++ trunk/Source/WebCore/ChangeLog 2012-10-03 21:00:40 UTC (rev 130319)
@@ -1,3 +1,51 @@
+2012-10-03 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 non-existent objects.
+ * dom/Document.h:
+ (WebCore):
+ (Document): Remove method declarations for custom font handling.
+ * platform/graphics/FontFallbackList.h:
+ (FontFallbackList): Moved some code around and made non-copyable.
+ (WebCore::FontFallbackList::setGlyphPageZero): Moved.
+ (WebCore::FontFallbackList::setGlyphPages): Moved.
+ * platform/graphics/GlyphPageTreeNode.cpp:
+ (WebCore::GlyphPageTreeNode::pruneFontData): Removed unnecessary null check.
+ * platform/graphics/SegmentedFontData.cpp:
+ (WebCore::SegmentedFontData::~SegmentedFontData): Added code to prune the Glyph pages when this is deleted.
+ * platform/graphics/SimpleFontData.cpp:
+ (WebCore::SimpleFontData::~SimpleFontData): Added code to prune the Glyph pages when this is deleted.
+
2012-10-03 Adam Barth <aba...@webkit.org>
Crash when calling document.open during unload
Modified: trunk/Source/WebCore/css/CSSFontFaceSource.cpp (130318 => 130319)
--- trunk/Source/WebCore/css/CSSFontFaceSource.cpp 2012-10-03 21:00:29 UTC (rev 130318)
+++ trunk/Source/WebCore/css/CSSFontFaceSource.cpp 2012-10-03 21:00:40 UTC (rev 130319)
@@ -184,11 +184,6 @@
fontData = SimpleFontData::create(temporaryFont->platformData(), true, true);
}
- if (Document* document = fontSelector->document())
- document->registerCustomFont(fontData);
- else
- fontData.clear();
-
return fontData; // No release, because fontData is a reference to a RefPtr that is held in the m_fontDataTable.
}
Modified: trunk/Source/WebCore/css/CSSSegmentedFontFace.cpp (130318 => 130319)
--- trunk/Source/WebCore/css/CSSSegmentedFontFace.cpp 2012-10-03 21:00:29 UTC (rev 130318)
+++ trunk/Source/WebCore/css/CSSSegmentedFontFace.cpp 2012-10-03 21:00:40 UTC (rev 130319)
@@ -108,10 +108,11 @@
unsigned hashKey = ((fontDescription.computedPixelSize() + 1) << (FontTraitsMaskWidth + 1)) | ((fontDescription.orientation() == Vertical ? 1 : 0) << FontTraitsMaskWidth) | desiredTraitsMask;
RefPtr<SegmentedFontData>& fontData = m_fontDataTable.add(hashKey, 0).iterator->second;
- if (fontData)
- return fontData;
+ if (fontData && fontData->numRanges())
+ return fontData; // No release, we have a reference to an object in the cache which should retain the ref count it has.
- fontData = SegmentedFontData::create();
+ if (!fontData)
+ fontData = SegmentedFontData::create();
unsigned size = m_fontFaces.size();
for (unsigned i = 0; i < size; i++) {
@@ -125,12 +126,8 @@
appendFontDataWithInvalidUnicodeRangeIfLoading(fontData.get(), faceFontData.release(), m_fontFaces[i]->ranges());
}
}
- if (fontData->numRanges()) {
- if (Document* document = m_fontSelector->document()) {
- document->registerCustomFont(fontData);
- return fontData;
- }
- }
+ if (fontData->numRanges())
+ return fontData; // No release, we have a reference to an object in the cache which should retain the ref count it has.
return 0;
}
Modified: trunk/Source/WebCore/dom/Document.cpp (130318 => 130319)
--- trunk/Source/WebCore/dom/Document.cpp 2012-10-03 21:00:29 UTC (rev 130318)
+++ trunk/Source/WebCore/dom/Document.cpp 2012-10-03 21:00:40 UTC (rev 130319)
@@ -645,8 +645,6 @@
if (m_elemSheet)
m_elemSheet->clearOwnerNode();
- deleteCustomFonts();
-
m_weakReference->clear();
if (m_mediaQueryMatcher)
@@ -1974,20 +1972,6 @@
return style.release();
}
-void Document::registerCustomFont(PassRefPtr<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);
@@ -5864,7 +5848,6 @@
MemoryClassInfo info(memoryObjectInfo, this, WebCoreMemoryTypes::DOM);
ContainerNode::reportMemoryUsage(memoryObjectInfo);
info.addMember(m_styleResolver);
- info.addMember(m_customFonts);
info.addMember(m_url);
info.addMember(m_baseURL);
info.addMember(m_baseURLOverride);
Modified: trunk/Source/WebCore/dom/Document.h (130318 => 130319)
--- trunk/Source/WebCore/dom/Document.h 2012-10-03 21:00:29 UTC (rev 130318)
+++ trunk/Source/WebCore/dom/Document.h 2012-10-03 21:00:40 UTC (rev 130319)
@@ -90,7 +90,6 @@
class EventListener;
class FloatRect;
class FloatQuad;
-class FontData;
class FormController;
class Frame;
class FrameView;
@@ -540,8 +539,6 @@
PassRefPtr<RenderStyle> styleForElementIgnoringPendingStylesheets(Element*);
PassRefPtr<RenderStyle> styleForPage(int pageIndex);
- void registerCustomFont(PassRefPtr<FontData>);
-
// Returns true if page box (margin boxes and page borders) is visible.
bool isPageBoxVisible(int pageIndex);
@@ -1210,8 +1207,6 @@
void seamlessParentUpdatedStylesheets();
- void deleteCustomFonts();
-
PassRefPtr<NodeList> handleZeroPadding(const HitTestRequest&, HitTestResult&) const;
void loadEventDelayTimerFired(Timer<Document>*);
@@ -1256,8 +1251,6 @@
// do eventually load.
PendingSheetLayout m_pendingSheetLayout;
- Vector<RefPtr<FontData> > m_customFonts;
-
Frame* m_frame;
RefPtr<DOMWindow> m_domWindow;
Modified: trunk/Source/WebCore/platform/graphics/FontFallbackList.h (130318 => 130319)
--- trunk/Source/WebCore/platform/graphics/FontFallbackList.h 2012-10-03 21:00:29 UTC (rev 130318)
+++ trunk/Source/WebCore/platform/graphics/FontFallbackList.h 2012-10-03 21:00:40 UTC (rev 130319)
@@ -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,10 +64,6 @@
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; }
-
FontFallbackList();
const SimpleFontData* primarySimpleFontData(const Font* f)
@@ -83,7 +80,9 @@
void setPlatformFont(const FontPlatformData&);
void releaseFontData();
-
+ void setGlyphPageZero(GlyphPageTreeNode* pageZero) { m_pageZero = pageZero; }
+ void setGlyphPages(const GlyphPages& pages) { m_pages = pages; }
+
mutable Vector<RefPtr<FontData>, 1> m_fontList;
mutable GlyphPages m_pages;
mutable GlyphPageTreeNode* m_pageZero;
@@ -96,6 +95,7 @@
mutable bool m_loadingCustomFonts : 1;
friend class Font;
+ friend class SVGTextRunRenderingContext;
};
}
Modified: trunk/Source/WebCore/platform/graphics/GlyphPageTreeNode.cpp (130318 => 130319)
--- trunk/Source/WebCore/platform/graphics/GlyphPageTreeNode.cpp 2012-10-03 21:00:29 UTC (rev 130318)
+++ trunk/Source/WebCore/platform/graphics/GlyphPageTreeNode.cpp 2012-10-03 21:00:40 UTC (rev 130319)
@@ -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)
Modified: trunk/Source/WebCore/platform/graphics/SegmentedFontData.cpp (130318 => 130319)
--- trunk/Source/WebCore/platform/graphics/SegmentedFontData.cpp 2012-10-03 21:00:29 UTC (rev 130318)
+++ trunk/Source/WebCore/platform/graphics/SegmentedFontData.cpp 2012-10-03 21:00:40 UTC (rev 130319)
@@ -34,6 +34,7 @@
SegmentedFontData::~SegmentedFontData()
{
+ GlyphPageTreeNode::pruneTreeCustomFontData(this);
}
const SimpleFontData* SegmentedFontData::fontDataForCharacter(UChar32 c) const
Modified: trunk/Source/WebCore/platform/graphics/SimpleFontData.cpp (130318 => 130319)
--- trunk/Source/WebCore/platform/graphics/SimpleFontData.cpp 2012-10-03 21:00:29 UTC (rev 130318)
+++ trunk/Source/WebCore/platform/graphics/SimpleFontData.cpp 2012-10-03 21:00:40 UTC (rev 130319)
@@ -150,7 +150,9 @@
#endif
platformDestroy();
- if (!isCustomFont())
+ if (isCustomFont())
+ GlyphPageTreeNode::pruneTreeCustomFontData(this);
+ else
GlyphPageTreeNode::pruneTreeFontData(this);
}
