- Revision
- 108574
- Author
- an...@apple.com
- Date
- 2012-02-22 16:42:47 -0800 (Wed, 22 Feb 2012)
Log Message
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.
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):
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (108573 => 108574)
--- trunk/LayoutTests/ChangeLog 2012-02-23 00:38:17 UTC (rev 108573)
+++ trunk/LayoutTests/ChangeLog 2012-02-23 00:42:47 UTC (rev 108574)
@@ -1,3 +1,15 @@
+2012-02-22 Antti Koivisto <an...@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 <jap...@chromium.org>
inspector/debugger/script-formatter-console.html should
Added: trunk/LayoutTests/fast/css/font-face-insert-link-expected.txt (0 => 108574)
--- trunk/LayoutTests/fast/css/font-face-insert-link-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/css/font-face-insert-link-expected.txt 2012-02-23 00:42:47 UTC (rev 108574)
@@ -0,0 +1,5 @@
+PASS window.getComputedStyle(a).width == window.getComputedStyle(b).width is false
+PASS successfullyParsed is true
+
+TEST COMPLETE
+TextText
Added: trunk/LayoutTests/fast/css/font-face-insert-link.html (0 => 108574)
--- trunk/LayoutTests/fast/css/font-face-insert-link.html (rev 0)
+++ trunk/LayoutTests/fast/css/font-face-insert-link.html 2012-02-23 00:42:47 UTC (rev 108574)
@@ -0,0 +1,39 @@
+<head>
+<script src=""
+<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=""
Added: trunk/LayoutTests/fast/css/resources/ahem.css (0 => 108574)
--- trunk/LayoutTests/fast/css/resources/ahem.css (rev 0)
+++ trunk/LayoutTests/fast/css/resources/ahem.css 2012-02-23 00:42:47 UTC (rev 108574)
@@ -0,0 +1,4 @@
+@font-face {
+ font-family: 'myahem';
+ src: url(../../../resources/Ahem.ttf);
+}
Modified: trunk/Source/WebCore/ChangeLog (108573 => 108574)
--- trunk/Source/WebCore/ChangeLog 2012-02-23 00:38:17 UTC (rev 108573)
+++ trunk/Source/WebCore/ChangeLog 2012-02-23 00:42:47 UTC (rev 108574)
@@ -1,3 +1,38 @@
+2012-02-22 Antti Koivisto <an...@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 <jap...@chromium.org>
Prevent CachedRawResource from sending the same data
Modified: trunk/Source/WebCore/css/CSSFontSelector.cpp (108573 => 108574)
--- trunk/Source/WebCore/css/CSSFontSelector.cpp 2012-02-23 00:38:17 UTC (rev 108573)
+++ trunk/Source/WebCore/css/CSSFontSelector.cpp 2012-02-23 00:42:47 UTC (rev 108574)
@@ -62,6 +62,7 @@
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 @@
}
familyFontFaces->append(fontFace);
+
+ ++m_version;
}
}
Modified: trunk/Source/WebCore/css/CSSFontSelector.h (108573 => 108574)
--- trunk/Source/WebCore/css/CSSFontSelector.h 2012-02-23 00:38:17 UTC (rev 108573)
+++ trunk/Source/WebCore/css/CSSFontSelector.h 2012-02-23 00:42:47 UTC (rev 108574)
@@ -51,6 +51,8 @@
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 @@
Vector<CachedResourceHandle<CachedFont> > m_fontsToBeginLoading;
Timer<CSSFontSelector> m_beginLoadingTimer;
+
+ unsigned m_version;
};
} // namespace WebCore
Modified: trunk/Source/WebCore/platform/graphics/Font.cpp (108573 => 108574)
--- trunk/Source/WebCore/platform/graphics/Font.cpp 2012-02-23 00:38:17 UTC (rev 108573)
+++ trunk/Source/WebCore/platform/graphics/Font.cpp 2012-02-23 00:42:47 UTC (rev 108574)
@@ -120,11 +120,12 @@
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);
}
Modified: trunk/Source/WebCore/platform/graphics/FontFallbackList.cpp (108573 => 108574)
--- trunk/Source/WebCore/platform/graphics/FontFallbackList.cpp 2012-02-23 00:38:17 UTC (rev 108573)
+++ trunk/Source/WebCore/platform/graphics/FontFallbackList.cpp 2012-02-23 00:42:47 UTC (rev 108574)
@@ -39,6 +39,7 @@
: 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 @@
m_pitch = UnknownPitch;
m_loadingCustomFonts = false;
m_fontSelector = fontSelector;
+ m_fontSelectorVersion = m_fontSelector ? m_fontSelector->version() : 0;
m_generation = fontCache()->generation();
}
Modified: trunk/Source/WebCore/platform/graphics/FontFallbackList.h (108573 => 108574)
--- trunk/Source/WebCore/platform/graphics/FontFallbackList.h 2012-02-23 00:38:17 UTC (rev 108573)
+++ trunk/Source/WebCore/platform/graphics/FontFallbackList.h 2012-02-23 00:42:47 UTC (rev 108574)
@@ -51,6 +51,8 @@
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 @@
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
Modified: trunk/Source/WebCore/platform/graphics/FontSelector.h (108573 => 108574)
--- trunk/Source/WebCore/platform/graphics/FontSelector.h 2012-02-23 00:38:17 UTC (rev 108573)
+++ trunk/Source/WebCore/platform/graphics/FontSelector.h 2012-02-23 00:42:47 UTC (rev 108574)
@@ -44,6 +44,8 @@
virtual void registerForInvalidationCallbacks(FontSelectorClient*) = 0;
virtual void unregisterForInvalidationCallbacks(FontSelectorClient*) = 0;
+
+ virtual unsigned version() const = 0;
};
class FontSelectorClient {
