Changeset 207063 in webkit


Ignore:
Timestamp:
Oct 11, 2016 1:08:39 AM (8 years ago)
Author:
Carlos Garcia Campos
Message:

Merge r205701 - v3: WebContent crash due to RELEASE_ASSERT in WebCore: WebCore::StyleResolver::styleForElement
​https://bugs.webkit.org/show_bug.cgi?id=161689

Reviewed by Andreas Kling.

These crashes happen because synchronously triggered resource loads generate callbacks that may end up
deleting the resource loader.

Stop triggering resource loads from StyleResolver. Instead trigger them when applying style to render tree.

  • css/StyleResolver.cpp:

(WebCore::StyleResolver::~StyleResolver):

Replace the RELEASE_ASSERT against deletion during resource loads by a general isDeleted assert.

(WebCore::StyleResolver::styleForElement):
(WebCore::StyleResolver::styleForKeyframe):
(WebCore::StyleResolver::pseudoStyleForElement):
(WebCore::StyleResolver::styleForPage):
(WebCore::StyleResolver::applyMatchedProperties):
(WebCore::StyleResolver::loadPendingResources): Deleted.

  • css/StyleResolver.h:
  • page/animation/KeyframeAnimation.cpp:

(WebCore::KeyframeAnimation::KeyframeAnimation):
(WebCore::KeyframeAnimation::resolveKeyframeStyles):

Ensure resource load for all animation frames.

  • page/animation/KeyframeAnimation.h:
  • rendering/RenderElement.cpp:

(WebCore::RenderElement::createFor):
(WebCore::RenderElement::initializeStyle):

Load resources when renderer initializes a style.

(WebCore::RenderElement::setStyle):
(WebCore::RenderElement::getUncachedPseudoStyle):

Load resources for pseudo styles.

  • rendering/RenderImage.cpp:

(WebCore::RenderImage::RenderImage):
(WebCore::RenderImage::styleWillChange):

Shuffle image resource initialization out from constructor so initializeStyle gets called before.

  • rendering/RenderImage.h:
  • rendering/style/StyleCachedImage.cpp:

(WebCore::StyleCachedImage::StyleCachedImage):

Track pending status with a bit instead of implicitly by the existence of CachedResource.
This is useful for asserts.

(WebCore::StyleCachedImage::load):
(WebCore::StyleCachedImage::isPending):
(WebCore::StyleCachedImage::addClient):
(WebCore::StyleCachedImage::removeClient):
(WebCore::StyleCachedImage::image):

  • rendering/style/StyleCachedImage.h:
Location:
releases/WebKitGTK/webkit-2.14/Source/WebCore
Files:
11 edited

Legend:

Unmodified
Added
Removed

  • releases/WebKitGTK/webkit-2.14/Source/WebCore/ChangeLog

    r207062 r207063  
     12016-09-09  Antti Koivisto  <antti@apple.com>
     2
     3        v3: WebContent crash due to RELEASE_ASSERT in WebCore: WebCore::StyleResolver::styleForElement
     4        https://bugs.webkit.org/show_bug.cgi?id=161689
     5
     6        Reviewed by Andreas Kling.
     7
     8        These crashes happen because synchronously triggered resource loads generate callbacks that may end up
     9        deleting the resource loader.
     10
     11        Stop triggering resource loads from StyleResolver. Instead trigger them when applying style to render tree.
     12
     13        * css/StyleResolver.cpp:
     14        (WebCore::StyleResolver::~StyleResolver):
     15
     16            Replace the RELEASE_ASSERT against deletion during resource loads by a general isDeleted assert.
     17
     18        (WebCore::StyleResolver::styleForElement):
     19        (WebCore::StyleResolver::styleForKeyframe):
     20        (WebCore::StyleResolver::pseudoStyleForElement):
     21        (WebCore::StyleResolver::styleForPage):
     22        (WebCore::StyleResolver::applyMatchedProperties):
     23        (WebCore::StyleResolver::loadPendingResources): Deleted.
     24        * css/StyleResolver.h:
     25        * page/animation/KeyframeAnimation.cpp:
     26        (WebCore::KeyframeAnimation::KeyframeAnimation):
     27        (WebCore::KeyframeAnimation::resolveKeyframeStyles):
     28
     29            Ensure resource load for all animation frames.
     30
     31        * page/animation/KeyframeAnimation.h:
     32        * rendering/RenderElement.cpp:
     33        (WebCore::RenderElement::createFor):
     34        (WebCore::RenderElement::initializeStyle):
     35
     36            Load resources when renderer initializes a style.
     37
     38        (WebCore::RenderElement::setStyle):
     39        (WebCore::RenderElement::getUncachedPseudoStyle):
     40
     41            Load resources for pseudo styles.
     42
     43        * rendering/RenderImage.cpp:
     44        (WebCore::RenderImage::RenderImage):
     45        (WebCore::RenderImage::styleWillChange):
     46
     47            Shuffle image resource initialization out from constructor so initializeStyle gets called before.
     48
     49        * rendering/RenderImage.h:
     50        * rendering/style/StyleCachedImage.cpp:
     51        (WebCore::StyleCachedImage::StyleCachedImage):
     52
     53            Track pending status with a bit instead of implicitly by the existence of CachedResource.
     54            This is useful for asserts.
     55
     56        (WebCore::StyleCachedImage::load):
     57        (WebCore::StyleCachedImage::isPending):
     58        (WebCore::StyleCachedImage::addClient):
     59        (WebCore::StyleCachedImage::removeClient):
     60        (WebCore::StyleCachedImage::image):
     61        * rendering/style/StyleCachedImage.h:
     62
    1632016-09-08  Yusuke Suzuki  <utatane.tea@gmail.com>
    264

  • releases/WebKitGTK/webkit-2.14/Source/WebCore/css/StyleResolver.cpp

    r205647 r207063  
    310310StyleResolver::~StyleResolver()
    311311{
    312     RELEASE_ASSERT(!m_inLoadPendingImages);
     312    RELEASE_ASSERT(!m_isDeleted);
     313    m_isDeleted = true;
    313314
    314315#if ENABLE(CSS_DEVICE_ADAPTATION)
    … …  
    386387ElementStyle StyleResolver::styleForElement(const Element& element, const RenderStyle* parentStyle, RuleMatchingBehavior matchingBehavior, const RenderRegion* regionForStyling, const SelectorFilter* selectorFilter)
    387388{
    388     RELEASE_ASSERT(!m_inLoadPendingImages);
     389    RELEASE_ASSERT(!m_isDeleted);
    389390
    390391    m_state = State(element, parentStyle, m_overrideDocumentElementStyle, regionForStyling, selectorFilter);
    … …  
    447448std::unique_ptr<RenderStyle> StyleResolver::styleForKeyframe(const RenderStyle* elementStyle, const StyleKeyframe* keyframe, KeyframeValue& keyframeValue)
    448449{
    449     RELEASE_ASSERT(!m_inLoadPendingImages);
     450    RELEASE_ASSERT(!m_isDeleted);
    450451
    451452    MatchResult result;
    … …  
    487488    adjustRenderStyle(*state.style(), *state.parentStyle(), nullptr);
    488489
    489     // Start loading resources referenced by this style.
    490     loadPendingResources();
    491    
    492490    // Add all the animating properties to the keyframe.
    493491    unsigned propertyCount = keyframe->properties().propertyCount();
    … …  
    643641        document().setHasStyleWithViewportUnits();
    644642
    645     // Start loading resources referenced by this style.
    646     loadPendingResources();
    647 
    648643    // Now return the style.
    649644    return state.takeStyle();
    … …  
    652647std::unique_ptr<RenderStyle> StyleResolver::styleForPage(int pageIndex)
    653648{
    654     RELEASE_ASSERT(!m_inLoadPendingImages);
     649    RELEASE_ASSERT(!m_isDeleted);
    655650
    656651    auto* documentElement = m_document.documentElement();
    … …  
    686681
    687682    cascade.applyDeferredProperties(*this, &result);
    688 
    689     // Start loading resources referenced by this style.
    690     loadPendingResources();
    691683
    692684    // Now return the style.
    … …  
    14141406    cascade.applyDeferredProperties(*this, &matchResult);
    14151407
    1416     // Start loading resources referenced by this style.
    1417     loadPendingResources();
    1418    
    14191408    ASSERT(!state.fontDirty());
    14201409   
    … …  
    20442033}
    20452034
    2046 void StyleResolver::loadPendingResources()
    2047 {
    2048     ASSERT(style());
    2049     if (!style())
    2050         return;
    2051 
    2052     RELEASE_ASSERT(!m_inLoadPendingImages);
    2053     TemporaryChange<bool> changeInLoadPendingImages(m_inLoadPendingImages, true);
    2054 
    2055     Style::loadPendingResources(*style(), document(), m_state.element());
    2056 }
    2057 
    20582035inline StyleResolver::MatchedProperties::MatchedProperties()
    20592036    : possiblyPaddedMember(nullptr)

  • releases/WebKitGTK/webkit-2.14/Source/WebCore/css/StyleResolver.h

    r205647 r207063  
    221221
    222222    bool createFilterOperations(const CSSValue& inValue, FilterOperations& outOperations);
    223     void loadPendingSVGDocuments();
    224 
    225     void loadPendingResources();
    226223
    227224    struct RuleRange {
    … …  
    483480    void applySVGProperty(CSSPropertyID, CSSValue*);
    484481
    485     void loadPendingImages();
    486 
    487482    static unsigned computeMatchedPropertiesHash(const MatchedProperties*, unsigned size);
    488483    struct MatchedPropertiesCacheItem {
    … …  
    526521    State m_state;
    527522
    528     // Try to catch a crash. https://bugs.webkit.org/show_bug.cgi?id=141561.
    529     bool m_inLoadPendingImages { false };
     523    // See if we still have crashes where StyleResolver gets deleted early.
     524    bool m_isDeleted { false };
    530525
    531526    friend bool operator==(const MatchedProperties&, const MatchedProperties&);

  • releases/WebKitGTK/webkit-2.14/Source/WebCore/page/animation/ImplicitAnimation.cpp

    r200098 r207063  
    3737#include "KeyframeAnimation.h"
    3838#include "RenderBox.h"
     39#include "StylePendingResources.h"
    3940
    4041namespace WebCore {
    … …  
    213214    m_toStyle = RenderStyle::clonePtr(*to);
    214215
     216    if (m_object && m_object->element())
     217        Style::loadPendingResources(*m_toStyle, m_object->element()->document(), m_object->element());
     218
    215219    // Restart the transition
    216220    if (m_fromStyle && m_toStyle)

  • releases/WebKitGTK/webkit-2.14/Source/WebCore/page/animation/KeyframeAnimation.cpp

    r200622 r207063  
    3838#include "RenderBox.h"
    3939#include "RenderStyle.h"
     40#include "StylePendingResources.h"
    4041#include "StyleResolver.h"
    4142
    … …  
    4748    , m_unanimatedStyle(RenderStyle::clonePtr(*unanimatedStyle))
    4849{
    49     // Get the keyframe RenderStyles
    50     if (m_object && m_object->element())
    51         m_object->element()->styleResolver().keyframeStylesForAnimation(*m_object->element(), unanimatedStyle, m_keyframes);
     50    resolveKeyframeStyles();
    5251
    5352    // Update the m_transformFunctionListValid flag based on whether the function lists in the keyframes match.
    … …  
    351350}
    352351
     352void KeyframeAnimation::resolveKeyframeStyles()
     353{
     354    if (!m_object || !m_object->element())
     355        return;
     356    auto& element = *m_object->element();
     357
     358    element.styleResolver().keyframeStylesForAnimation(*m_object->element(), m_unanimatedStyle.get(), m_keyframes);
     359
     360    // Ensure resource loads for all the frames.
     361    for (auto& keyframe : m_keyframes.keyframes()) {
     362        if (auto* style = const_cast<RenderStyle*>(keyframe.style()))
     363            Style::loadPendingResources(*style, element.document(), &element);
     364    }
     365}
     366
    353367void KeyframeAnimation::validateTransformFunctionList()
    354368{

  • releases/WebKitGTK/webkit-2.14/Source/WebCore/page/animation/KeyframeAnimation.h

    r200098 r207063  
    8282    bool computeExtentOfAnimationForMatchingTransformLists(const FloatRect& rendererBox, LayoutRect&) const;
    8383
     84    void resolveKeyframeStyles();
    8485    void validateTransformFunctionList();
    8586    void checkForMatchingFilterFunctionLists();

  • releases/WebKitGTK/webkit-2.14/Source/WebCore/rendering/RenderElement.cpp

    r204667 r207063  
    6767#include "Settings.h"
    6868#include "ShadowRoot.h"
     69#include "StylePendingResources.h"
    6970#include "StyleResolver.h"
    7071#include <wtf/MathExtras.h>
    … …  
    151152    const ContentData* contentData = style.contentData();
    152153    if (contentData && !contentData->next() && is<ImageContentData>(*contentData) && !element.isPseudoElement()) {
     154        Style::loadPendingResources(style, element.document(), &element);
    153155        auto& styleImage = downcast<ImageContentData>(*contentData).image();
    154156        auto image = createRenderer<RenderImage>(element, WTFMove(style), const_cast<StyleImage*>(&styleImage));
    … …  
    364366void RenderElement::initializeStyle()
    365367{
     368    Style::loadPendingResources(m_style, document(), element());
     369
    366370    styleWillChange(StyleDifferenceNewStyle, style());
    367371
    … …  
    402406
    403407    diff = adjustStyleDifference(diff, contextSensitiveProperties);
     408
     409    Style::loadPendingResources(style, document(), element());
    404410
    405411    styleWillChange(diff, style);
    … …  
    15721578    auto& styleResolver = element()->styleResolver();
    15731579
     1580    std::unique_ptr<RenderStyle> style;
    15741581    if (pseudoStyleRequest.pseudoId == FIRST_LINE_INHERITED) {
    1575         auto result = styleResolver.styleForElement(*element(), parentStyle).renderStyle;
    1576         result->setStyleType(FIRST_LINE_INHERITED);
    1577         return result;
    1578     }
    1579 
    1580     return styleResolver.pseudoStyleForElement(*element(), pseudoStyleRequest, *parentStyle);
     1582        style = styleResolver.styleForElement(*element(), parentStyle).renderStyle;
     1583        style->setStyleType(FIRST_LINE_INHERITED);
     1584    } else
     1585        style = styleResolver.pseudoStyleForElement(*element(), pseudoStyleRequest, *parentStyle);
     1586
     1587    if (style)
     1588        Style::loadPendingResources(*style, document(), element());
     1589
     1590    return style;
    15811591}
    15821592

  • releases/WebKitGTK/webkit-2.14/Source/WebCore/rendering/RenderImage.cpp

    r204983 r207063  
    132132{
    133133    updateAltText();
    134     imageResource().initialize(this);
    135134    if (is<HTMLImageElement>(element))
    136135        m_hasShadowControls = downcast<HTMLImageElement>(element).hasShadowControls();
    … …  
    141140    , m_imageResource(styleImage ? std::make_unique<RenderImageResourceStyleImage>(*styleImage) : std::make_unique<RenderImageResource>())
    142141{
    143     imageResource().initialize(this);
    144142}
    145143
    … …  
    200198    setIntrinsicSize(imageSize);
    201199    return ImageSizeChangeForAltText;
     200}
     201
     202void RenderImage::styleWillChange(StyleDifference diff, const RenderStyle& newStyle)
     203{
     204    if (!hasInitializedStyle())
     205        imageResource().initialize(this);
     206    RenderReplaced::styleWillChange(diff, newStyle);
    202207}
    203208

  • releases/WebKitGTK/webkit-2.14/Source/WebCore/rendering/RenderImage.h

    r200486 r207063  
    7878    bool foregroundIsKnownToBeOpaqueInRect(const LayoutRect& localRect, unsigned maxDepthToTest) const override;
    7979
     80    void styleWillChange(StyleDifference, const RenderStyle& newStyle) override;
    8081    void styleDidChange(StyleDifference, const RenderStyle*) override;
    8182

  • releases/WebKitGTK/webkit-2.14/Source/WebCore/rendering/style/StyleCachedImage.cpp

    r205647 r207063  
    4141
    4242    // CSSImageValue doesn't get invalidated so we can grab the CachedImage immediately if it exists.
    43     if (is<CSSImageValue>(m_cssValue))
     43    if (is<CSSImageValue>(m_cssValue)) {
    4444        m_cachedImage = downcast<CSSImageValue>(m_cssValue.get()).cachedImage();
     45        if (m_cachedImage)
     46            m_isPending = false;
     47    }
    4548}
    4649
    … …  
    6770void StyleCachedImage::load(CachedResourceLoader& loader, const ResourceLoaderOptions& options)
    6871{
    69     ASSERT(isPending());
     72    ASSERT(m_isPending);
     73    m_isPending = false;
    7074
    7175    if (is<CSSImageValue>(m_cssValue)) {
    … …  
    107111bool StyleCachedImage::isPending() const
    108112{
    109     return !m_cachedImage;
     113    return m_isPending;
    110114}
    111115
    … …  
    170174void StyleCachedImage::addClient(RenderElement* renderer)
    171175{
     176    ASSERT(!m_isPending);
    172177    if (!m_cachedImage)
    173178        return;
    … …  
    177182void StyleCachedImage::removeClient(RenderElement* renderer)
    178183{
     184    ASSERT(!m_isPending);
    179185    if (!m_cachedImage)
    180186        return;
    … …  
    184190RefPtr<Image> StyleCachedImage::image(RenderElement* renderer, const FloatSize&) const
    185191{
     192    ASSERT(!m_isPending);
    186193    if (!m_cachedImage)
    187194        return nullptr;

  • releases/WebKitGTK/webkit-2.14/Source/WebCore/rendering/style/StyleCachedImage.h

    r205646 r207063  
    7070
    7171    Ref<CSSValue> m_cssValue;
     72    bool m_isPending { true };
    7273    mutable float m_scaleFactor { 1 };
    7374    mutable CachedResourceHandle<CachedImage> m_cachedImage;
Note: See TracChangeset for help on using the changeset viewer.