Changeset 201906 in webkit


Ignore:
Timestamp:
Jun 9, 2016 10:24:46 PM (8 years ago)
Author:
Ryan Haddad
Message:

Unreviewed, rolling out r201887.
​https://bugs.webkit.org/show_bug.cgi?id=158610

This change caused LayoutTest crashes under GuardMalloc and
ASan (Requested by ryanhaddad on #webkit).

Reverted changeset:

"Deleting a CSSOM style rule invalidates any previously-added
FontFaces"
​https://bugs.webkit.org/show_bug.cgi?id=158450
​http://trac.webkit.org/changeset/201887

Patch by Commit Queue <​commit-queue@webkit.org> on 2016-06-09

Location:
trunk
Files:
2 deleted
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r201904 r201906  
     12016-06-09  Commit Queue  <commit-queue@webkit.org>
     2
     3        Unreviewed, rolling out r201887.
     4        https://bugs.webkit.org/show_bug.cgi?id=158610
     5
     6        This change caused LayoutTest crashes under GuardMalloc and
     7        ASan (Requested by ryanhaddad on #webkit).
     8
     9        Reverted changeset:
     10
     11        "Deleting a CSSOM style rule invalidates any previously-added
     12        FontFaces"
     13        https://bugs.webkit.org/show_bug.cgi?id=158450
     14        http://trac.webkit.org/changeset/201887
     15
    1162016-06-09  Zalan Bujtas  <zalan@apple.com>
    217

  • trunk/Source/WebCore/ChangeLog

    r201903 r201906  
     12016-06-09  Commit Queue  <commit-queue@webkit.org>
     2
     3        Unreviewed, rolling out r201887.
     4        https://bugs.webkit.org/show_bug.cgi?id=158610
     5
     6        This change caused LayoutTest crashes under GuardMalloc and
     7        ASan (Requested by ryanhaddad on #webkit).
     8
     9        Reverted changeset:
     10
     11        "Deleting a CSSOM style rule invalidates any previously-added
     12        FontFaces"
     13        https://bugs.webkit.org/show_bug.cgi?id=158450
     14        http://trac.webkit.org/changeset/201887
     15
    1162016-06-09  Chris Dumez  <cdumez@apple.com>
    217

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

    r201887 r201906  
    9595    , m_wrapper(wrapper ? wrapper->createWeakPtr() : WeakPtr<FontFace>())
    9696    , m_isLocalFallback(isLocalFallback)
    97     , m_mayBePurged(!wrapper)
    9897{
    9998}
    … …  
    114113    RefPtr<CSSValueList> oldFamilies = m_families;
    115114    m_families = &familyList;
    116 
    117     if (m_cssConnection)
    118         m_cssConnection->mutableProperties().setProperty(CSSPropertyFontFamily, &family);
    119115
    120116    iterateClients(m_clients, [&](Client& client) {
    … …  
    147143    if (auto mask = calculateStyleMask(style)) {
    148144        m_traitsMask = static_cast<FontTraitsMask>((static_cast<unsigned>(m_traitsMask) & (~FontStyleMask)) | mask.value());
    149 
    150         if (m_cssConnection)
    151             m_cssConnection->mutableProperties().setProperty(CSSPropertyFontStyle, &style);
    152145
    153146        iterateClients(m_clients, [&](Client& client) {
    … …  
    200193        m_traitsMask = static_cast<FontTraitsMask>((static_cast<unsigned>(m_traitsMask) & (~FontWeightMask)) | mask.value());
    201194
    202         if (m_cssConnection)
    203             m_cssConnection->mutableProperties().setProperty(CSSPropertyFontWeight, &weight);
    204 
    205195        iterateClients(m_clients, [&](Client& client) {
    206196            client.fontPropertyChanged(*this);
    … …  
    225215    }
    226216
    227     if (m_cssConnection)
    228         m_cssConnection->mutableProperties().setProperty(CSSPropertyUnicodeRange, &unicodeRange);
    229 
    230217    iterateClients(m_clients, [&](Client& client) {
    231218        client.fontPropertyChanged(*this);
    … …  
    244231    m_variantSettings.contextualAlternates = ligatures.contextualAlternates;
    245232
    246     if (m_cssConnection)
    247         m_cssConnection->mutableProperties().setProperty(CSSPropertyFontVariantLigatures, &variantLigatures);
    248 
    249233    iterateClients(m_clients, [&](Client& client) {
    250234        client.fontPropertyChanged(*this);
    … …  
    261245    m_variantSettings.position = downcast<CSSPrimitiveValue>(variantPosition);
    262246
    263     if (m_cssConnection)
    264         m_cssConnection->mutableProperties().setProperty(CSSPropertyFontVariantPosition, &variantPosition);
    265 
    266247    iterateClients(m_clients, [&](Client& client) {
    267248        client.fontPropertyChanged(*this);
    … …  
    277258
    278259    m_variantSettings.caps = downcast<CSSPrimitiveValue>(variantCaps);
    279 
    280     if (m_cssConnection)
    281         m_cssConnection->mutableProperties().setProperty(CSSPropertyFontVariantCaps, &variantCaps);
    282260
    283261    iterateClients(m_clients, [&](Client& client) {
    … …  
    298276    m_variantSettings.numericSlashedZero = numeric.slashedZero;
    299277
    300     if (m_cssConnection)
    301         m_cssConnection->mutableProperties().setProperty(CSSPropertyFontVariantNumeric, &variantNumeric);
    302 
    303278    iterateClients(m_clients, [&](Client& client) {
    304279        client.fontPropertyChanged(*this);
    … …  
    314289
    315290    m_variantSettings.alternates = downcast<CSSPrimitiveValue>(variantAlternates);
    316 
    317     if (m_cssConnection)
    318         m_cssConnection->mutableProperties().setProperty(CSSPropertyFontVariantAlternates, &variantAlternates);
    319291
    320292    iterateClients(m_clients, [&](Client& client) {
    … …  
    332304    m_variantSettings.eastAsianWidth = eastAsian.width;
    333305    m_variantSettings.eastAsianRuby = eastAsian.ruby;
    334 
    335     if (m_cssConnection)
    336         m_cssConnection->mutableProperties().setProperty(CSSPropertyFontVariantEastAsian, &variantEastAsian);
    337306
    338307    iterateClients(m_clients, [&](Client& client) {
    … …  
    363332    m_featureSettings = WTFMove(settings);
    364333
    365     if (m_cssConnection)
    366         m_cssConnection->mutableProperties().setProperty(CSSPropertyFontFeatureSettings, &featureSettings);
    367 
    368334    iterateClients(m_clients, [&](Client& client) {
    369335        client.fontPropertyChanged(*this);
    … …  
    416382}
    417383
    418 void CSSFontFace::initializeWrapper()
    419 {
     384Ref<FontFace> CSSFontFace::wrapper()
     385{
     386    if (m_wrapper)
     387        return Ref<FontFace>(*m_wrapper.get());
     388
     389    Ref<FontFace> wrapper = FontFace::create(*this);
    420390    switch (m_status) {
    421391    case Status::Pending:
    422392        break;
    423393    case Status::Loading:
    424         m_wrapper->fontStateChanged(*this, Status::Pending, Status::Loading);
     394        wrapper->fontStateChanged(*this, Status::Pending, Status::Loading);
    425395        break;
    426396    case Status::TimedOut:
    427         m_wrapper->fontStateChanged(*this, Status::Pending, Status::Loading);
    428         m_wrapper->fontStateChanged(*this, Status::Loading, Status::TimedOut);
     397        wrapper->fontStateChanged(*this, Status::Pending, Status::Loading);
     398        wrapper->fontStateChanged(*this, Status::Loading, Status::TimedOut);
    429399        break;
    430400    case Status::Success:
    431         m_wrapper->fontStateChanged(*this, Status::Pending, Status::Loading);
    432         m_wrapper->fontStateChanged(*this, Status::Pending, Status::Success);
     401        wrapper->fontStateChanged(*this, Status::Pending, Status::Loading);
     402        wrapper->fontStateChanged(*this, Status::Pending, Status::Success);
    433403        break;
    434404    case Status::Failure:
    435         m_wrapper->fontStateChanged(*this, Status::Pending, Status::Loading);
    436         m_wrapper->fontStateChanged(*this, Status::Pending, Status::Failure);
    437         break;
    438     }
    439 }
    440 
    441 Ref<FontFace> CSSFontFace::wrapper()
    442 {
    443     if (m_wrapper)
    444         return *m_wrapper.get();
    445 
    446     auto wrapper = FontFace::create(*this);
     405        wrapper->fontStateChanged(*this, Status::Pending, Status::Loading);
     406        wrapper->fontStateChanged(*this, Status::Pending, Status::Failure);
     407        break;
     408    }
    447409    m_wrapper = wrapper->createWeakPtr();
    448     initializeWrapper();
    449     m_mayBePurged = false;
    450410    return wrapper;
    451 }
    452 
    453 void CSSFontFace::setWrapper(FontFace& newWrapper)
    454 {
    455     m_wrapper = newWrapper.createWeakPtr();
    456     initializeWrapper();
    457411}
    458412
    … …  
    599553}
    600554
    601 bool CSSFontFace::purgeable() const
    602 {
    603     return cssConnection() && m_mayBePurged;
    604 }
    605 
    606 void CSSFontFace::updateStyleIfNeeded()
    607 {
    608     if (m_fontSelector && m_fontSelector->document())
    609         m_fontSelector->document()->updateStyleIfNeeded();
    610 }
    611 
    612555#if ENABLE(SVG_FONTS)
    613556bool CSSFontFace::hasSVGFontFaceSource() const

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

    r201887 r201906  
    136136    // We don't guarantee that the FontFace wrapper will be the same every time you ask for it.
    137137    Ref<FontFace> wrapper();
    138     void setWrapper(FontFace&);
    139     FontFace* existingWrapper() { return m_wrapper.get(); }
    140138
    141139    bool webFontsShouldAlwaysFallBack() const;
    142 
    143     bool purgeable() const;
    144 
    145     void updateStyleIfNeeded();
    146140
    147141#if ENABLE(SVG_FONTS)
    … …  
    155149    void setStatus(Status);
    156150    void notifyClientsOfFontPropertyChange();
    157 
    158     void initializeWrapper();
    159151
    160152    void fontLoadEventOccurred();
    … …  
    175167    bool m_isLocalFallback { false };
    176168    bool m_sourcesPopulated { false };
    177     bool m_mayBePurged { true };
    178169};
    179170

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

    r201887 r201906  
    9696}
    9797
    98 void CSSFontFaceSet::ensureLocalFontFacesForFamilyRegistered(const String& familyName)
    99 {
    100     if (m_locallyInstalledFacesLookupTable.contains(familyName))
    101         return;
     98void CSSFontFaceSet::registerLocalFontFacesForFamily(const String& familyName)
     99{
     100    ASSERT(!m_locallyInstalledFacesLookupTable.contains(familyName));
    102101
    103102    Vector<FontTraitsMask> traitsMasks = FontCache::singleton().getTraitsInFamily(familyName);
    … …  
    162161            // m_locallyInstalledFontFaces grows without bound, eventually encorporating every font installed on the system.
    163162            // This is by design.
    164             ensureLocalFontFacesForFamilyRegistered(familyName);
     163            registerLocalFontFacesForFamily(familyName);
    165164            familyFontFaces = { };
    166165        }
    … …  
    189188    if (face.status() == CSSFontFace::Status::Loading || face.status() == CSSFontFace::Status::TimedOut)
    190189        incrementActiveCount();
    191 
    192     if (face.cssConnection()) {
    193         auto addResult = m_constituentCSSConnections.add(face.cssConnection(), &face);
    194         ASSERT_UNUSED(addResult, addResult.isNewEntry);
    195     }
    196190}
    197191
    … …  
    229223        removeFromFacesLookupTable(face, *face.families());
    230224
    231     if (face.cssConnection()) {
    232         bool removed = m_constituentCSSConnections.remove(face.cssConnection());
    233         ASSERT_UNUSED(removed, removed);
    234     }
    235 
    236225    for (size_t i = 0; i < m_faces.size(); ++i) {
    237226        if (m_faces[i].ptr() == &face) {
    … …  
    248237}
    249238
    250 CSSFontFace* CSSFontFaceSet::lookupByCSSConnection(StyleRuleFontFace& target)
    251 {
    252     return m_constituentCSSConnections.get(&target);
    253 }
    254 
    255 void CSSFontFaceSet::purge()
    256 {
    257     Vector<std::reference_wrapper<CSSFontFace>> toRemove;
    258     for (auto& face : m_faces) {
    259         if (face->purgeable())
    260             toRemove.append(face.get());
    261     }
    262 
    263     for (auto& item : toRemove)
    264         remove(item.get());
    265 }
    266 
    267239void CSSFontFaceSet::clear()
    268240{
    … …  
    273245    m_locallyInstalledFacesLookupTable.clear();
    274246    m_cache.clear();
    275     m_constituentCSSConnections.clear();
    276247    m_facesPartitionIndex = 0;
    277248    m_status = Status::Loaded;

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

    r201887 r201906  
    6161    void add(CSSFontFace&);
    6262    void remove(const CSSFontFace&);
    63     void purge();
    6463    void clear();
    6564    CSSFontFace& operator[](size_t i);
    66 
    67     CSSFontFace* lookupByCSSConnection(StyleRuleFontFace&);
    6865
    6966    bool check(const String& font, const String& text, ExceptionCode&);
    … …  
    9592    void fontPropertyChanged(CSSFontFace&, CSSValueList* oldFamilies = nullptr) override;
    9693
    97     void ensureLocalFontFacesForFamilyRegistered(const String&);
     94    void registerLocalFontFacesForFamily(const String&);
    9895
    9996    static String familyNameFromPrimitive(const CSSPrimitiveValue&);
    … …  
    104101    HashMap<String, Vector<Ref<CSSFontFace>>, ASCIICaseInsensitiveHash> m_locallyInstalledFacesLookupTable;
    105102    HashMap<String, HashMap<unsigned, RefPtr<CSSSegmentedFontFace>>, ASCIICaseInsensitiveHash> m_cache;
    106     HashMap<StyleRuleFontFace*, CSSFontFace*> m_constituentCSSConnections;
    107103    size_t m_facesPartitionIndex { 0 }; // All entries in m_faces before this index are CSS-connected.
    108104    Status m_status { Status::Loaded };

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

    r201887 r201906  
    4646#include "Font.h"
    4747#include "FontCache.h"
    48 #include "FontFace.h"
    4948#include "FontFaceSet.h"
    5049#include "FontSelectorClient.h"
    … …  
    103102{
    104103    m_buildIsUnderway = true;
    105     m_stagingArea.clear();
    106     m_cssFontFaceSet->purge();
    107104    ++m_version;
    108 
    109     m_cssConnectionsPossiblyToRemove.clear();
    110     m_cssConnectionsEncounteredDuringBuild.clear();
    111     for (size_t i = 0; i < m_cssFontFaceSet->faceCount(); ++i) {
    112         CSSFontFace& face = m_cssFontFaceSet.get()[i];
    113         if (face.cssConnection())
    114             m_cssConnectionsPossiblyToRemove.add(&face);
    115     }
    116105}
    117106
    … …  
    123112    m_buildIsUnderway = false;
    124113
    125     // Some font faces weren't re-added during the build process.
    126     for (auto& face : m_cssConnectionsPossiblyToRemove) {
    127         auto* connection = face->cssConnection();
    128         ASSERT(connection);
    129         if (!m_cssConnectionsEncounteredDuringBuild.contains(connection))
    130             m_cssFontFaceSet->remove(*face);
    131     }
     114    m_cssFontFaceSet->clear();
    132115
    133116    for (auto& item : m_stagingArea)
    … …  
    139122{
    140123    if (m_buildIsUnderway) {
    141         m_cssConnectionsEncounteredDuringBuild.add(&fontFaceRule);
    142124        m_stagingArea.append({fontFaceRule, isInitiatingElementInUserAgentShadowTree});
    143125        return;
    … …  
    205187    if (fontFace->allSourcesFailed())
    206188        return;
    207 
    208     if (RefPtr<CSSFontFace> existingFace = m_cssFontFaceSet->lookupByCSSConnection(fontFaceRule)) {
    209         m_cssFontFaceSet->remove(*existingFace);
    210         if (auto* existingWrapper = existingFace->existingWrapper())
    211             existingWrapper->adopt(fontFace.get());
    212     }
    213189
    214190    m_cssFontFaceSet->add(fontFace.get());

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

    r201887 r201906  
    106106
    107107    Vector<CachedResourceHandle<CachedFont>> m_fontsToBeginLoading;
    108     HashSet<RefPtr<CSSFontFace>> m_cssConnectionsPossiblyToRemove;
    109     HashSet<RefPtr<StyleRuleFontFace>> m_cssConnectionsEncounteredDuringBuild;
    110108    Timer m_beginLoadingTimer;
    111109

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

    r201887 r201906  
    271271String FontFace::family() const
    272272{
    273     const_cast<CSSFontFace&>(m_backing.get()).updateStyleIfNeeded();
    274273    return m_backing->families()->cssText();
    275274}
    … …  
    277276String FontFace::style() const
    278277{
    279     const_cast<CSSFontFace&>(m_backing.get()).updateStyleIfNeeded();
    280278    switch (m_backing->traitsMask() & FontStyleMask) {
    281279    case FontStyleNormalMask:
    … …  
    290288String FontFace::weight() const
    291289{
    292     const_cast<CSSFontFace&>(m_backing.get()).updateStyleIfNeeded();
    293290    switch (m_backing->traitsMask() & FontWeightMask) {
    294291    case FontWeight100Mask:
    … …  
    322319String FontFace::unicodeRange() const
    323320{
    324     const_cast<CSSFontFace&>(m_backing.get()).updateStyleIfNeeded();
    325321    if (!m_backing->ranges().size())
    326322        return "U+0-10FFFF";
    … …  
    333329String FontFace::variant() const
    334330{
    335     const_cast<CSSFontFace&>(m_backing.get()).updateStyleIfNeeded();
    336331    return computeFontVariant(m_backing->variantSettings())->cssText();
    337332}
    … …  
    339334String FontFace::featureSettings() const
    340335{
    341     const_cast<CSSFontFace&>(m_backing.get()).updateStyleIfNeeded();
    342336    if (!m_backing->featureSettings().size())
    343337        return "normal";
    … …  
    364358    ASSERT_NOT_REACHED();
    365359    return LoadStatus::Error;
    366 }
    367 
    368 void FontFace::adopt(CSSFontFace& newFace)
    369 {
    370     m_promise = Nullopt;
    371     m_backing->removeClient(*this);
    372     m_backing = newFace;
    373     m_backing->addClient(*this);
    374     newFace.setWrapper(*this);
    375360}
    376361

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

    r201887 r201906  
    7070    void registerLoaded(Promise&&);
    7171
    72     void adopt(CSSFontFace&);
    73 
    7472    void load();
    7573
Note: See TracChangeset for help on using the changeset viewer.