Changeset 206603 in webkit


Ignore:
Timestamp:
Sep 29, 2016, 12:36:39 PM (9 years ago)
Author:
Antti Koivisto
Message:

Remove addSubresourceStyleURLs functions
​https://bugs.webkit.org/show_bug.cgi?id=162731

Reviewed by Ryosuke Niwa.

Use the generic std::function taking traverseSubresources instead. This prevents bugs caused by the code paths
not being in sync.

These functions are only used by the legacy webarchive code to gather URLs to locate CachedResources from the memory cache.
This can be improved further by returning the cached resources themselves instead of the URLs.

  • css/CSSFontFaceSrcValue.cpp:

(WebCore::CSSFontFaceSrcValue::addSubresourceStyleURLs): Deleted.

  • css/CSSFontFaceSrcValue.h:
  • css/CSSPrimitiveValue.cpp:

(WebCore::CSSPrimitiveValue::addSubresourceStyleURLs): Deleted.

  • css/CSSPrimitiveValue.h:
  • css/CSSReflectValue.cpp:

(WebCore::CSSReflectValue::addSubresourceStyleURLs): Deleted.

  • css/CSSReflectValue.h:
  • css/CSSValue.cpp:

(WebCore::CSSValue::addSubresourceStyleURLs): Deleted.

  • css/CSSValue.h:
  • css/CSSValueList.cpp:

(WebCore::CSSValueList::addSubresourceStyleURLs): Deleted.

  • css/CSSValueList.h:
  • css/StyleProperties.cpp:

(WebCore::StyleProperties::addSubresourceStyleURLs): Deleted.

  • css/StyleProperties.h:
  • css/StyleRuleImport.h:
  • css/StyleSheetContents.cpp:

(WebCore::StyleSheetContents::traverseSubresources):

Fix a bug where this would miss @import rules in @imported stylesheets.
Include the CachedResource for the imported stylesheet itself.

Tested by the test cases under LayoutTests/webarchive

(WebCore::StyleSheetContents::addSubresourceStyleURLs): Deleted.

  • css/StyleSheetContents.h:
  • dom/StyledElement.cpp:

(WebCore::StyledElement::addSubresourceAttributeURLs):

  • html/HTMLLinkElement.cpp:

(WebCore::HTMLLinkElement::addSubresourceAttributeURLs):

  • html/HTMLStyleElement.cpp:

(WebCore::HTMLStyleElement::addSubresourceAttributeURLs):

Location:
trunk/Source/WebCore
Files:
19 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r206597 r206603  
     12016-09-29  Antti Koivisto  <antti@apple.com>
     2
     3        Remove addSubresourceStyleURLs functions
     4        https://bugs.webkit.org/show_bug.cgi?id=162731
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        Use the generic std::function taking traverseSubresources instead. This prevents bugs caused by the code paths
     9        not being in sync.
     10
     11        These functions are only used by the legacy webarchive code to gather URLs to locate CachedResources from the memory cache.
     12        This can be improved further by returning the cached resources themselves instead of the URLs.
     13
     14        * css/CSSFontFaceSrcValue.cpp:
     15        (WebCore::CSSFontFaceSrcValue::addSubresourceStyleURLs): Deleted.
     16        * css/CSSFontFaceSrcValue.h:
     17        * css/CSSPrimitiveValue.cpp:
     18        (WebCore::CSSPrimitiveValue::addSubresourceStyleURLs): Deleted.
     19        * css/CSSPrimitiveValue.h:
     20        * css/CSSReflectValue.cpp:
     21        (WebCore::CSSReflectValue::addSubresourceStyleURLs): Deleted.
     22        * css/CSSReflectValue.h:
     23        * css/CSSValue.cpp:
     24        (WebCore::CSSValue::addSubresourceStyleURLs): Deleted.
     25        * css/CSSValue.h:
     26        * css/CSSValueList.cpp:
     27        (WebCore::CSSValueList::addSubresourceStyleURLs): Deleted.
     28        * css/CSSValueList.h:
     29        * css/StyleProperties.cpp:
     30        (WebCore::StyleProperties::addSubresourceStyleURLs): Deleted.
     31        * css/StyleProperties.h:
     32        * css/StyleRuleImport.h:
     33        * css/StyleSheetContents.cpp:
     34        (WebCore::StyleSheetContents::traverseSubresources):
     35
     36            Fix a bug where this would miss @import rules in @imported stylesheets.
     37            Include the CachedResource for the imported stylesheet itself.
     38
     39            Tested by the test cases under LayoutTests/webarchive
     40
     41        (WebCore::StyleSheetContents::addSubresourceStyleURLs): Deleted.
     42        * css/StyleSheetContents.h:
     43        * dom/StyledElement.cpp:
     44        (WebCore::StyledElement::addSubresourceAttributeURLs):
     45        * html/HTMLLinkElement.cpp:
     46        (WebCore::HTMLLinkElement::addSubresourceAttributeURLs):
     47        * html/HTMLStyleElement.cpp:
     48        (WebCore::HTMLStyleElement::addSubresourceAttributeURLs):
     49
    1502016-09-29  Brent Fulgham  <bfulgham@apple.com>
    251

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

    r206016 r206603  
    8686}
    8787
    88 void CSSFontFaceSrcValue::addSubresourceStyleURLs(ListHashSet<URL>& urls, const StyleSheetContents* styleSheet) const
    89 {
    90     if (!isLocal())
    91         addSubresourceURL(urls, styleSheet->completeURL(m_resource));
    92 }
    93 
    9488bool CSSFontFaceSrcValue::traverseSubresources(const std::function<bool (const CachedResource&)>& handler) const
    9589{

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

    r205093 r206603  
    6666    String customCSSText() const;
    6767
    68     void addSubresourceStyleURLs(ListHashSet<URL>&, const StyleSheetContents*) const;
    69 
    7068    bool traverseSubresources(const std::function<bool (const CachedResource&)>& handler) const;
    7169

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

    r206043 r206603  
    12651265}
    12661266
    1267 void CSSPrimitiveValue::addSubresourceStyleURLs(ListHashSet<URL>& urls, const StyleSheetContents* styleSheet) const
    1268 {
    1269     if (m_primitiveUnitType == CSS_URI)
    1270         addSubresourceURL(urls, styleSheet->completeURL(m_value.string));
    1271 }
    1272 
    12731267Ref<CSSPrimitiveValue> CSSPrimitiveValue::cloneForCSSOM() const
    12741268{

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

    r206043 r206603  
    382382    bool isQuirkValue() const { return m_isQuirkValue || primitiveType() == CSS_QUIRKY_EMS; }
    383383
    384     void addSubresourceStyleURLs(ListHashSet<URL>&, const StyleSheetContents*) const;
    385 
    386384    Ref<CSSPrimitiveValue> cloneForCSSOM() const;
    387385    void setCSSOMSafe() { m_isCSSOMSafe = true; }

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

    r201290 r206603  
    3939}
    4040
    41 void CSSReflectValue::addSubresourceStyleURLs(ListHashSet<URL>& urls, const StyleSheetContents* styleSheet) const
    42 {
    43     if (m_mask)
    44         m_mask->addSubresourceStyleURLs(urls, styleSheet);
    45 }
    46 
    4741bool CSSReflectValue::equals(const CSSReflectValue& other) const
    4842{

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

    r205093 r206603  
    4949    String customCSSText() const;
    5050
    51     void addSubresourceStyleURLs(ListHashSet<URL>&, const StyleSheetContents*) const;
    52 
    5351    bool equals(const CSSReflectValue&) const;
    5452

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

    r205869 r206603  
    122122        return CSS_REVERT;
    123123    return CSS_CUSTOM;
    124 }
    125 
    126 void CSSValue::addSubresourceStyleURLs(ListHashSet<URL>& urls, const StyleSheetContents* styleSheet) const
    127 {
    128     // This should get called for internal instances only.
    129     ASSERT(!isCSSOMSafe());
    130 
    131     if (is<CSSPrimitiveValue>(*this))
    132         downcast<CSSPrimitiveValue>(*this).addSubresourceStyleURLs(urls, styleSheet);
    133     else if (is<CSSValueList>(*this))
    134         downcast<CSSValueList>(*this).addSubresourceStyleURLs(urls, styleSheet);
    135     else if (is<CSSFontFaceSrcValue>(*this))
    136         downcast<CSSFontFaceSrcValue>(*this).addSubresourceStyleURLs(urls, styleSheet);
    137     else if (is<CSSReflectValue>(*this))
    138         downcast<CSSReflectValue>(*this).addSubresourceStyleURLs(urls, styleSheet);
    139124}
    140125

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

    r205869 r206603  
    138138    RefPtr<CSSValue> cloneForCSSOM() const;
    139139
    140     void addSubresourceStyleURLs(ListHashSet<URL>&, const StyleSheetContents*) const;
    141 
    142140    bool traverseSubresources(const std::function<bool (const CachedResource&)>& handler) const;
    143141

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

    r201690 r206603  
    156156}
    157157
    158 void CSSValueList::addSubresourceStyleURLs(ListHashSet<URL>& urls, const StyleSheetContents* styleSheet) const
    159 {
    160     for (unsigned i = 0, size = m_values.size(); i < size; ++i)
    161         m_values[i].get().addSubresourceStyleURLs(urls, styleSheet);
    162 }
    163 
    164158bool CSSValueList::traverseSubresources(const std::function<bool (const CachedResource&)>& handler) const
    165159{

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

    r204466 r206603  
    7373    bool equals(const CSSValue&) const;
    7474
    75     void addSubresourceStyleURLs(ListHashSet<URL>&, const StyleSheetContents*) const;
    76 
    7775    bool traverseSubresources(const std::function<bool (const CachedResource&)>& handler) const;
    7876   

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

    r205809 r206603  
    11141114}
    11151115
    1116 void StyleProperties::addSubresourceStyleURLs(ListHashSet<URL>& urls, StyleSheetContents* contextStyleSheet) const
    1117 {
    1118     unsigned size = propertyCount();
    1119     for (unsigned i = 0; i < size; ++i)
    1120         propertyAt(i).value()->addSubresourceStyleURLs(urls, contextStyleSheet);
    1121 }
    1122 
    11231116bool StyleProperties::traverseSubresources(const std::function<bool (const CachedResource&)>& handler) const
    11241117{

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

    r205660 r206603  
    9797    CSSParserMode cssParserMode() const { return static_cast<CSSParserMode>(m_cssParserMode); }
    9898
    99     void addSubresourceStyleURLs(ListHashSet<URL>&, StyleSheetContents* contextStyleSheet) const;
    100 
    10199    WEBCORE_EXPORT Ref<MutableStyleProperties> mutableCopy() const;
    102100    Ref<ImmutableStyleProperties> immutableCopyIfNeeded() const;

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

    r205093 r206603  
    5252
    5353    void requestStyleSheet();
     54    const CachedCSSStyleSheet* cachedCSSStyleSheet() const { return m_cachedSheet.get(); }
    5455
    5556private:

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

    r205790 r206603  
    409409{
    410410    return CSSParser::completeURL(m_parserContext, url);
    411 }
    412 
    413 void StyleSheetContents::addSubresourceStyleURLs(ListHashSet<URL>& urls)
    414 {
    415     Deque<StyleSheetContents*> styleSheetQueue;
    416     styleSheetQueue.append(this);
    417 
    418     while (!styleSheetQueue.isEmpty()) {
    419         StyleSheetContents* styleSheet = styleSheetQueue.takeFirst();
    420        
    421         for (auto& importRule : styleSheet->m_importRules) {
    422             if (importRule->styleSheet()) {
    423                 styleSheetQueue.append(importRule->styleSheet());
    424                 addSubresourceURL(urls, importRule->styleSheet()->baseURL());
    425             }
    426         }
    427         for (auto& rule : styleSheet->m_childRules) {
    428             if (is<StyleRule>(*rule))
    429                 downcast<StyleRule>(*rule).properties().addSubresourceStyleURLs(urls, this);
    430             else if (is<StyleRuleFontFace>(*rule))
    431                 downcast<StyleRuleFontFace>(*rule).properties().addSubresourceStyleURLs(urls, this);
    432         }
    433     }
    434411}
    435412
    … …  
    478455{
    479456    for (auto& importRule : m_importRules) {
    480         if (!importRule->styleSheet())
    481             continue;
    482         if (traverseSubresourcesInRules(importRule->styleSheet()->m_childRules, handler))
     457        if (auto* cachedResource = importRule->cachedCSSStyleSheet()) {
     458            if (handler(*cachedResource))
     459                return true;
     460        }
     461        auto* importedStyleSheet = importRule->styleSheet();
     462        if (importedStyleSheet && importedStyleSheet->traverseSubresources(handler))
    483463            return true;
    484464    }

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

    r205660 r206603  
    8787
    8888    URL completeURL(const String& url) const;
    89     void addSubresourceStyleURLs(ListHashSet<URL>&);
    9089    bool traverseSubresources(const std::function<bool (const CachedResource&)>& handler) const;
    9190

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

    r205660 r206603  
    2929#include "CSSStyleSheet.h"
    3030#include "CSSValuePool.h"
     31#include "CachedResource.h"
    3132#include "ContentSecurityPolicy.h"
    3233#include "DOMTokenList.h"
    … …  
    267268void StyledElement::addSubresourceAttributeURLs(ListHashSet<URL>& urls) const
    268269{
    269     if (const StyleProperties* inlineStyle = elementData() ? elementData()->inlineStyle() : nullptr)
    270         inlineStyle->addSubresourceStyleURLs(urls, &document().elementSheet().contents());
     270    auto* inlineStyle = this->inlineStyle();
     271    if (!inlineStyle)
     272        return;
     273    inlineStyle->traverseSubresources([&] (auto& resource) {
     274        urls.add(resource.url());
     275        return false;
     276    });
    271277}
    272278

  • trunk/Source/WebCore/html/HTMLLinkElement.cpp

    r206561 r206603  
    531531    // Append the URL of this link element.
    532532    addSubresourceURL(urls, href());
    533    
    534     // Walk the URLs linked by the linked-to stylesheet.
    535     if (CSSStyleSheet* styleSheet = const_cast<HTMLLinkElement*>(this)->sheet())
    536         styleSheet->contents().addSubresourceStyleURLs(urls);
     533
     534    if (auto* styleSheet = this->sheet()) {
     535        styleSheet->contents().traverseSubresources([&] (auto& resource) {
     536            urls.add(resource.url());
     537            return false;
     538        });
     539    }
    537540}
    538541

  • trunk/Source/WebCore/html/HTMLStyleElement.cpp

    r206361 r206603  
    2626
    2727#include "AuthorStyleSheets.h"
     28#include "CachedResource.h"
    2829#include "Document.h"
    2930#include "Event.h"
    … …  
    143144    HTMLElement::addSubresourceAttributeURLs(urls);
    144145
    145     if (CSSStyleSheet* styleSheet = const_cast<HTMLStyleElement*>(this)->sheet())
    146         styleSheet->contents().addSubresourceStyleURLs(urls);
     146    if (auto* styleSheet = this->sheet()) {
     147        styleSheet->contents().traverseSubresources([&] (auto& resource) {
     148            urls.add(resource.url());
     149            return false;
     150        });
     151    }
    147152}
    148153
Note: See TracChangeset for help on using the changeset viewer.