Attachment 258436 Details for Bug 147765 – Patch

[patch] Patch

bug-147765-20150806201553.patch (text/plain), 4.89 KB, created by Myles C. Maxfield on 2015-08-06 20:16:03 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Myles C. Maxfield

Created: 2015-08-06 20:16:03 PDT

Size: 4.89 KB

patch

obsolete

>Subversion Revision: 188104
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 517a3b04261b90d50149359f669c3d860323f6ab..3d5126e03e12cc8df32bff24c075cd5ae341689d 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,5 +1,33 @@
>+2015-08-06  Myles C. Maxfield  <mmaxfield@apple.com>
>+
>+        CSSSegmentedFontFace::fontRanges() does not handle duplicate fonts correctly
>+        https://bugs.webkit.org/show_bug.cgi?id=147765
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        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  <mmaxfield@apple.com>
> 
>         [iOS] Remove dead code from FontCache::createFontPlatformData()
>         https://bugs.webkit.org/show_bug.cgi?id=147762
> 
>diff --git a/Source/WebCore/css/CSSSegmentedFontFace.cpp b/Source/WebCore/css/CSSSegmentedFontFace.cpp
>index bf99488b3be354f5c3e750b238063a773534813f..2f37d6a0fd0964a09979e6df7e9f7f18299a0677 100644
>--- a/Source/WebCore/css/CSSSegmentedFontFace.cpp
>+++ b/Source/WebCore/css/CSSSegmentedFontFace.cpp
>@@ -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 @@ FontRanges CSSSegmentedFontFace::fontRanges(const FontDescription& fontDescripti
>         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) {
>diff --git a/Source/WebCore/css/CSSSegmentedFontFace.h b/Source/WebCore/css/CSSSegmentedFontFace.h
>index 2e563663f2f5c1fe41993c9b08baaed2be86492a..f869ff6da0b7efa2e45bf126854532c48655652c 100644
>--- a/Source/WebCore/css/CSSSegmentedFontFace.h
>+++ b/Source/WebCore/css/CSSSegmentedFontFace.h
>@@ -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 @@ private:
> #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;
>diff --git a/Source/WebCore/platform/graphics/FontCache.h b/Source/WebCore/platform/graphics/FontCache.h
>index bd03b0708ca4dabf986575645ac02bd8254c35f1..7134cf10556b89a4edf6c4e22f2cb96e586cdbf3 100644
>--- a/Source/WebCore/platform/graphics/FontCache.h
>+++ b/Source/WebCore/platform/graphics/FontCache.h
>@@ -124,6 +124,20 @@ private:
>     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>;
>

Flags:

fpizlo: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 147765: 258436