REGRESSION (r104060): Web font is not loaded if specified by link element dynamically...
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Feb 2012 00:42:47 +0000 (00:42 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Feb 2012 00:42:47 +0000 (00:42 +0000)
https://bugs.webkit.org/show_bug.cgi?id=79186

Reviewed by Andreas Kling.

Source/WebCore:

Test: fast/css/font-face-insert-link.html

If a dynamically inserted stylesheet contains @font-face rules, we may fail to update the rendering.

Before r104060 the style selector was destroyed on every style change, and the font selector along with it.
This is no longer the case and we can't rely on comparing font selector pointers when comparing Fonts
for equality. This patch adds version number to the font selector and checks it in Font::operator==.
The version number is incremented when new font-face rules are added to the font selector.

FontFallbackList is an object shared between Fonts so the extra field shouldn't matter much in terms
of memory.

* css/CSSFontSelector.cpp:
(WebCore::CSSFontSelector::CSSFontSelector):
(WebCore::CSSFontSelector::addFontFaceRule):
* css/CSSFontSelector.h:
(CSSFontSelector):
* platform/graphics/Font.cpp:
(WebCore::Font::operator==):
* platform/graphics/FontFallbackList.cpp:
(WebCore::FontFallbackList::FontFallbackList):
(WebCore::FontFallbackList::invalidate):
* platform/graphics/FontFallbackList.h:
(FontFallbackList):
(WebCore::FontFallbackList::fontSelectorVersion):
* platform/graphics/FontSelector.h:
(FontSelector):

LayoutTests:

* fast/css/font-face-insert-link-expected.txt: Added.
* fast/css/font-face-insert-link.html: Added.
* fast/css/resources/ahem.css: Added.
(@font-face):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@108574 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/css/font-face-insert-link-expected.txt [new file with mode: 0644]
LayoutTests/fast/css/font-face-insert-link.html [new file with mode: 0644]
LayoutTests/fast/css/resources/ahem.css [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/css/CSSFontSelector.cpp
Source/WebCore/css/CSSFontSelector.h
Source/WebCore/platform/graphics/Font.cpp
Source/WebCore/platform/graphics/FontFallbackList.cpp
Source/WebCore/platform/graphics/FontFallbackList.h
Source/WebCore/platform/graphics/FontSelector.h

index b509db3fc9c9b1758b220178ab5636bb92b514a0..5e2c53b9b968d7b8300e67757afe03791fc782bd 100644 (file)
@@ -1,3 +1,15 @@
+2012-02-22  Antti Koivisto  <antti@apple.com>
+
+        REGRESSION (r104060): Web font is not loaded if specified by link element dynamically inserted
+        https://bugs.webkit.org/show_bug.cgi?id=79186
+
+        Reviewed by Andreas Kling.
+
+        * fast/css/font-face-insert-link-expected.txt: Added.
+        * fast/css/font-face-insert-link.html: Added.
+        * fast/css/resources/ahem.css: Added.
+        (@font-face):
+
 2012-02-22  Nate Chapin  <japhet@chromium.org>
 
         inspector/debugger/script-formatter-console.html should
diff --git a/LayoutTests/fast/css/font-face-insert-link-expected.txt b/LayoutTests/fast/css/font-face-insert-link-expected.txt
new file mode 100644 (file)
index 0000000..a60ac90
--- /dev/null
@@ -0,0 +1,5 @@
+PASS window.getComputedStyle(a).width == window.getComputedStyle(b).width is false
+PASS successfullyParsed is true
+
+TEST COMPLETE
+TextText
diff --git a/LayoutTests/fast/css/font-face-insert-link.html b/LayoutTests/fast/css/font-face-insert-link.html
new file mode 100644 (file)
index 0000000..ef770b3
--- /dev/null
@@ -0,0 +1,39 @@
+<head>
+<script src="../js/resources/js-test-pre.js"></script>
+<style>
+.test {
+  font-family: 'myahem';
+}
+#a, #b, #container { position:absolute; }
+#b { top: 20px }
+</style>
+</head>
+
+<div id=container>
+<div id=a>Text</div>
+<div id=b class="test">Text</div>
+</div>
+
+<script>
+jsTestIsAsync = true;
+
+window.setTimeout(
+  function() {
+    var link = document.createElement("link");
+    link.setAttribute("href", "resources/ahem.css");
+    link.setAttribute("rel", "stylesheet");
+    link.setAttribute("type", "text/css");
+    document.head.appendChild(link);
+    window.setTimeout(
+      function() {
+        var a = document.getElementById('a');
+        var b = document.getElementById('b');
+        shouldBeFalse('window.getComputedStyle(a).width == window.getComputedStyle(b).width');
+        finishJSTest();
+      },
+     500
+    );
+  },
+  1);
+</script>
+<script src="../js/resources/js-test-post.js"></script>
diff --git a/LayoutTests/fast/css/resources/ahem.css b/LayoutTests/fast/css/resources/ahem.css
new file mode 100644 (file)
index 0000000..640efe5
--- /dev/null
@@ -0,0 +1,4 @@
+@font-face {
+    font-family: 'myahem';
+    src: url(../../../resources/Ahem.ttf);
+}
index 6ec0f0ad601b0c4e98bdb07c699349f3251e7879..1d1535ee3a0e1f2b55665d282c1503fb2bf260ac 100644 (file)
@@ -1,3 +1,38 @@
+2012-02-22  Antti Koivisto  <antti@apple.com>
+
+        REGRESSION (r104060): Web font is not loaded if specified by link element dynamically inserted
+        https://bugs.webkit.org/show_bug.cgi?id=79186
+
+        Reviewed by Andreas Kling.
+
+        Test: fast/css/font-face-insert-link.html
+        
+        If a dynamically inserted stylesheet contains @font-face rules, we may fail to update the rendering.
+        
+        Before r104060 the style selector was destroyed on every style change, and the font selector along with it.
+        This is no longer the case and we can't rely on comparing font selector pointers when comparing Fonts
+        for equality. This patch adds version number to the font selector and checks it in Font::operator==.
+        The version number is incremented when new font-face rules are added to the font selector.
+        
+        FontFallbackList is an object shared between Fonts so the extra field shouldn't matter much in terms
+        of memory.
+
+        * css/CSSFontSelector.cpp:
+        (WebCore::CSSFontSelector::CSSFontSelector):
+        (WebCore::CSSFontSelector::addFontFaceRule):
+        * css/CSSFontSelector.h:
+        (CSSFontSelector):
+        * platform/graphics/Font.cpp:
+        (WebCore::Font::operator==):
+        * platform/graphics/FontFallbackList.cpp:
+        (WebCore::FontFallbackList::FontFallbackList):
+        (WebCore::FontFallbackList::invalidate):
+        * platform/graphics/FontFallbackList.h:
+        (FontFallbackList):
+        (WebCore::FontFallbackList::fontSelectorVersion):
+        * platform/graphics/FontSelector.h:
+        (FontSelector):
+
 2012-02-22  Nate Chapin  <japhet@chromium.org>
 
         Prevent CachedRawResource from sending the same data
index 08650b0efe1c379288b8eb22ad660a299416a207..6d22fdccd0d579d08c4a65c2e6e5ffbbb5a24fa5 100644 (file)
@@ -62,6 +62,7 @@ namespace WebCore {
 CSSFontSelector::CSSFontSelector(Document* document)
     : m_document(document)
     , m_beginLoadingTimer(this, &CSSFontSelector::beginLoadTimerFired)
+    , m_version(0)
 {
     // FIXME: An old comment used to say there was no need to hold a reference to m_document
     // because "we are guaranteed to be destroyed before the document". But there does not
@@ -307,6 +308,8 @@ void CSSFontSelector::addFontFaceRule(const CSSFontFaceRule* fontFaceRule)
         }
 
         familyFontFaces->append(fontFace);
+        
+        ++m_version;
     }
 }
 
index 3e992cc742b151b099b2441f42527c4aeb97f0c0..a5f8db5d239d0c5bf7a1498a842d6f4412f0107c 100644 (file)
@@ -51,6 +51,8 @@ public:
         return adoptRef(new CSSFontSelector(document));
     }
     virtual ~CSSFontSelector();
+    
+    virtual unsigned version() const OVERRIDE { return m_version; }
 
     virtual FontData* getFontData(const FontDescription& fontDescription, const AtomicString& familyName);
 
@@ -85,6 +87,8 @@ private:
 
     Vector<CachedResourceHandle<CachedFont> > m_fontsToBeginLoading;
     Timer<CSSFontSelector> m_beginLoadingTimer;
+    
+    unsigned m_version;
 };
 
 } // namespace WebCore
index c2d9c58c3f504815d5c349652cafa24dd9314735..50eee53a7ff7b75f8a8703878462caf1138db725 100644 (file)
@@ -120,11 +120,12 @@ bool Font::operator==(const Font& other) const
     
     FontSelector* first = m_fontList ? m_fontList->fontSelector() : 0;
     FontSelector* second = other.m_fontList ? other.m_fontList->fontSelector() : 0;
-    
+
     return first == second
            && m_fontDescription == other.m_fontDescription
            && m_letterSpacing == other.m_letterSpacing
            && m_wordSpacing == other.m_wordSpacing
+           && (m_fontList ? m_fontList->fontSelectorVersion() : 0) == (other.m_fontList ? other.m_fontList->fontSelectorVersion() : 0)
            && (m_fontList ? m_fontList->generation() : 0) == (other.m_fontList ? other.m_fontList->generation() : 0);
 }
 
index 2f1b68f8b765ae1da57181fe2bfbcfc2a5e2c571..6d11c09e34b70ebcd37fd791de9cdd8f812b95f7 100644 (file)
@@ -39,6 +39,7 @@ FontFallbackList::FontFallbackList()
     : m_pageZero(0)
     , m_cachedPrimarySimpleFontData(0)
     , m_fontSelector(0)
+    , m_fontSelectorVersion(0)
     , m_familyIndex(0)
     , m_generation(fontCache()->generation())
     , m_pitch(UnknownPitch)
@@ -57,6 +58,7 @@ void FontFallbackList::invalidate(PassRefPtr<FontSelector> fontSelector)
     m_pitch = UnknownPitch;
     m_loadingCustomFonts = false;
     m_fontSelector = fontSelector;
+    m_fontSelectorVersion = m_fontSelector ? m_fontSelector->version() : 0;
     m_generation = fontCache()->generation();
 }
 
index 10251a4eb7088f3ce871cbd32a73040f449c76f0..12479b2c9a0ad575d1e00a6740b19b9cba0ec1d7 100644 (file)
@@ -51,6 +51,8 @@ public:
     bool loadingCustomFonts() const { return m_loadingCustomFonts; }
 
     FontSelector* fontSelector() const { return m_fontSelector.get(); }
+    // FIXME: It should be possible to combine fontSelectorVersion and generation.
+    unsigned fontSelectorVersion() const { return m_fontSelectorVersion; }
     unsigned generation() const { return m_generation; }
 
     struct GlyphPagesHashTraits : HashTraits<int> {
@@ -87,6 +89,7 @@ private:
     mutable GlyphPageTreeNode* m_pageZero;
     mutable const SimpleFontData* m_cachedPrimarySimpleFontData;
     RefPtr<FontSelector> m_fontSelector;
+    unsigned m_fontSelectorVersion;
     mutable int m_familyIndex;
     unsigned short m_generation;
     mutable unsigned m_pitch : 3; // Pitch
index e18161bb8f5dddff6bf6ca4abd81e73a79dcb897..2c7d1494e47a46e3f8b0ee2e4ef92bfce7b19342 100644 (file)
@@ -44,6 +44,8 @@ public:
 
     virtual void registerForInvalidationCallbacks(FontSelectorClient*) = 0;
     virtual void unregisterForInvalidationCallbacks(FontSelectorClient*) = 0;
+    
+    virtual unsigned version() const = 0;
 };
 
 class FontSelectorClient {