Crash when font completes downloading after calling 2D canvas setText() multiple...
authormmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 5 Sep 2015 05:33:16 +0000 (05:33 +0000)
committermmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 5 Sep 2015 05:33:16 +0000 (05:33 +0000)
https://bugs.webkit.org/show_bug.cgi?id=148789

Reviewed by Darin Adler.

Source/WebCore:

The CSSFontSelector has a list of clients, and when fonts complete downloading, these
clients get a call back. CanvasRenderingContext2D::State is one such of these clients. However,
the CSSFontSelector may be destroyed and recreated at any time. We were getting into a case
where multiple CSSFontSelectors were thinking that the same CanvasRenderingContext2D::State were
their client. When the CanvasRenderingContext2D::State was destroyed, it only unregistered
itself from one of the CSSFontSelectors, which means the CSSFontSelector left over has a dangling
pointer to it.

The solution is to implement a new helper class, FontProxy, to hold the
CanvasRenderingContext2D::State's font, and maintain the invariant that this object is always
registered to exactly one CSSFontSelector, and this CSSFontSelector is the one which is associated
with the FontProxy's FontCascade object. This patch maintains this invariant, as well as protecting
all access to the State's FontCascade object so no one can reach in and change it without going
through functions which maintain the invariant.

Test: fast/canvas/font-selector-crash.html

* css/CSSFontSelector.cpp:
(WebCore::CSSFontSelector::registerForInvalidationCallbacks):
(WebCore::CSSFontSelector::unregisterForInvalidationCallbacks):
(WebCore::CSSFontSelector::dispatchInvalidationCallbacks):
* css/CSSFontSelector.h:
* dom/Document.cpp:
(WebCore::Document::fontsNeedUpdate):
(WebCore::Document::fontSelector):
(WebCore::Document::clearStyleResolver):
* dom/Document.h:
* html/canvas/CanvasRenderingContext2D.cpp:
(WebCore::CanvasRenderingContext2D::State::State):
(WebCore::CanvasRenderingContext2D::State::operator=):
(WebCore::CanvasRenderingContext2D::FontProxy::~FontProxy):
(WebCore::CanvasRenderingContext2D::FontProxy::FontProxy):
(WebCore::CanvasRenderingContext2D::FontProxy::update):
(WebCore::CanvasRenderingContext2D::FontProxy::fontsNeedUpdate):
(WebCore::CanvasRenderingContext2D::FontProxy::initialize):
(WebCore::CanvasRenderingContext2D::FontProxy::fontMetrics):
(WebCore::CanvasRenderingContext2D::FontProxy::fontDescription):
(WebCore::CanvasRenderingContext2D::FontProxy::width):
(WebCore::CanvasRenderingContext2D::FontProxy::drawBidiText):
(WebCore::CanvasRenderingContext2D::font):
(WebCore::CanvasRenderingContext2D::setFont):
(WebCore::CanvasRenderingContext2D::measureText):
(WebCore::CanvasRenderingContext2D::drawTextInternal):
(WebCore::CanvasRenderingContext2D::State::~State): Deleted.
(WebCore::CanvasRenderingContext2D::State::fontsNeedUpdate): Deleted.
(WebCore::CanvasRenderingContext2D::accessFont): Deleted.
* html/canvas/CanvasRenderingContext2D.h:
* platform/graphics/FontSelector.h:

LayoutTests:

* fast/canvas/font-selector-crash-expected.txt: Added.
* fast/canvas/font-selector-crash.html: Added.
* fast/canvas/resources/font-selector-crash.ttf: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@189421 268f45cc-cd09-0410-ab3c-d52691b4dbfc

12 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/canvas/font-selector-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/canvas/font-selector-crash.html [new file with mode: 0644]
LayoutTests/fast/canvas/resources/font-selector-crash.ttf [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/css/CSSFontSelector.cpp
Source/WebCore/css/CSSFontSelector.h
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Document.h
Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp
Source/WebCore/html/canvas/CanvasRenderingContext2D.h
Source/WebCore/platform/graphics/FontSelector.h

index d7e06a1..4856b76 100644 (file)
@@ -1,3 +1,14 @@
+2015-09-04  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Crash when font completes downloading after calling 2D canvas setText() multiple times
+        https://bugs.webkit.org/show_bug.cgi?id=148789
+
+        Reviewed by Darin Adler.
+
+        * fast/canvas/font-selector-crash-expected.txt: Added.
+        * fast/canvas/font-selector-crash.html: Added.
+        * fast/canvas/resources/font-selector-crash.ttf: Added.
+
 2015-09-04  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r189386.
diff --git a/LayoutTests/fast/canvas/font-selector-crash-expected.txt b/LayoutTests/fast/canvas/font-selector-crash-expected.txt
new file mode 100644 (file)
index 0000000..0219534
--- /dev/null
@@ -0,0 +1,7 @@
+PASS document.getElementById('sleepOnMe').getClientRects()[0].width became different from initialWidth
+PASS successfullyParsed is true
+
+TEST COMPLETE
+This test makes sure there is no crash when the CSSFontSelector is cleared in between two canvas setFont() calls. The test is successful if there is no crash.
+
+Test 
diff --git a/LayoutTests/fast/canvas/font-selector-crash.html b/LayoutTests/fast/canvas/font-selector-crash.html
new file mode 100644 (file)
index 0000000..d94e19a
--- /dev/null
@@ -0,0 +1,54 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style id="styleElement">
+p {
+    font-family: helvetica;
+}
+</style>
+<style>
+@font-face {
+    font-family: 'WebAhem';
+    src: url('resources/font-selector-crash.ttf') format('truetype');
+}
+</style>
+<script src="../../resources/js-test-pre.js"></script>
+</head>
+<body>
+<p>This test makes sure there is no crash when the CSSFontSelector is cleared in between two canvas setFont() calls. The test is successful if there is no crash.</p>
+<span id="sleepOnMe" style="font-family: WebAhem;">Test</span>
+<canvas id="canvas"></canvas>
+<script>
+function clearFontSelector() {
+    var e = document.getElementById("styleElement");
+    e.setAttribute("media", "print");
+}
+
+window.jsTestIsAsync = true;
+
+var canvasElement = document.getElementById("canvas");
+var context = canvasElement.getContext('2d');
+
+// Step 1: Create a State object.
+context.save();
+// Step 2: Make the CSSFontSelector have a pointer to the State object.
+context.font = "15px Times";
+// Step 3: Create a second CSSFontSelector.
+clearFontSelector();
+// Step 4: Make the second CSSFontSelector have a pointer to the State object.
+context.font = "15px Helvetica";
+// Step 5: Destroy the State object.
+context.restore();
+// Step 6: When a font finishes downloading, the CSSFontSelector will call back the
+// State object it has a pointer to. When we destroyed the State object, we unregistered
+// it from the second CSSFontSelector, but not the first (which is the problem). The font
+// load was already kicked off earlier because of sleepOnMe, so it targets the first
+// CSSFontSelector, which now has a dangling pointer to the State object.
+
+// Wait for the font to download
+var initialWidth = document.getElementById('sleepOnMe').getClientRects()[0].width;
+shouldBecomeDifferent("document.getElementById('sleepOnMe').getClientRects()[0].width", "initialWidth", finishJSTest);
+</script>
+<script src="../../resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/fast/canvas/resources/font-selector-crash.ttf b/LayoutTests/fast/canvas/resources/font-selector-crash.ttf
new file mode 100644 (file)
index 0000000..ac81cb0
Binary files /dev/null and b/LayoutTests/fast/canvas/resources/font-selector-crash.ttf differ
index d76e643..f3c1326 100644 (file)
@@ -1,3 +1,59 @@
+2015-09-04  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Crash when font completes downloading after calling 2D canvas setText() multiple times
+        https://bugs.webkit.org/show_bug.cgi?id=148789
+
+        Reviewed by Darin Adler.
+
+        The CSSFontSelector has a list of clients, and when fonts complete downloading, these
+        clients get a call back. CanvasRenderingContext2D::State is one such of these clients. However,
+        the CSSFontSelector may be destroyed and recreated at any time. We were getting into a case
+        where multiple CSSFontSelectors were thinking that the same CanvasRenderingContext2D::State were
+        their client. When the CanvasRenderingContext2D::State was destroyed, it only unregistered
+        itself from one of the CSSFontSelectors, which means the CSSFontSelector left over has a dangling
+        pointer to it.
+
+        The solution is to implement a new helper class, FontProxy, to hold the
+        CanvasRenderingContext2D::State's font, and maintain the invariant that this object is always
+        registered to exactly one CSSFontSelector, and this CSSFontSelector is the one which is associated
+        with the FontProxy's FontCascade object. This patch maintains this invariant, as well as protecting
+        all access to the State's FontCascade object so no one can reach in and change it without going
+        through functions which maintain the invariant.
+
+        Test: fast/canvas/font-selector-crash.html
+
+        * css/CSSFontSelector.cpp:
+        (WebCore::CSSFontSelector::registerForInvalidationCallbacks):
+        (WebCore::CSSFontSelector::unregisterForInvalidationCallbacks):
+        (WebCore::CSSFontSelector::dispatchInvalidationCallbacks):
+        * css/CSSFontSelector.h:
+        * dom/Document.cpp:
+        (WebCore::Document::fontsNeedUpdate):
+        (WebCore::Document::fontSelector):
+        (WebCore::Document::clearStyleResolver):
+        * dom/Document.h:
+        * html/canvas/CanvasRenderingContext2D.cpp:
+        (WebCore::CanvasRenderingContext2D::State::State):
+        (WebCore::CanvasRenderingContext2D::State::operator=):
+        (WebCore::CanvasRenderingContext2D::FontProxy::~FontProxy):
+        (WebCore::CanvasRenderingContext2D::FontProxy::FontProxy):
+        (WebCore::CanvasRenderingContext2D::FontProxy::update):
+        (WebCore::CanvasRenderingContext2D::FontProxy::fontsNeedUpdate):
+        (WebCore::CanvasRenderingContext2D::FontProxy::initialize):
+        (WebCore::CanvasRenderingContext2D::FontProxy::fontMetrics):
+        (WebCore::CanvasRenderingContext2D::FontProxy::fontDescription):
+        (WebCore::CanvasRenderingContext2D::FontProxy::width):
+        (WebCore::CanvasRenderingContext2D::FontProxy::drawBidiText):
+        (WebCore::CanvasRenderingContext2D::font):
+        (WebCore::CanvasRenderingContext2D::setFont):
+        (WebCore::CanvasRenderingContext2D::measureText):
+        (WebCore::CanvasRenderingContext2D::drawTextInternal):
+        (WebCore::CanvasRenderingContext2D::State::~State): Deleted.
+        (WebCore::CanvasRenderingContext2D::State::fontsNeedUpdate): Deleted.
+        (WebCore::CanvasRenderingContext2D::accessFont): Deleted.
+        * html/canvas/CanvasRenderingContext2D.h:
+        * platform/graphics/FontSelector.h:
+
 2015-09-04  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r189386.
index 202cfc0..9a38bad 100644 (file)
@@ -329,14 +329,14 @@ void CSSFontSelector::addFontFaceRule(const StyleRuleFontFace* fontFaceRule, boo
     }
 }
 
-void CSSFontSelector::registerForInvalidationCallbacks(FontSelectorClient* client)
+void CSSFontSelector::registerForInvalidationCallbacks(FontSelectorClient& client)
 {
-    m_clients.add(client);
+    m_clients.add(&client);
 }
 
-void CSSFontSelector::unregisterForInvalidationCallbacks(FontSelectorClient* client)
+void CSSFontSelector::unregisterForInvalidationCallbacks(FontSelectorClient& client)
 {
-    m_clients.remove(client);
+    m_clients.remove(&client);
 }
 
 void CSSFontSelector::dispatchInvalidationCallbacks()
@@ -346,7 +346,7 @@ void CSSFontSelector::dispatchInvalidationCallbacks()
     Vector<FontSelectorClient*> clients;
     copyToVector(m_clients, clients);
     for (size_t i = 0; i < clients.size(); ++i)
-        clients[i]->fontsNeedUpdate(this);
+        clients[i]->fontsNeedUpdate(*this);
 }
 
 void CSSFontSelector::fontLoaded()
index 47ea640..72d6f11 100644 (file)
@@ -74,8 +74,8 @@ public:
 
     bool isEmpty() const;
 
-    virtual void registerForInvalidationCallbacks(FontSelectorClient*) override;
-    virtual void unregisterForInvalidationCallbacks(FontSelectorClient*) override;
+    virtual void registerForInvalidationCallbacks(FontSelectorClient&) override;
+    virtual void unregisterForInvalidationCallbacks(FontSelectorClient&) override;
 
     Document* document() const { return m_document; }
 
index 897bae2..c0941d2 100644 (file)
@@ -2081,7 +2081,7 @@ void Document::createStyleResolver()
     m_styleSheetCollection.combineCSSFeatureFlags();
 }
 
-void Document::fontsNeedUpdate(FontSelector*)
+void Document::fontsNeedUpdate(FontSelector&)
 {
     if (m_styleResolver)
         m_styleResolver->invalidateMatchedPropertiesCache();
@@ -2094,7 +2094,7 @@ CSSFontSelector& Document::fontSelector()
 {
     if (!m_fontSelector) {
         m_fontSelector = CSSFontSelector::create(*this);
-        m_fontSelector->registerForInvalidationCallbacks(this);
+        m_fontSelector->registerForInvalidationCallbacks(*this);
     }
     return *m_fontSelector;
 }
@@ -2106,7 +2106,7 @@ void Document::clearStyleResolver()
     // FIXME: It would be better if the FontSelector could survive this operation.
     if (m_fontSelector) {
         m_fontSelector->clearDocument();
-        m_fontSelector->unregisterForInvalidationCallbacks(this);
+        m_fontSelector->unregisterForInvalidationCallbacks(*this);
         m_fontSelector = nullptr;
     }
 }
index 89d3d08..e521067 100644 (file)
@@ -1302,7 +1302,7 @@ private:
     void processArguments(const String& features, void* data, ArgumentsCallback);
 
     // FontSelectorClient
-    virtual void fontsNeedUpdate(FontSelector*) override final;
+    virtual void fontsNeedUpdate(FontSelector&) override final;
 
     virtual bool isDocument() const override final { return true; }
 
index 53c9e44..8ab535e 100644 (file)
@@ -177,13 +177,11 @@ CanvasRenderingContext2D::State::State()
     , m_textBaseline(AlphabeticTextBaseline)
     , m_direction(Direction::Inherit)
     , m_unparsedFont(defaultFont)
-    , m_realizedFont(false)
 {
 }
 
 CanvasRenderingContext2D::State::State(const State& other)
-    : FontSelectorClient()
-    , m_unparsedStrokeColor(other.m_unparsedStrokeColor)
+    : m_unparsedStrokeColor(other.m_unparsedStrokeColor)
     , m_unparsedFillColor(other.m_unparsedFillColor)
     , m_strokeStyle(other.m_strokeStyle)
     , m_fillStyle(other.m_fillStyle)
@@ -206,10 +204,7 @@ CanvasRenderingContext2D::State::State(const State& other)
     , m_direction(other.m_direction)
     , m_unparsedFont(other.m_unparsedFont)
     , m_font(other.m_font)
-    , m_realizedFont(other.m_realizedFont)
 {
-    if (m_realizedFont)
-        m_font.fontSelector()->registerForInvalidationCallbacks(this);
 }
 
 CanvasRenderingContext2D::State& CanvasRenderingContext2D::State::operator=(const State& other)
@@ -217,9 +212,6 @@ CanvasRenderingContext2D::State& CanvasRenderingContext2D::State::operator=(cons
     if (this == &other)
         return *this;
 
-    if (m_realizedFont)
-        m_font.fontSelector()->unregisterForInvalidationCallbacks(this);
-
     m_unparsedStrokeColor = other.m_unparsedStrokeColor;
     m_unparsedFillColor = other.m_unparsedFillColor;
     m_strokeStyle = other.m_strokeStyle;
@@ -242,26 +234,85 @@ CanvasRenderingContext2D::State& CanvasRenderingContext2D::State::operator=(cons
     m_direction = other.m_direction;
     m_unparsedFont = other.m_unparsedFont;
     m_font = other.m_font;
-    m_realizedFont = other.m_realizedFont;
 
-    if (m_realizedFont)
-        m_font.fontSelector()->registerForInvalidationCallbacks(this);
+    return *this;
+}
+
+CanvasRenderingContext2D::FontProxy::~FontProxy()
+{
+    if (realized())
+        m_font.fontSelector()->unregisterForInvalidationCallbacks(*this);
+}
+
+CanvasRenderingContext2D::FontProxy::FontProxy(const FontProxy& other)
+    : m_font(other.m_font)
+{
+    if (realized())
+        m_font.fontSelector()->registerForInvalidationCallbacks(*this);
+}
+
+auto CanvasRenderingContext2D::FontProxy::operator=(const FontProxy& other) -> FontProxy&
+{
+    if (realized())
+        m_font.fontSelector()->unregisterForInvalidationCallbacks(*this);
+
+    m_font = other.m_font;
+
+    if (realized())
+        m_font.fontSelector()->registerForInvalidationCallbacks(*this);
 
     return *this;
 }
 
-CanvasRenderingContext2D::State::~State()
+inline void CanvasRenderingContext2D::FontProxy::update(FontSelector& selector)
 {
-    if (m_realizedFont)
-        m_font.fontSelector()->unregisterForInvalidationCallbacks(this);
+    ASSERT(&selector == m_font.fontSelector()); // This is an invariant. We should only ever be registered for callbacks on m_font.m_fonts.m_fontSelector.
+    if (realized())
+        m_font.fontSelector()->unregisterForInvalidationCallbacks(*this);
+    m_font.update(&selector);
+    if (realized())
+        m_font.fontSelector()->registerForInvalidationCallbacks(*this);
+    ASSERT(&selector == m_font.fontSelector());
 }
 
-void CanvasRenderingContext2D::State::fontsNeedUpdate(FontSelector* fontSelector)
+void CanvasRenderingContext2D::FontProxy::fontsNeedUpdate(FontSelector& selector)
 {
-    ASSERT_ARG(fontSelector, fontSelector == m_font.fontSelector());
-    ASSERT(m_realizedFont);
+    ASSERT_ARG(selector, &selector == m_font.fontSelector());
+    ASSERT(realized());
 
-    m_font.update(fontSelector);
+    update(selector);
+}
+
+inline void CanvasRenderingContext2D::FontProxy::initialize(FontSelector& fontSelector, RenderStyle& newStyle)
+{
+    // Beware! m_font.fontSelector() might not point to document.fontSelector()!
+    ASSERT(newStyle.fontCascade().fontSelector() == &fontSelector);
+    if (realized())
+        m_font.fontSelector()->unregisterForInvalidationCallbacks(*this);
+    m_font = newStyle.fontCascade();
+    m_font.update(&fontSelector);
+    ASSERT(&fontSelector == m_font.fontSelector());
+    m_font.fontSelector()->registerForInvalidationCallbacks(*this);
+}
+
+inline FontMetrics CanvasRenderingContext2D::FontProxy::fontMetrics() const
+{
+    return m_font.fontMetrics();
+}
+
+inline const FontDescription& CanvasRenderingContext2D::FontProxy::fontDescription() const
+{
+    return m_font.fontDescription();
+}
+
+inline float CanvasRenderingContext2D::FontProxy::width(const TextRun& textRun) const
+{
+    return m_font.width(textRun);
+}
+
+inline void CanvasRenderingContext2D::FontProxy::drawBidiText(GraphicsContext& context, const TextRun& run, const FloatPoint& point, FontCascade::CustomFontNotReadyAction action) const
+{
+    context.drawBidiText(m_font, run, point, action);
 }
 
 void CanvasRenderingContext2D::realizeSavesLoop()
@@ -2038,7 +2089,7 @@ void CanvasRenderingContext2D::putImageData(ImageData* data, ImageBuffer::Coordi
 
 String CanvasRenderingContext2D::font() const
 {
-    if (!state().m_realizedFont)
+    if (!state().m_font.realized())
         return defaultFont;
 
     StringBuilder serializedFont;
@@ -2052,11 +2103,11 @@ String CanvasRenderingContext2D::font() const
     serializedFont.appendNumber(fontDescription.computedPixelSize());
     serializedFont.appendLiteral("px");
 
-    for (unsigned i = 0; i < state().m_font.familyCount(); ++i) {
+    for (unsigned i = 0; i < fontDescription.familyCount(); ++i) {
         if (i)
             serializedFont.append(',');
         // FIXME: We should append family directly to serializedFont rather than building a temporary string.
-        String family = state().m_font.familyAt(i);
+        String family = fontDescription.familyAt(i);
         if (family.startsWith("-webkit-"))
             family = family.substring(8);
         if (family.contains(' '))
@@ -2071,7 +2122,7 @@ String CanvasRenderingContext2D::font() const
 
 void CanvasRenderingContext2D::setFont(const String& newFont)
 {
-    if (newFont == state().m_unparsedFont && state().m_realizedFont)
+    if (newFont == state().m_unparsedFont && state().m_font.realized())
         return;
 
     RefPtr<MutableStyleProperties> parsedStyle = MutableStyleProperties::create();
@@ -2093,7 +2144,7 @@ void CanvasRenderingContext2D::setFont(const String& newFont)
 
     // Map the <canvas> font into the text style. If the font uses keywords like larger/smaller, these will work
     // relative to the canvas.
-    RefPtr<RenderStyle> newStyle = RenderStyle::create();
+    Ref<RenderStyle> newStyle = RenderStyle::create();
 
     Document& document = canvas()->document();
     document.updateStyleIfNeeded();
@@ -2109,11 +2160,11 @@ void CanvasRenderingContext2D::setFont(const String& newFont)
         newStyle->setFontDescription(defaultFontDescription);
     }
 
-    newStyle->fontCascade().update(newStyle->fontCascade().fontSelector());
+    newStyle->fontCascade().update(&document.fontSelector());
 
     // Now map the font property longhands into the style.
     StyleResolver& styleResolver = canvas()->document().ensureStyleResolver();
-    styleResolver.applyPropertyToStyle(CSSPropertyFontFamily, parsedStyle->getPropertyCSSValue(CSSPropertyFontFamily).get(), newStyle.get());
+    styleResolver.applyPropertyToStyle(CSSPropertyFontFamily, parsedStyle->getPropertyCSSValue(CSSPropertyFontFamily).get(), &newStyle.get());
     styleResolver.applyPropertyToCurrentStyle(CSSPropertyFontStyle, parsedStyle->getPropertyCSSValue(CSSPropertyFontStyle).get());
     styleResolver.applyPropertyToCurrentStyle(CSSPropertyFontVariant, parsedStyle->getPropertyCSSValue(CSSPropertyFontVariant).get());
     styleResolver.applyPropertyToCurrentStyle(CSSPropertyFontWeight, parsedStyle->getPropertyCSSValue(CSSPropertyFontWeight).get());
@@ -2126,10 +2177,7 @@ void CanvasRenderingContext2D::setFont(const String& newFont)
     styleResolver.updateFont();
     styleResolver.applyPropertyToCurrentStyle(CSSPropertyLineHeight, parsedStyle->getPropertyCSSValue(CSSPropertyLineHeight).get());
 
-    modifiableState().m_font = newStyle->fontCascade();
-    modifiableState().m_font.update(&document.fontSelector());
-    modifiableState().m_realizedFont = true;
-    document.fontSelector().registerForInvalidationCallbacks(&modifiableState());
+    modifiableState().m_font.initialize(document.fontSelector(), newStyle);
 }
 
 String CanvasRenderingContext2D::textAlign() const
@@ -2265,7 +2313,7 @@ Ref<TextMetrics> CanvasRenderingContext2D::measureText(const String& text)
     String normalizedText = text;
     normalizeSpaces(normalizedText);
 
-    metrics->setWidth(accessFont().width(TextRun(normalizedText)));
+    metrics->setWidth(fontProxy().width(TextRun(normalizedText)));
 
     return metrics;
 }
@@ -2291,8 +2339,8 @@ void CanvasRenderingContext2D::drawTextInternal(const String& text, float x, flo
     if (fill && gradient && gradient->isZeroSize())
         return;
 
-    const FontCascade& font = accessFont();
-    const FontMetrics& fontMetrics = font.fontMetrics();
+    const auto& fontProxy = this->fontProxy();
+    const FontMetrics& fontMetrics = fontProxy.fontMetrics();
 
     String normalizedText = text;
     normalizeSpaces(normalizedText);
@@ -2326,7 +2374,7 @@ void CanvasRenderingContext2D::drawTextInternal(const String& text, float x, flo
         break;
     }
 
-    float fontWidth = font.width(TextRun(normalizedText, 0, 0, AllowTrailingExpansion, direction, override));
+    float fontWidth = fontProxy.width(TextRun(normalizedText, 0, 0, AllowTrailingExpansion, direction, override));
 
     useMaxWidth = (useMaxWidth && maxWidth < fontWidth);
     float width = useMaxWidth ? maxWidth : fontWidth;
@@ -2388,7 +2436,7 @@ void CanvasRenderingContext2D::drawTextInternal(const String& text, float x, flo
             else
                 c->setStrokeColor(Color::black, ColorSpaceDeviceRGB);
 
-            c->drawBidiText(font, textRun, location + offset, FontCascade::UseFallbackIfFontNotReady);
+            fontProxy.drawBidiText(*c, textRun, location + offset, FontCascade::UseFallbackIfFontNotReady);
         }
 
         std::unique_ptr<ImageBuffer> maskImage = c->createCompatibleBuffer(maskRect.size());
@@ -2408,10 +2456,10 @@ void CanvasRenderingContext2D::drawTextInternal(const String& text, float x, flo
             maskImageContext.translate(location.x() - maskRect.x(), location.y() - maskRect.y());
             // We draw when fontWidth is 0 so compositing operations (eg, a "copy" op) still work.
             maskImageContext.scale(FloatSize((fontWidth > 0 ? (width / fontWidth) : 0), 1));
-            maskImageContext.drawBidiText(font, textRun, FloatPoint(0, 0), FontCascade::UseFallbackIfFontNotReady);
+            fontProxy.drawBidiText(maskImageContext, textRun, FloatPoint(0, 0), FontCascade::UseFallbackIfFontNotReady);
         } else {
             maskImageContext.translate(-maskRect.x(), -maskRect.y());
-            maskImageContext.drawBidiText(font, textRun, location, FontCascade::UseFallbackIfFontNotReady);
+            fontProxy.drawBidiText(maskImageContext, textRun, location, FontCascade::UseFallbackIfFontNotReady);
         }
 
         GraphicsContextStateSaver stateSaver(*c);
@@ -2434,15 +2482,15 @@ void CanvasRenderingContext2D::drawTextInternal(const String& text, float x, flo
 
     if (isFullCanvasCompositeMode(state().m_globalComposite)) {
         beginCompositeLayer();
-        c->drawBidiText(font, textRun, location, FontCascade::UseFallbackIfFontNotReady);
+        fontProxy.drawBidiText(*c, textRun, location, FontCascade::UseFallbackIfFontNotReady);
         endCompositeLayer();
         didDrawEntireCanvas();
     } else if (state().m_globalComposite == CompositeCopy) {
         clearCanvas();
-        c->drawBidiText(font, textRun, location, FontCascade::UseFallbackIfFontNotReady);
+        fontProxy.drawBidiText(*c, textRun, location, FontCascade::UseFallbackIfFontNotReady);
         didDrawEntireCanvas();
     } else {
-        c->drawBidiText(font, textRun, location, FontCascade::UseFallbackIfFontNotReady);
+        fontProxy.drawBidiText(*c, textRun, location, FontCascade::UseFallbackIfFontNotReady);
         didDraw(textRect);
     }
 }
@@ -2462,11 +2510,11 @@ void CanvasRenderingContext2D::inflateStrokeRect(FloatRect& rect) const
     rect.inflate(delta);
 }
 
-const FontCascade& CanvasRenderingContext2D::accessFont()
+auto CanvasRenderingContext2D::fontProxy() -> const FontProxy&
 {
     canvas()->document().updateStyleIfNeeded();
 
-    if (!state().m_realizedFont)
+    if (!state().m_font.realized())
         setFont(state().m_unparsedFont);
     return state().m_font;
 }
index 4e84983..61567fe 100644 (file)
@@ -235,15 +235,33 @@ private:
         LTR
     };
 
-    struct State final : FontSelectorClient {
+    class FontProxy : public FontSelectorClient {
+    public:
+        FontProxy() = default;
+        virtual ~FontProxy();
+        FontProxy(const FontProxy&);
+        FontProxy& operator=(const FontProxy&);
+
+        bool realized() const { return m_font.fontSelector(); }
+        void initialize(FontSelector&, RenderStyle&);
+        FontMetrics fontMetrics() const;
+        const FontDescription& fontDescription() const;
+        float width(const TextRun&) const;
+        void drawBidiText(GraphicsContext&, const TextRun&, const FloatPoint&, FontCascade::CustomFontNotReadyAction) const;
+
+    private:
+        void update(FontSelector&);
+        virtual void fontsNeedUpdate(FontSelector&) override;
+
+        FontCascade m_font;
+    };
+
+    struct State final {
         State();
-        virtual ~State();
 
         State(const State&);
         State& operator=(const State&);
 
-        virtual void fontsNeedUpdate(FontSelector*) override;
-
         String m_unparsedStrokeColor;
         String m_unparsedFillColor;
         CanvasStyle m_strokeStyle;
@@ -270,8 +288,7 @@ private:
         Direction m_direction;
 
         String m_unparsedFont;
-        FontCascade m_font;
-        bool m_realizedFont;
+        FontProxy m_font;
     };
 
     enum CanvasDidDrawOption {
@@ -308,7 +325,9 @@ private:
 
     void drawTextInternal(const String& text, float x, float y, bool fill, float maxWidth = 0, bool useMaxWidth = false);
 
-    const FontCascade& accessFont();
+    // The relationship between FontCascade and CanvasRenderingContext2D::FontProxy must hold certain invariants.
+    // Therefore, all font operations must pass through the State.
+    const FontProxy& fontProxy();
 
 #if ENABLE(DASHBOARD_SUPPORT)
     void clearPathForDashboardBackwardCompatibilityMode();
index ebcbfd3..3b11772 100644 (file)
@@ -48,8 +48,8 @@ public:
 
     virtual void fontCacheInvalidated() { }
 
-    virtual void registerForInvalidationCallbacks(FontSelectorClient*) = 0;
-    virtual void unregisterForInvalidationCallbacks(FontSelectorClient*) = 0;
+    virtual void registerForInvalidationCallbacks(FontSelectorClient&) = 0;
+    virtual void unregisterForInvalidationCallbacks(FontSelectorClient&) = 0;
 
     virtual unsigned uniqueId() const = 0;
     virtual unsigned version() const = 0;
@@ -59,7 +59,7 @@ class FontSelectorClient {
 public:
     virtual ~FontSelectorClient() { }
 
-    virtual void fontsNeedUpdate(FontSelector*) = 0;
+    virtual void fontsNeedUpdate(FontSelector&) = 0;
 };
 
 } // namespace WebCore