Changeset 195567 in webkit

Timestamp:

Jan 25, 2016 4:35:17 PM (8 years ago)

Author:

mmaxfield@apple.com

Message:

Remove broken cache from CSSFontFaceSource
​https://bugs.webkit.org/show_bug.cgi?id=153440

Reviewed by Simon Fraser.

This cache has been broken since 2013 (r158085). Given we didn't notice a perf
hit when it broke, and the fact it's been broken for years, it clearly isn't
necessary.

​https://bugs.webkit.org/show_bug.cgi?id=153414 consists of a fairly invasive
change to CSSFontFaceSource; this patch includes a working version of this
cache, along with an easy way to enable/disable it (to measure possible perf
changes).

This patch is a short-term cleanup patch in the mean time until the above
invasive change gets landed.

No new tests because there is no behavior (or performance!) change.

• css/CSSFontFaceSource.cpp:

(WebCore::CSSFontFaceSource::font):
(WebCore::CSSFontFaceSource::~CSSFontFaceSource): Deleted.
(WebCore::CSSFontFaceSource::pruneTable): Deleted.
(WebCore::CSSFontFaceSource::fontLoaded): Deleted.

• css/CSSFontFaceSource.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• css/CSSFontFaceSource.cpp (modified) (6 diffs)
• css/CSSFontFaceSource.h (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-01-25  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Remove broken cache from CSSFontFaceSource
+        https://bugs.webkit.org/show_bug.cgi?id=153440
+
+        Reviewed by Simon Fraser.
+
+        This cache has been broken since 2013 (r158085). Given we didn't notice a perf
+        hit when it broke, and the fact it's been broken for years, it clearly isn't
+        necessary.
+
+        https://bugs.webkit.org/show_bug.cgi?id=153414 consists of a fairly invasive
+        change to CSSFontFaceSource; this patch includes a working version of this
+        cache, along with an easy way to enable/disable it (to measure possible perf
+        changes).
+
+        This patch is a short-term cleanup patch in the mean time until the above
+        invasive change gets landed.
+
+        No new tests because there is no behavior (or performance!) change.
+
+        * css/CSSFontFaceSource.cpp:
+        (WebCore::CSSFontFaceSource::font):
+        (WebCore::CSSFontFaceSource::~CSSFontFaceSource): Deleted.
+        (WebCore::CSSFontFaceSource::pruneTable): Deleted.
+        (WebCore::CSSFontFaceSource::fontLoaded): Deleted.
+        * css/CSSFontFaceSource.h:
+
 2016-01-25  Sam Weinig  <sam@webkit.org>
 

trunk/Source/WebCore/css/CSSFontFaceSource.cpp

@@ -67 +67 @@
     if (m_font)
         m_font->removeClient(this);
-    pruneTable();
-}
-
-void CSSFontFaceSource::pruneTable()
-{
-    if (m_fontTable.isEmpty())
-        return;
-
-    m_fontTable.clear();
 }
 
@@ -87 +78 @@
 void CSSFontFaceSource::fontLoaded(CachedFont*)
 {
-    pruneTable();
     if (m_face)
         m_face->fontLoaded(this);
@@ -108 +98 @@
     }
 
-    unsigned hashKey = (fontDescription.computedPixelSize() + 1) << 5 | fontDescription.widthVariant() << 3
-                       | (fontDescription.orientation() == Vertical ? 4 : 0) | (syntheticBold ? 2 : 0) | (syntheticItalic ? 1 : 0);
-
-    RefPtr<Font> font = m_fontTable.add(hashKey, nullptr).iterator->value;
-    if (font)
-        return font.release();
-
     if (!m_font || m_font->isLoaded()) {
         if (m_font) {
@@ -120 +103 @@
                 return nullptr;
 
-            font = m_font->createFont(fontDescription, m_string, syntheticBold, syntheticItalic, fontFaceFeatures, fontFaceVariantSettings);
+            return m_font->createFont(fontDescription, m_string, syntheticBold, syntheticItalic, fontFaceFeatures, fontFaceVariantSettings);
         } else {
 #if ENABLE(SVG_FONTS)
@@ -138 +121 @@
                 if (!customPlatformData)
                     return nullptr;
-                font = Font::create(customPlatformData->fontPlatformData(fontDescription, syntheticBold, syntheticItalic, fontFaceFeatures, fontFaceVariantSettings), true, false);
+                return Font::create(customPlatformData->fontPlatformData(fontDescription, syntheticBold, syntheticItalic, fontFaceFeatures, fontFaceVariantSettings), true, false);
 #else
-                font = Font::create(std::make_unique<SVGFontData>(m_svgFontFaceElement.get()), fontDescription.computedPixelSize(), syntheticBold, syntheticItalic);
+                return Font::create(std::make_unique<SVGFontData>(m_svgFontFaceElement.get()), fontDescription.computedPixelSize(), syntheticBold, syntheticItalic);
 #endif
             }
 #endif
+            return nullptr;
         }
     } else {
@@ -150 +134 @@
         fontSelector->beginLoadingFontSoon(m_font.get());
 
-        Ref<Font> placeholderFont = FontCache::singleton().lastResortFallbackFont(fontDescription);
-        Ref<Font> placeholderFontCopyInLoadingState = Font::create(placeholderFont->platformData(), true, true);
-        return WTFMove(placeholderFontCopyInLoadingState);
+        return Font::create(FontCache::singleton().lastResortFallbackFont(fontDescription)->platformData(), true, true);
     }
-
-    return font.release();
 }
 

trunk/Source/WebCore/css/CSSFontFaceSource.h

@@ -35 +35 @@
 #endif
 #include "Timer.h"
-#include <wtf/HashMap.h>
 #include <wtf/text/AtomicString.h>
 
@@ -82 +81 @@
     CachedResourceHandle<CachedFont> m_font; // For remote fonts, a pointer to our cached resource.
     CSSFontFace* m_face; // Our owning font face.
-    HashMap<unsigned, RefPtr<Font>> m_fontTable; // The hash key is composed of size synthetic styles.
 
 #if ENABLE(SVG_OTF_CONVERTER)