Title: [195567] trunk/Source/WebCore
Revision
195567
Author
[email protected]
Date
2016-01-25 16:35:17 -0800 (Mon, 25 Jan 2016)

Log 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:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (195566 => 195567)


--- trunk/Source/WebCore/ChangeLog	2016-01-26 00:25:42 UTC (rev 195566)
+++ trunk/Source/WebCore/ChangeLog	2016-01-26 00:35:17 UTC (rev 195567)
@@ -1,3 +1,31 @@
+2016-01-25  Myles C. Maxfield  <[email protected]>
+
+        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  <[email protected]>
 
         Try to fix the simulator build.

Modified: trunk/Source/WebCore/css/CSSFontFaceSource.cpp (195566 => 195567)


--- trunk/Source/WebCore/css/CSSFontFaceSource.cpp	2016-01-26 00:25:42 UTC (rev 195566)
+++ trunk/Source/WebCore/css/CSSFontFaceSource.cpp	2016-01-26 00:35:17 UTC (rev 195567)
@@ -66,17 +66,8 @@
 {
     if (m_font)
         m_font->removeClient(this);
-    pruneTable();
 }
 
-void CSSFontFaceSource::pruneTable()
-{
-    if (m_fontTable.isEmpty())
-        return;
-
-    m_fontTable.clear();
-}
-
 bool CSSFontFaceSource::isValid() const
 {
     if (m_font)
@@ -86,7 +77,6 @@
 
 void CSSFontFaceSource::fontLoaded(CachedFont*)
 {
-    pruneTable();
     if (m_face)
         m_face->fontLoaded(this);
 }
@@ -107,19 +97,12 @@
         return FontCache::singleton().fontForFamily(fontDescription, m_string, true);
     }
 
-    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) {
             if (!m_font->ensureCustomFontData(m_string))
                 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)
             // In-Document SVG Fonts
@@ -137,24 +120,21 @@
                 auto customPlatformData = createFontCustomPlatformData(*m_generatedOTFBuffer);
                 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 {
         // Kick off the load. Do it soon rather than now, because we may be in the middle of layout,
         // and the loader may invoke arbitrary delegate or event handler code.
         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();
 }
 
 #if ENABLE(SVG_FONTS)

Modified: trunk/Source/WebCore/css/CSSFontFaceSource.h (195566 => 195567)


--- trunk/Source/WebCore/css/CSSFontFaceSource.h	2016-01-26 00:25:42 UTC (rev 195566)
+++ trunk/Source/WebCore/css/CSSFontFaceSource.h	2016-01-26 00:35:17 UTC (rev 195567)
@@ -34,7 +34,6 @@
 #include "SVGFontFaceElement.h"
 #endif
 #include "Timer.h"
-#include <wtf/HashMap.h>
 #include <wtf/text/AtomicString.h>
 
 namespace WebCore {
@@ -81,7 +80,6 @@
     AtomicString m_string; // URI for remote, built-in font name for local.
     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)
     RefPtr<SharedBuffer> m_generatedOTFBuffer;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to