Title: [97402] trunk
Revision
97402
Author
infe...@chromium.org
Date
2011-10-13 14:37:04 -0700 (Thu, 13 Oct 2011)

Log Message

Source/WebCore: Register custom fonts at their creation time,  
rather than at retirement time.
https://bugs.webkit.org/show_bug.cgi?id=68929

Reviewed by Dan Bernstein.

Test: fast/text/custom-font-data-crash2.html

* css/CSSFontFace.cpp:
* css/CSSFontFace.h: remove function added in r94508,
which is no longer needed. We now register custom fonts
at creation time.
* css/CSSFontFaceSource.cpp:
(WebCore::CSSFontFaceSource::pruneTable): no longer need
to delete/retire font data here, it will be handled in ~Document.
(WebCore::CSSFontFaceSource::getFontData): register custom
font to document's m_customFonts.
* css/CSSFontSelector.cpp:
* css/CSSFontSelector.h: remove function added in r94508,
which is no longer needed. We now register custom fonts
at creation time.
* css/CSSSegmentedFontFace.cpp:
(WebCore::CSSSegmentedFontFace::pruneTable): no longer need
to delete/retire font data here, it will be handled in ~Document.
(WebCore::CSSSegmentedFontFace::getFontData): register custom
font to document's m_customFonts.
* dom/Document.cpp: Change function names to registerCustomFont
, deleteCustomFonts and local to m_customFont.
(WebCore::Document::~Document):
(WebCore::Document::recalcStyle): yanking out the comment. We
no longer keep retired custom fonts. We clear all custom fonts
on Document destruction.
(WebCore::Document::registerCustomFont): 
(WebCore::Document::deleteCustomFonts):
* dom/Document.h:

LayoutTests: Register custom fonts at their creation time,
rather than at retirement time.   
https://bugs.webkit.org/show_bug.cgi?id=68929

Reviewed by Dan Bernstein.

* fast/text/custom-font-data-crash2-expected.txt: Added.
* fast/text/custom-font-data-crash2.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (97401 => 97402)


--- trunk/LayoutTests/ChangeLog	2011-10-13 21:28:13 UTC (rev 97401)
+++ trunk/LayoutTests/ChangeLog	2011-10-13 21:37:04 UTC (rev 97402)
@@ -1,3 +1,14 @@
+2011-10-12  Abhishek Arya  <infe...@chromium.org>
+
+        Register custom fonts at their creation time,
+        rather than at retirement time.   
+        https://bugs.webkit.org/show_bug.cgi?id=68929
+
+        Reviewed by Dan Bernstein.
+
+        * fast/text/custom-font-data-crash2-expected.txt: Added.
+        * fast/text/custom-font-data-crash2.html: Added.
+
 2011-10-13  Simon Fraser  <simon.fra...@apple.com>
 
         Add results to keep the bots quiet.

Added: trunk/LayoutTests/fast/text/custom-font-data-crash2-expected.txt (0 => 97402)


--- trunk/LayoutTests/fast/text/custom-font-data-crash2-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/text/custom-font-data-crash2-expected.txt	2011-10-13 21:37:04 UTC (rev 97402)
@@ -0,0 +1 @@
+PASS

Added: trunk/LayoutTests/fast/text/custom-font-data-crash2.html (0 => 97402)


--- trunk/LayoutTests/fast/text/custom-font-data-crash2.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/custom-font-data-crash2.html	2011-10-13 21:37:04 UTC (rev 97402)
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<html>
+<style>
+@font-face { font-family: A; src: url(); }
+#y { font-family: A; }
+#y:first-letter { content: "A"; }
+</style>
+<body>
+<div id="y">EFGH</div>
+</body>
+<script>
+function finish() {
+    document.body.innerHTML = "PASS";
+	if (window.layoutTestController)
+	    layoutTestController.notifyDone();
+}
+
+if (window.layoutTestController) {
+    layoutTestController.dumpAsText();
+    layoutTestController.waitUntilDone();
+}
+
+document.designMode = 'on';
+document.execCommand('selectall');
+
+// Let the font load to finish.
+setTimeout("finish()", 50);
+</script>
+<style>
+#y:before { content: "ABCD"; }
+</style>
+</body>
+</html>
\ No newline at end of file

Modified: trunk/Source/WebCore/ChangeLog (97401 => 97402)


--- trunk/Source/WebCore/ChangeLog	2011-10-13 21:28:13 UTC (rev 97401)
+++ trunk/Source/WebCore/ChangeLog	2011-10-13 21:37:04 UTC (rev 97402)
@@ -1,3 +1,41 @@
+2011-10-12  Abhishek Arya  <infe...@chromium.org>
+
+        Register custom fonts at their creation time,  
+        rather than at retirement time.
+        https://bugs.webkit.org/show_bug.cgi?id=68929
+
+        Reviewed by Dan Bernstein.
+
+        Test: fast/text/custom-font-data-crash2.html
+
+        * css/CSSFontFace.cpp:
+        * css/CSSFontFace.h: remove function added in r94508,
+        which is no longer needed. We now register custom fonts
+        at creation time.
+        * css/CSSFontFaceSource.cpp:
+        (WebCore::CSSFontFaceSource::pruneTable): no longer need
+        to delete/retire font data here, it will be handled in ~Document.
+        (WebCore::CSSFontFaceSource::getFontData): register custom
+        font to document's m_customFonts.
+        * css/CSSFontSelector.cpp:
+        * css/CSSFontSelector.h: remove function added in r94508,
+        which is no longer needed. We now register custom fonts
+        at creation time.
+        * css/CSSSegmentedFontFace.cpp:
+        (WebCore::CSSSegmentedFontFace::pruneTable): no longer need
+        to delete/retire font data here, it will be handled in ~Document.
+        (WebCore::CSSSegmentedFontFace::getFontData): register custom
+        font to document's m_customFonts.
+        * dom/Document.cpp: Change function names to registerCustomFont
+        , deleteCustomFonts and local to m_customFont.
+        (WebCore::Document::~Document):
+        (WebCore::Document::recalcStyle): yanking out the comment. We
+        no longer keep retired custom fonts. We clear all custom fonts
+        on Document destruction.
+        (WebCore::Document::registerCustomFont): 
+        (WebCore::Document::deleteCustomFonts):
+        * dom/Document.h:
+
 2011-10-13  Ryosuke Niwa  <rn...@webkit.org>
 
         Crash in DeleteSelectionCommand::handleGeneralDelete

Modified: trunk/Source/WebCore/css/CSSFontFace.cpp (97401 => 97402)


--- trunk/Source/WebCore/css/CSSFontFace.cpp	2011-10-13 21:28:13 UTC (rev 97401)
+++ trunk/Source/WebCore/css/CSSFontFace.cpp	2011-10-13 21:37:04 UTC (rev 97402)
@@ -118,19 +118,6 @@
     return 0;
 }
 
-void CSSFontFace::retireCustomFont(SimpleFontData* fontData)
-{
-    if (m_segmentedFontFaces.isEmpty()) {
-        GlyphPageTreeNode::pruneTreeCustomFontData(fontData);
-        delete fontData;
-        return;
-    }
-
-    // Use one of the CSSSegmentedFontFaces' font selector. They all have
-    // the same font selector.
-    (*m_segmentedFontFaces.begin())->fontSelector()->retireCustomFont(fontData);
-}
-
 #if ENABLE(SVG_FONTS)
 bool CSSFontFace::hasSVGFontFaceSource() const
 {

Modified: trunk/Source/WebCore/css/CSSFontFace.h (97401 => 97402)


--- trunk/Source/WebCore/css/CSSFontFace.h	2011-10-13 21:28:13 UTC (rev 97401)
+++ trunk/Source/WebCore/css/CSSFontFace.h	2011-10-13 21:37:04 UTC (rev 97402)
@@ -63,7 +63,6 @@
     void addSource(CSSFontFaceSource*);
 
     void fontLoaded(CSSFontFaceSource*);
-    void retireCustomFont(SimpleFontData*);
 
     SimpleFontData* getFontData(const FontDescription&, bool syntheticBold, bool syntheticItalic);
 

Modified: trunk/Source/WebCore/css/CSSFontFaceSource.cpp (97401 => 97402)


--- trunk/Source/WebCore/css/CSSFontFaceSource.cpp	2011-10-13 21:28:13 UTC (rev 97401)
+++ trunk/Source/WebCore/css/CSSFontFaceSource.cpp	2011-10-13 21:37:04 UTC (rev 97402)
@@ -72,10 +72,6 @@
     if (m_fontDataTable.isEmpty())
         return;
 
-    HashMap<unsigned, SimpleFontData*>::iterator end = m_fontDataTable.end();
-    for (HashMap<unsigned, SimpleFontData*>::iterator it = m_fontDataTable.begin(); it != end; ++it)
-        m_face->retireCustomFont(it->second);
-
     m_fontDataTable.clear();
 }
 
@@ -194,6 +190,9 @@
 
     SimpleFontData* fontDataRawPtr = fontData.leakPtr();
     m_fontDataTable.set(hashKey, fontDataRawPtr);
+    ASSERT(fontSelector->document());
+    if (Document* doc = fontSelector->document())
+        doc->registerCustomFont(fontDataRawPtr);
 
     return fontDataRawPtr;
 }

Modified: trunk/Source/WebCore/css/CSSFontSelector.cpp (97401 => 97402)


--- trunk/Source/WebCore/css/CSSFontSelector.cpp	2011-10-13 21:28:13 UTC (rev 97401)
+++ trunk/Source/WebCore/css/CSSFontSelector.cpp	2011-10-13 21:37:04 UTC (rev 97402)
@@ -383,16 +383,6 @@
     dispatchInvalidationCallbacks();
 }
 
-void CSSFontSelector::retireCustomFont(FontData* fontData)
-{
-    if (m_document)
-        m_document->retireCustomFont(fontData);
-    else {
-        GlyphPageTreeNode::pruneTreeCustomFontData(fontData);
-        delete fontData;
-    }
-}
-
 static FontData* fontDataForGenericFamily(Document* document, const FontDescription& fontDescription, const AtomicString& familyName)
 {
     if (!document || !document->frame())

Modified: trunk/Source/WebCore/css/CSSFontSelector.h (97401 => 97402)


--- trunk/Source/WebCore/css/CSSFontSelector.h	2011-10-13 21:28:13 UTC (rev 97401)
+++ trunk/Source/WebCore/css/CSSFontSelector.h	2011-10-13 21:37:04 UTC (rev 97402)
@@ -59,8 +59,6 @@
     void fontLoaded();
     virtual void fontCacheInvalidated();
 
-    void retireCustomFont(FontData*);
-
     bool isEmpty() const;
 
     CachedResourceLoader* cachedResourceLoader() const;

Modified: trunk/Source/WebCore/css/CSSSegmentedFontFace.cpp (97401 => 97402)


--- trunk/Source/WebCore/css/CSSSegmentedFontFace.cpp	2011-10-13 21:28:13 UTC (rev 97401)
+++ trunk/Source/WebCore/css/CSSSegmentedFontFace.cpp	2011-10-13 21:37:04 UTC (rev 97402)
@@ -28,6 +28,7 @@
 
 #include "CSSFontFace.h"
 #include "CSSFontSelector.h"
+#include "Document.h"
 #include "FontDescription.h"
 #include "SegmentedFontData.h"
 #include "SimpleFontData.h"
@@ -53,10 +54,6 @@
     if (m_fontDataTable.isEmpty())
         return;
 
-    HashMap<unsigned, SegmentedFontData*>::iterator end = m_fontDataTable.end();
-    for (HashMap<unsigned, SegmentedFontData*>::iterator it = m_fontDataTable.begin(); it != end; ++it)
-        m_fontSelector->retireCustomFont(it->second);
-
     m_fontDataTable.clear();
 }
 
@@ -116,9 +113,12 @@
             }
         }
     }
-    if (fontData->numRanges())
+    if (fontData->numRanges()) {
         m_fontDataTable.set(hashKey, fontData);
-    else {
+        ASSERT(m_fontSelector->document());
+        if (Document* doc = m_fontSelector->document())
+            doc->registerCustomFont(fontData);
+    } else {
         delete fontData;
         fontData = 0;
     }

Modified: trunk/Source/WebCore/dom/Document.cpp (97401 => 97402)


--- trunk/Source/WebCore/dom/Document.cpp	2011-10-13 21:28:13 UTC (rev 97401)
+++ trunk/Source/WebCore/dom/Document.cpp	2011-10-13 21:37:04 UTC (rev 97402)
@@ -567,7 +567,7 @@
             (*m_userSheets)[i]->clearOwnerNode();
     }
 
-    deleteRetiredCustomFonts();
+    deleteCustomFonts();
 
     m_weakReference->clear();
 
@@ -1557,15 +1557,6 @@
             element->recalcStyle(change);
     }
 
-    // FIXME: Disabling the deletion of retired custom font data until
-    // we fix all the stale style bugs (68804, 68624, etc). These bugs
-    // indicate problems where some styles were not updated in recalcStyle,
-    // thereby retaining stale copy of font data. To prevent that, we
-    // disable this code for now and only delete retired custom font data
-    // in Document destructor.
-    // Now that all RenderStyles that pointed to retired fonts have been updated, the fonts can safely be deleted.
-    // deleteRetiredCustomFonts();
-
 #if USE(ACCELERATED_COMPOSITING)
     if (view()) {
         bool layoutPending = view()->layoutPending() || renderer()->needsLayout();
@@ -1702,18 +1693,18 @@
     return style.release();
 }
 
-void Document::retireCustomFont(FontData* fontData)
+void Document::registerCustomFont(FontData* fontData)
 {
-    m_retiredCustomFonts.append(adoptPtr(fontData));
+    m_customFonts.append(adoptPtr(fontData));
 }
 
-void Document::deleteRetiredCustomFonts()
+void Document::deleteCustomFonts()
 {
-    size_t size = m_retiredCustomFonts.size();
+    size_t size = m_customFonts.size();
     for (size_t i = 0; i < size; ++i)
-        GlyphPageTreeNode::pruneTreeCustomFontData(m_retiredCustomFonts[i].get());
+        GlyphPageTreeNode::pruneTreeCustomFontData(m_customFonts[i].get());
 
-    m_retiredCustomFonts.clear();
+    m_customFonts.clear();
 }
 
 bool Document::isPageBoxVisible(int pageIndex)

Modified: trunk/Source/WebCore/dom/Document.h (97401 => 97402)


--- trunk/Source/WebCore/dom/Document.h	2011-10-13 21:28:13 UTC (rev 97401)
+++ trunk/Source/WebCore/dom/Document.h	2011-10-13 21:37:04 UTC (rev 97402)
@@ -545,7 +545,7 @@
     PassRefPtr<RenderStyle> styleForElementIgnoringPendingStylesheets(Element*);
     PassRefPtr<RenderStyle> styleForPage(int pageIndex);
 
-    void retireCustomFont(FontData*);
+    void registerCustomFont(FontData*);
 
     // Returns true if page box (margin boxes and page borders) is visible.
     bool isPageBoxVisible(int pageIndex);
@@ -1135,7 +1135,7 @@
 
     void createStyleSelector();
 
-    void deleteRetiredCustomFonts();
+    void deleteCustomFonts();
 
     PassRefPtr<NodeList> handleZeroPadding(const HitTestRequest&, HitTestResult&) const;
 
@@ -1150,7 +1150,7 @@
     OwnPtr<CSSStyleSelector> m_styleSelector;
     bool m_didCalculateStyleSelector;
     bool m_hasDirtyStyleSelector;
-    Vector<OwnPtr<FontData> > m_retiredCustomFonts;
+    Vector<OwnPtr<FontData> > m_customFonts;
 
     mutable RefPtr<CSSPrimitiveValueCache> m_cssPrimitiveValueCache;
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to