⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 197804 in webkit


Ignore:
Timestamp:
Mar 8, 2016, 2:22:40 PM (11 years ago)
Author:
mmaxfield@apple.com
Message:

[Font Loading] Crash when a single load request causes multiple fonts to fail loading
​https://bugs.webkit.org/show_bug.cgi?id=155009

Reviewed by Simon Fraser.

Source/WebCore:

In JavaScript, the first promise fulfillment/failure wins. However, in C++, any
subsequent fulfillments/failures cause a crash.

Test: fast/text/font-face-set-document-multiple-failure.html

  • css/CSSFontFace.cpp:

(WebCore::iterateClients): Notifying a client may cause some other client
to be destroyed, thereby modifying the clients set. This function allows
for notifying clients in a resilient manner.
(WebCore::CSSFontFace::setStyle): Update to use iterateClients().
(WebCore::CSSFontFace::setWeight): Ditto.
(WebCore::CSSFontFace::setUnicodeRange): Ditto.
(WebCore::CSSFontFace::setVariantLigatures): Ditto.
(WebCore::CSSFontFace::setVariantPosition): Ditto.
(WebCore::CSSFontFace::setVariantCaps): Ditto.
(WebCore::CSSFontFace::setVariantNumeric): Ditto.
(WebCore::CSSFontFace::setVariantAlternates): Ditto.
(WebCore::CSSFontFace::setVariantEastAsian): Ditto.
(WebCore::CSSFontFace::setFeatureSettings): Ditto.
(WebCore::CSSFontFace::setStatus): Ditto.
(WebCore::CSSFontFace::notifyClientsOfFontPropertyChange): Deleted.

  • css/CSSFontFace.h: Adding a way for clients to make sure they don't register

or deregister another client.

  • css/CSSFontFaceSet.cpp:

(WebCore::CSSFontFaceSet::guardAgainstClientRegistrationChanges): Simple
ref()/deref() pair.
(WebCore::CSSFontFaceSet::stopGuardingAgainstClientRegistrationChanges):

  • css/CSSFontFaceSet.h:
  • css/FontFace.cpp: Ditto.

(WebCore::FontFace::guardAgainstClientRegistrationChanges):
(WebCore::FontFace::stopGuardingAgainstClientRegistrationChanges):

  • css/FontFace.h:
  • css/FontFaceSet.cpp:

(WebCore::FontFaceSet::faceFinished): Make sure that we only fulfil or reject
a promise once.

  • css/FontFaceSet.h:
  • dom/Document.cpp:

(WebCore::Document::fonts): The CSSFontFaces inside the CSSFontSelector get
created during style recalc. We may be in a state where there is a style
recalc pending. In order to make sure the Javascript API sees the current
state of the world, force a style recalc here (but only if one is pending).

LayoutTests:

  • fast/text/font-face-set-document-multiple-failure-expected.txt: Added.
  • fast/text/font-face-set-document-multiple-failure.html: Added.
Location:
trunk
Files:
2 added
11 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r197800 r197804  
     12016-03-08  Myles C. Maxfield  <mmaxfield@apple.com>
     2
     3        [Font Loading] Crash when a single load request causes multiple fonts to fail loading
     4        https://bugs.webkit.org/show_bug.cgi?id=155009
     5
     6        Reviewed by Simon Fraser.
     7
     8        * fast/text/font-face-set-document-multiple-failure-expected.txt: Added.
     9        * fast/text/font-face-set-document-multiple-failure.html: Added.
     10
    1112016-03-08  Ryan Haddad  <ryanhaddad@apple.com>
    212

  • trunk/Source/WebCore/ChangeLog

    r197803 r197804  
     12016-03-08  Myles C. Maxfield  <mmaxfield@apple.com>
     2
     3        [Font Loading] Crash when a single load request causes multiple fonts to fail loading
     4        https://bugs.webkit.org/show_bug.cgi?id=155009
     5
     6        Reviewed by Simon Fraser.
     7
     8        In JavaScript, the first promise fulfillment/failure wins. However, in C++, any
     9        subsequent fulfillments/failures cause a crash.
     10
     11        Test: fast/text/font-face-set-document-multiple-failure.html
     12
     13        * css/CSSFontFace.cpp:
     14        (WebCore::iterateClients): Notifying a client may cause some other client
     15        to be destroyed, thereby modifying the clients set. This function allows
     16        for notifying clients in a resilient manner.
     17        (WebCore::CSSFontFace::setStyle): Update to use iterateClients().
     18        (WebCore::CSSFontFace::setWeight): Ditto.
     19        (WebCore::CSSFontFace::setUnicodeRange): Ditto.
     20        (WebCore::CSSFontFace::setVariantLigatures): Ditto.
     21        (WebCore::CSSFontFace::setVariantPosition): Ditto.
     22        (WebCore::CSSFontFace::setVariantCaps): Ditto.
     23        (WebCore::CSSFontFace::setVariantNumeric): Ditto.
     24        (WebCore::CSSFontFace::setVariantAlternates): Ditto.
     25        (WebCore::CSSFontFace::setVariantEastAsian): Ditto.
     26        (WebCore::CSSFontFace::setFeatureSettings): Ditto.
     27        (WebCore::CSSFontFace::setStatus): Ditto.
     28        (WebCore::CSSFontFace::notifyClientsOfFontPropertyChange): Deleted.
     29        * css/CSSFontFace.h: Adding a way for clients to make sure they don't register
     30        or deregister another client.
     31        * css/CSSFontFaceSet.cpp:
     32        (WebCore::CSSFontFaceSet::guardAgainstClientRegistrationChanges): Simple
     33        ref()/deref() pair.
     34        (WebCore::CSSFontFaceSet::stopGuardingAgainstClientRegistrationChanges):
     35        * css/CSSFontFaceSet.h:
     36        * css/FontFace.cpp: Ditto.
     37        (WebCore::FontFace::guardAgainstClientRegistrationChanges):
     38        (WebCore::FontFace::stopGuardingAgainstClientRegistrationChanges):
     39        * css/FontFace.h:
     40        * css/FontFaceSet.cpp:
     41        (WebCore::FontFaceSet::faceFinished): Make sure that we only fulfil or reject
     42        a promise once.
     43        * css/FontFaceSet.h:
     44        * dom/Document.cpp:
     45        (WebCore::Document::fonts): The CSSFontFaces inside the CSSFontSelector get
     46        created during style recalc. We may be in a state where there is a style
     47        recalc pending. In order to make sure the Javascript API sees the current
     48        state of the world, force a style recalc here (but only if one is pending).
     49
    1502016-03-08  Commit Queue  <commit-queue@webkit.org>
    251

  • trunk/Source/WebCore/css/CSSFontFace.cpp

    r196954 r197804  
    4949namespace WebCore {
    5050
     51template<typename T> void iterateClients(HashSet<CSSFontFace::Client*>& clients, T callback)
     52{
     53    Vector<Ref<CSSFontFace::Client>> clientsCopy;
     54    clientsCopy.reserveInitialCapacity(clients.size());
     55    for (auto* client : clients)
     56        clientsCopy.uncheckedAppend(*client);
     57
     58    for (auto* client : clients)
     59        callback(*client);
     60}
     61
    5162void CSSFontFace::appendSources(CSSFontFace& fontFace, CSSValueList& srcList, Document* document, bool isInitiatingElementInUserAgentShadowTree)
    5263{
    … …  
    90101}
    91102
    92 void CSSFontFace::notifyClientsOfFontPropertyChange()
    93 {
    94     auto clientsCopy = m_clients;
    95     for (auto* client : clientsCopy) {
    96         if (m_clients.contains(client))
    97             client->fontPropertyChanged(*this);
    98     }
    99 }
    100 
    101103bool CSSFontFace::setFamilies(CSSValue& family)
    102104{
    … …  
    111113    m_families = &familyList;
    112114
    113     auto clientsCopy = m_clients;
    114     for (auto* client : clientsCopy) {
    115         if (m_clients.contains(client))
    116             client->fontPropertyChanged(*this, oldFamilies.get());
    117     }
     115    iterateClients(m_clients, [&](Client& client) {
     116        client.fontPropertyChanged(*this, oldFamilies.get());
     117    });
    118118
    119119    return true;
    … …  
    143143        m_traitsMask = static_cast<FontTraitsMask>((static_cast<unsigned>(m_traitsMask) & (~FontStyleMask)) | mask.value());
    144144
    145         notifyClientsOfFontPropertyChange();
     145        iterateClients(m_clients, [&](Client& client) {
     146            client.fontPropertyChanged(*this);
     147        });
    146148
    147149        return true;
    … …  
    190192        m_traitsMask = static_cast<FontTraitsMask>((static_cast<unsigned>(m_traitsMask) & (~FontWeightMask)) | mask.value());
    191193
    192         notifyClientsOfFontPropertyChange();
     194        iterateClients(m_clients, [&](Client& client) {
     195            client.fontPropertyChanged(*this);
     196        });
    193197
    194198        return true;
    … …  
    210214    }
    211215
    212     notifyClientsOfFontPropertyChange();
     216    iterateClients(m_clients, [&](Client& client) {
     217        client.fontPropertyChanged(*this);
     218    });
    213219
    214220    return true;
    … …  
    223229    m_variantSettings.contextualAlternates = ligatures.contextualAlternates;
    224230
    225     notifyClientsOfFontPropertyChange();
     231    iterateClients(m_clients, [&](Client& client) {
     232        client.fontPropertyChanged(*this);
     233    });
    226234
    227235    return true;
    … …  
    234242    m_variantSettings.position = downcast<CSSPrimitiveValue>(variantPosition);
    235243
    236     notifyClientsOfFontPropertyChange();
     244    iterateClients(m_clients, [&](Client& client) {
     245        client.fontPropertyChanged(*this);
     246    });
    237247
    238248    return true;
    … …  
    245255    m_variantSettings.caps = downcast<CSSPrimitiveValue>(variantCaps);
    246256
    247     notifyClientsOfFontPropertyChange();
     257    iterateClients(m_clients, [&](Client& client) {
     258        client.fontPropertyChanged(*this);
     259    });
    248260
    249261    return true;
    … …  
    259271    m_variantSettings.numericSlashedZero = numeric.slashedZero;
    260272
    261     notifyClientsOfFontPropertyChange();
     273    iterateClients(m_clients, [&](Client& client) {
     274        client.fontPropertyChanged(*this);
     275    });
    262276
    263277    return true;
    … …  
    270284    m_variantSettings.alternates = downcast<CSSPrimitiveValue>(variantAlternates);
    271285
    272     notifyClientsOfFontPropertyChange();
     286    iterateClients(m_clients, [&](Client& client) {
     287        client.fontPropertyChanged(*this);
     288    });
    273289
    274290    return true;
    … …  
    282298    m_variantSettings.eastAsianRuby = eastAsian.ruby;
    283299
    284     notifyClientsOfFontPropertyChange();
     300    iterateClients(m_clients, [&](Client& client) {
     301        client.fontPropertyChanged(*this);
     302    });
    285303
    286304    return true;
    … …  
    299317    }
    300318
    301     notifyClientsOfFontPropertyChange();
     319    iterateClients(m_clients, [&](Client& client) {
     320        client.fontPropertyChanged(*this);
     321    });
    302322
    303323    return true;
    … …  
    381401    }
    382402
    383     for (auto* client : m_clients)
    384         client->fontStateChanged(*this, m_status, newStatus);
     403    iterateClients(m_clients, [&](Client& client) {
     404        client.fontStateChanged(*this, m_status, newStatus);
     405    });
    385406
    386407    m_status = newStatus;
    … …  
    398419    m_fontSelector->fontLoaded();
    399420
    400     for (auto* client : m_clients)
    401         client->fontLoaded(*this);
     421    iterateClients(m_clients, [&](Client& client) {
     422        client.fontLoaded(*this);
     423    });
    402424}
    403425

  • trunk/Source/WebCore/css/CSSFontFace.h

    r196954 r197804  
    109109    public:
    110110        virtual ~Client() { }
    111         virtual void fontLoaded(CSSFontFace&) { };
    112         virtual void fontStateChanged(CSSFontFace&, Status oldState, Status newState) { UNUSED_PARAM(oldState); UNUSED_PARAM(newState); };
    113         virtual void fontPropertyChanged(CSSFontFace&, CSSValueList* oldFamilies = nullptr) { UNUSED_PARAM(oldFamilies); };
     111        virtual void fontLoaded(CSSFontFace&) { }
     112        virtual void fontStateChanged(CSSFontFace&, Status oldState, Status newState) { UNUSED_PARAM(oldState); UNUSED_PARAM(newState); }
     113        virtual void fontPropertyChanged(CSSFontFace&, CSSValueList* oldFamilies = nullptr) { UNUSED_PARAM(oldFamilies); }
     114        virtual void ref() = 0;
     115        virtual void deref() = 0;
    114116    };
    115117

  • trunk/Source/WebCore/css/CSSFontFaceSet.cpp

    r197456 r197804  
    399399    auto& familyFontFaces = iterator->value;
    400400
    401     auto& segmentedFontFaceCache = m_cache.add(family, HashMap<unsigned, std::unique_ptr<CSSSegmentedFontFace>>()).iterator->value;
     401    auto& segmentedFontFaceCache = m_cache.add(family, HashMap<unsigned, RefPtr<CSSSegmentedFontFace>>()).iterator->value;
    402402
    403403    auto& face = segmentedFontFaceCache.add(traitsMask, nullptr).iterator->value;
    … …  
    405405        return face.get();
    406406
    407     face = std::make_unique<CSSSegmentedFontFace>();
     407    face = CSSSegmentedFontFace::create();
    408408
    409409    Vector<std::reference_wrapper<CSSFontFace>, 32> candidateFontFaces;

  • trunk/Source/WebCore/css/CSSFontFaceSet.h

    r197563 r197804  
    7676    Vector<std::reference_wrapper<CSSFontFace>> matchingFaces(const String& font, const String& text, ExceptionCode&);
    7777
     78    // CSSFontFace::Client needs to be able to be held in a RefPtr.
     79    void ref() override { RefCounted<CSSFontFaceSet>::ref(); }
     80    void deref() override { RefCounted<CSSFontFaceSet>::deref(); }
     81
    7882private:
    7983    CSSFontFaceSet();
    … …  
    96100    HashMap<String, Vector<Ref<CSSFontFace>>, ASCIICaseInsensitiveHash> m_facesLookupTable;
    97101    HashMap<String, Vector<Ref<CSSFontFace>>, ASCIICaseInsensitiveHash> m_locallyInstalledFacesLookupTable;
    98     HashMap<String, HashMap<unsigned, std::unique_ptr<CSSSegmentedFontFace>>, ASCIICaseInsensitiveHash> m_cache;
     102    HashMap<String, HashMap<unsigned, RefPtr<CSSSegmentedFontFace>>, ASCIICaseInsensitiveHash> m_cache;
    99103    size_t m_facesPartitionIndex { 0 }; // All entries in m_faces before this index are CSS-connected.
    100104    Status m_status { Status::Loaded };

  • trunk/Source/WebCore/css/CSSSegmentedFontFace.h

    r197563 r197804  
    4040class FontDescription;
    4141
    42 class CSSSegmentedFontFace final : public CSSFontFace::Client {
     42class CSSSegmentedFontFace final : public RefCounted<CSSSegmentedFontFace>, public CSSFontFace::Client {
    4343    WTF_MAKE_FAST_ALLOCATED;
    4444public:
    45     CSSSegmentedFontFace();
     45    static Ref<CSSSegmentedFontFace> create()
     46    {
     47        return adoptRef(*new CSSSegmentedFontFace());
     48    }
    4649    ~CSSSegmentedFontFace();
    4750
    … …  
    5255    Vector<Ref<CSSFontFace>, 1>& constituentFaces() { return m_fontFaces; }
    5356
     57    // CSSFontFace::Client needs to be able to be held in a RefPtr.
     58    void ref() override { RefCounted<CSSSegmentedFontFace>::ref(); }
     59    void deref() override { RefCounted<CSSSegmentedFontFace>::deref(); }
     60
    5461private:
     62    CSSSegmentedFontFace();
    5563    void fontLoaded(CSSFontFace&) override;
    5664

  • trunk/Source/WebCore/css/FontFace.h

    r197563 r197804  
    8383    WeakPtr<FontFace> createWeakPtr() const;
    8484
     85    // CSSFontFace::Client needs to be able to be held in a RefPtr.
     86    void ref() override { RefCounted<FontFace>::ref(); }
     87    void deref() override { RefCounted<FontFace>::deref(); }
     88
    8589private:
    8690    FontFace(JSC::ExecState&, CSSFontSelector&);

  • trunk/Source/WebCore/css/FontFaceSet.cpp

    r196973 r197804  
    237237    for (auto& pendingPromise : iterator->value) {
    238238        if (newStatus == CSSFontFace::Status::Success) {
    239             if (pendingPromise->hasOneRef())
     239            if (pendingPromise->hasOneRef() && !pendingPromise->hasReachedTerminalState) {
    240240                pendingPromise->promise.resolve(pendingPromise->faces);
     241                pendingPromise->hasReachedTerminalState = true;
     242            }
    241243        } else {
    242244            ASSERT(newStatus == CSSFontFace::Status::Failure);
    243             // The first resolution wins, so we can just reject early now.
    244             pendingPromise->promise.reject(DOMCoreException::create(ExceptionCodeDescription(NETWORK_ERR)));
     245            if (!pendingPromise->hasReachedTerminalState) {
     246                pendingPromise->promise.reject(DOMCoreException::create(ExceptionCodeDescription(NETWORK_ERR)));
     247                pendingPromise->hasReachedTerminalState = true;
     248            }
    245249        }
    246250    }

  • trunk/Source/WebCore/css/FontFaceSet.h

    r197563 r197804  
    102102        Vector<RefPtr<FontFace>> faces;
    103103        Promise promise;
     104        bool hasReachedTerminalState { false };
    104105    };
    105106

  • trunk/Source/WebCore/dom/Document.cpp

    r197764 r197804  
    67076707Ref<FontFaceSet> Document::fonts()
    67086708{
     6709    updateStyleIfNeeded();
    67096710    return fontSelector().fontFaceSet();
    67106711}
Note: See TracChangeset for help on using the changeset viewer.