Title: [188114] trunk/Source/WebCore
- Revision
- 188114
- Author
- mmaxfi...@apple.com
- Date
- 2015-08-06 21:21:01 -0700 (Thu, 06 Aug 2015)
Log Message
CSSSegmentedFontFace::fontRanges() does not handle duplicate fonts correctly
https://bugs.webkit.org/show_bug.cgi?id=147765
Reviewed by Filip Pizlo.
CSSSegmentedFontFace::fontRanges() was trying to hash on FontDescriptors by
picking a few specific pieces of data out of the FontDescriptor, computing
a hash on it, and using that unsigned as a key in a HashMap. This has two
problems: it doesn't handle equality correctly, as hash collisions cannot
depend on an equality operator to dedup, and it doesn't hash on all the
members of a FontDescription.
Instead, this HashMap should use FontDescriptionKey, which represents a
FontDescription, and is designed exactly for the purpose of being used as a
key in a HashMap.
No new tests because there is no behavior change (because a problem occurs
when two different FontDescriptions hash to the same value, which is rare).
* css/CSSSegmentedFontFace.cpp:
(WebCore::CSSSegmentedFontFace::fontRanges):
* css/CSSSegmentedFontFace.h:
* platform/graphics/FontCache.h:
(WebCore::FontDescriptionKeyHash::hash):
(WebCore::FontDescriptionKeyHash::equal):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (188113 => 188114)
--- trunk/Source/WebCore/ChangeLog 2015-08-07 04:17:04 UTC (rev 188113)
+++ trunk/Source/WebCore/ChangeLog 2015-08-07 04:21:01 UTC (rev 188114)
@@ -1,5 +1,33 @@
2015-08-06 Myles C. Maxfield <mmaxfi...@apple.com>
+ CSSSegmentedFontFace::fontRanges() does not handle duplicate fonts correctly
+ https://bugs.webkit.org/show_bug.cgi?id=147765
+
+ Reviewed by Filip Pizlo.
+
+ CSSSegmentedFontFace::fontRanges() was trying to hash on FontDescriptors by
+ picking a few specific pieces of data out of the FontDescriptor, computing
+ a hash on it, and using that unsigned as a key in a HashMap. This has two
+ problems: it doesn't handle equality correctly, as hash collisions cannot
+ depend on an equality operator to dedup, and it doesn't hash on all the
+ members of a FontDescription.
+
+ Instead, this HashMap should use FontDescriptionKey, which represents a
+ FontDescription, and is designed exactly for the purpose of being used as a
+ key in a HashMap.
+
+ No new tests because there is no behavior change (because a problem occurs
+ when two different FontDescriptions hash to the same value, which is rare).
+
+ * css/CSSSegmentedFontFace.cpp:
+ (WebCore::CSSSegmentedFontFace::fontRanges):
+ * css/CSSSegmentedFontFace.h:
+ * platform/graphics/FontCache.h:
+ (WebCore::FontDescriptionKeyHash::hash):
+ (WebCore::FontDescriptionKeyHash::equal):
+
+2015-08-06 Myles C. Maxfield <mmaxfi...@apple.com>
+
[iOS] Remove dead code from FontCache::createFontPlatformData()
https://bugs.webkit.org/show_bug.cgi?id=147762
Modified: trunk/Source/WebCore/css/CSSSegmentedFontFace.cpp (188113 => 188114)
--- trunk/Source/WebCore/css/CSSSegmentedFontFace.cpp 2015-08-07 04:17:04 UTC (rev 188113)
+++ trunk/Source/WebCore/css/CSSSegmentedFontFace.cpp 2015-08-07 04:21:01 UTC (rev 188114)
@@ -31,6 +31,7 @@
#include "CSSFontSelector.h"
#include "Document.h"
#include "Font.h"
+#include "FontCache.h"
#include "FontDescription.h"
#include "RuntimeEnabledFeatures.h"
@@ -113,14 +114,8 @@
return FontRanges();
FontTraitsMask desiredTraitsMask = fontDescription.traitsMask();
- // FIXME: Unify this function with FontDescriptionKey in FontCache.h (Or just use the regular FontCache instead of this)
- unsigned hashKey = ((fontDescription.computedPixelSize() + 1) << (FontTraitsMaskWidth + FontWidthVariantWidth + FontSynthesisWidth + 1))
- | (fontDescription.fontSynthesis() << (FontTraitsMaskWidth + FontWidthVariantWidth + 1))
- | ((fontDescription.orientation() == Vertical ? 1 : 0) << (FontTraitsMaskWidth + FontWidthVariantWidth))
- | fontDescription.widthVariant() << FontTraitsMaskWidth
- | desiredTraitsMask;
- auto addResult = m_descriptionToRangesMap.add(hashKey, FontRanges());
+ auto addResult = m_descriptionToRangesMap.add(FontDescriptionKey(fontDescription), FontRanges());
auto& fontRanges = addResult.iterator->value;
if (addResult.isNewEntry) {
Modified: trunk/Source/WebCore/css/CSSSegmentedFontFace.h (188113 => 188114)
--- trunk/Source/WebCore/css/CSSSegmentedFontFace.h 2015-08-07 04:17:04 UTC (rev 188113)
+++ trunk/Source/WebCore/css/CSSSegmentedFontFace.h 2015-08-07 04:21:01 UTC (rev 188114)
@@ -26,6 +26,7 @@
#ifndef CSSSegmentedFontFace_h
#define CSSSegmentedFontFace_h
+#include "FontCache.h"
#include "FontRanges.h"
#include <wtf/HashMap.h>
#include <wtf/PassRefPtr.h>
@@ -73,7 +74,7 @@
#endif
CSSFontSelector* m_fontSelector;
- HashMap<unsigned, FontRanges> m_descriptionToRangesMap;
+ HashMap<FontDescriptionKey, FontRanges, FontDescriptionKeyHash, WTF::SimpleClassHashTraits<FontDescriptionKey>> m_descriptionToRangesMap;
Vector<RefPtr<CSSFontFace>, 1> m_fontFaces;
#if ENABLE(FONT_LOAD_EVENTS)
Vector<RefPtr<LoadFontCallback>> m_callbacks;
Modified: trunk/Source/WebCore/platform/graphics/FontCache.h (188113 => 188114)
--- trunk/Source/WebCore/platform/graphics/FontCache.h 2015-08-07 04:17:04 UTC (rev 188113)
+++ trunk/Source/WebCore/platform/graphics/FontCache.h 2015-08-07 04:21:01 UTC (rev 188114)
@@ -124,6 +124,20 @@
RefPtr<FontFeatureSettings> m_featureSettings;
};
+struct FontDescriptionKeyHash {
+ static unsigned hash(const FontDescriptionKey& key)
+ {
+ return key.computeHash();
+ }
+
+ static bool equal(const FontDescriptionKey& a, const FontDescriptionKey& b)
+ {
+ return a == b;
+ }
+
+ static const bool safeToCompareToEmptyOrDeleted = true;
+};
+
class FontCache {
friend class WTF::NeverDestroyed<FontCache>;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
