Title: [207063] releases/WebKitGTK/webkit-2.14/Source/WebCore
Revision
207063
Author
carlo...@webkit.org
Date
2016-10-11 01:08:39 -0700 (Tue, 11 Oct 2016)

Log 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:

Modified Paths

Diff

Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/ChangeLog (207062 => 207063)


--- releases/WebKitGTK/webkit-2.14/Source/WebCore/ChangeLog	2016-10-11 08:05:04 UTC (rev 207062)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/ChangeLog	2016-10-11 08:08:39 UTC (rev 207063)
@@ -1,3 +1,65 @@
+2016-09-09  Antti Koivisto  <an...@apple.com>
+
+        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:
+
 2016-09-08  Yusuke Suzuki  <utatane....@gmail.com>
 
         [WTF] HashTable's rehash is not compatible to Ref<T> and ASan

Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/css/StyleResolver.cpp (207062 => 207063)


--- releases/WebKitGTK/webkit-2.14/Source/WebCore/css/StyleResolver.cpp	2016-10-11 08:05:04 UTC (rev 207062)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/css/StyleResolver.cpp	2016-10-11 08:08:39 UTC (rev 207063)
@@ -309,7 +309,8 @@
 
 StyleResolver::~StyleResolver()
 {
-    RELEASE_ASSERT(!m_inLoadPendingImages);
+    RELEASE_ASSERT(!m_isDeleted);
+    m_isDeleted = true;
 
 #if ENABLE(CSS_DEVICE_ADAPTATION)
     m_viewportStyleResolver->clearDocument();
@@ -385,7 +386,7 @@
 
 ElementStyle StyleResolver::styleForElement(const Element& element, const RenderStyle* parentStyle, RuleMatchingBehavior matchingBehavior, const RenderRegion* regionForStyling, const SelectorFilter* selectorFilter)
 {
-    RELEASE_ASSERT(!m_inLoadPendingImages);
+    RELEASE_ASSERT(!m_isDeleted);
 
     m_state = State(element, parentStyle, m_overrideDocumentElementStyle, regionForStyling, selectorFilter);
     State& state = m_state;
@@ -446,7 +447,7 @@
 
 std::unique_ptr<RenderStyle> StyleResolver::styleForKeyframe(const RenderStyle* elementStyle, const StyleKeyframe* keyframe, KeyframeValue& keyframeValue)
 {
-    RELEASE_ASSERT(!m_inLoadPendingImages);
+    RELEASE_ASSERT(!m_isDeleted);
 
     MatchResult result;
     result.addMatchedProperties(keyframe->properties());
@@ -486,9 +487,6 @@
 
     adjustRenderStyle(*state.style(), *state.parentStyle(), nullptr);
 
-    // Start loading resources referenced by this style.
-    loadPendingResources();
-    
     // Add all the animating properties to the keyframe.
     unsigned propertyCount = keyframe->properties().propertyCount();
     for (unsigned i = 0; i < propertyCount; ++i) {
@@ -642,9 +640,6 @@
     if (state.style()->hasViewportUnits())
         document().setHasStyleWithViewportUnits();
 
-    // Start loading resources referenced by this style.
-    loadPendingResources();
-
     // Now return the style.
     return state.takeStyle();
 }
@@ -651,7 +646,7 @@
 
 std::unique_ptr<RenderStyle> StyleResolver::styleForPage(int pageIndex)
 {
-    RELEASE_ASSERT(!m_inLoadPendingImages);
+    RELEASE_ASSERT(!m_isDeleted);
 
     auto* documentElement = m_document.documentElement();
     if (!documentElement)
@@ -686,9 +681,6 @@
 
     cascade.applyDeferredProperties(*this, &result);
 
-    // Start loading resources referenced by this style.
-    loadPendingResources();
-
     // Now return the style.
     return m_state.takeStyle();
 }
@@ -1413,9 +1405,6 @@
     // so to preserve behavior, we queue them up during cascade and flush here.
     cascade.applyDeferredProperties(*this, &matchResult);
 
-    // Start loading resources referenced by this style.
-    loadPendingResources();
-    
     ASSERT(!state.fontDirty());
     
     if (cacheItem || !cacheHash)
@@ -2043,18 +2032,6 @@
     return true;
 }
 
-void StyleResolver::loadPendingResources()
-{
-    ASSERT(style());
-    if (!style())
-        return;
-
-    RELEASE_ASSERT(!m_inLoadPendingImages);
-    TemporaryChange<bool> changeInLoadPendingImages(m_inLoadPendingImages, true);
-
-    Style::loadPendingResources(*style(), document(), m_state.element());
-}
-
 inline StyleResolver::MatchedProperties::MatchedProperties()
     : possiblyPaddedMember(nullptr)
 {

Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/css/StyleResolver.h (207062 => 207063)


--- releases/WebKitGTK/webkit-2.14/Source/WebCore/css/StyleResolver.h	2016-10-11 08:05:04 UTC (rev 207062)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/css/StyleResolver.h	2016-10-11 08:08:39 UTC (rev 207063)
@@ -220,10 +220,7 @@
     void clearCachedPropertiesAffectedByViewportUnits();
 
     bool createFilterOperations(const CSSValue& inValue, FilterOperations& outOperations);
-    void loadPendingSVGDocuments();
 
-    void loadPendingResources();
-
     struct RuleRange {
         RuleRange(int& firstRuleIndex, int& lastRuleIndex): firstRuleIndex(firstRuleIndex), lastRuleIndex(lastRuleIndex) { }
         int& firstRuleIndex;
@@ -482,8 +479,6 @@
 
     void applySVGProperty(CSSPropertyID, CSSValue*);
 
-    void loadPendingImages();
-
     static unsigned computeMatchedPropertiesHash(const MatchedProperties*, unsigned size);
     struct MatchedPropertiesCacheItem {
         Vector<MatchedProperties> matchedProperties;
@@ -525,8 +520,8 @@
 
     State m_state;
 
-    // Try to catch a crash. https://bugs.webkit.org/show_bug.cgi?id=141561.
-    bool m_inLoadPendingImages { false };
+    // See if we still have crashes where StyleResolver gets deleted early.
+    bool m_isDeleted { false };
 
     friend bool operator==(const MatchedProperties&, const MatchedProperties&);
     friend bool operator!=(const MatchedProperties&, const MatchedProperties&);

Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/page/animation/ImplicitAnimation.cpp (207062 => 207063)


--- releases/WebKitGTK/webkit-2.14/Source/WebCore/page/animation/ImplicitAnimation.cpp	2016-10-11 08:05:04 UTC (rev 207062)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/page/animation/ImplicitAnimation.cpp	2016-10-11 08:08:39 UTC (rev 207063)
@@ -36,6 +36,7 @@
 #include "ImplicitAnimation.h"
 #include "KeyframeAnimation.h"
 #include "RenderBox.h"
+#include "StylePendingResources.h"
 
 namespace WebCore {
 
@@ -212,6 +213,9 @@
 
     m_toStyle = RenderStyle::clonePtr(*to);
 
+    if (m_object && m_object->element())
+        Style::loadPendingResources(*m_toStyle, m_object->element()->document(), m_object->element());
+
     // Restart the transition
     if (m_fromStyle && m_toStyle)
         updateStateMachine(AnimationStateInput::RestartAnimation, -1);

Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/page/animation/KeyframeAnimation.cpp (207062 => 207063)


--- releases/WebKitGTK/webkit-2.14/Source/WebCore/page/animation/KeyframeAnimation.cpp	2016-10-11 08:05:04 UTC (rev 207062)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/page/animation/KeyframeAnimation.cpp	2016-10-11 08:08:39 UTC (rev 207063)
@@ -37,6 +37,7 @@
 #include "GeometryUtilities.h"
 #include "RenderBox.h"
 #include "RenderStyle.h"
+#include "StylePendingResources.h"
 #include "StyleResolver.h"
 
 namespace WebCore {
@@ -46,9 +47,7 @@
     , m_keyframes(animation.name())
     , m_unanimatedStyle(RenderStyle::clonePtr(*unanimatedStyle))
 {
-    // Get the keyframe RenderStyles
-    if (m_object && m_object->element())
-        m_object->element()->styleResolver().keyframeStylesForAnimation(*m_object->element(), unanimatedStyle, m_keyframes);
+    resolveKeyframeStyles();
 
     // Update the m_transformFunctionListValid flag based on whether the function lists in the keyframes match.
     validateTransformFunctionList();
@@ -350,6 +349,21 @@
     return m_keyframes.containsProperty(property);
 }
 
+void KeyframeAnimation::resolveKeyframeStyles()
+{
+    if (!m_object || !m_object->element())
+        return;
+    auto& element = *m_object->element();
+
+    element.styleResolver().keyframeStylesForAnimation(*m_object->element(), m_unanimatedStyle.get(), m_keyframes);
+
+    // Ensure resource loads for all the frames.
+    for (auto& keyframe : m_keyframes.keyframes()) {
+        if (auto* style = const_cast<RenderStyle*>(keyframe.style()))
+            Style::loadPendingResources(*style, element.document(), &element);
+    }
+}
+
 void KeyframeAnimation::validateTransformFunctionList()
 {
     m_transformFunctionListsMatch = false;

Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/page/animation/KeyframeAnimation.h (207062 => 207063)


--- releases/WebKitGTK/webkit-2.14/Source/WebCore/page/animation/KeyframeAnimation.h	2016-10-11 08:05:04 UTC (rev 207062)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/page/animation/KeyframeAnimation.h	2016-10-11 08:08:39 UTC (rev 207063)
@@ -81,6 +81,7 @@
 
     bool computeExtentOfAnimationForMatchingTransformLists(const FloatRect& rendererBox, LayoutRect&) const;
 
+    void resolveKeyframeStyles();
     void validateTransformFunctionList();
     void checkForMatchingFilterFunctionLists();
 #if ENABLE(FILTERS_LEVEL_2)

Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/rendering/RenderElement.cpp (207062 => 207063)


--- releases/WebKitGTK/webkit-2.14/Source/WebCore/rendering/RenderElement.cpp	2016-10-11 08:05:04 UTC (rev 207062)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/rendering/RenderElement.cpp	2016-10-11 08:08:39 UTC (rev 207063)
@@ -66,6 +66,7 @@
 #include "SVGRenderSupport.h"
 #include "Settings.h"
 #include "ShadowRoot.h"
+#include "StylePendingResources.h"
 #include "StyleResolver.h"
 #include <wtf/MathExtras.h>
 #include <wtf/StackStats.h>
@@ -150,6 +151,7 @@
     // Otherwise acts as if we didn't support this feature.
     const ContentData* contentData = style.contentData();
     if (contentData && !contentData->next() && is<ImageContentData>(*contentData) && !element.isPseudoElement()) {
+        Style::loadPendingResources(style, element.document(), &element);
         auto& styleImage = downcast<ImageContentData>(*contentData).image();
         auto image = createRenderer<RenderImage>(element, WTFMove(style), const_cast<StyleImage*>(&styleImage));
         image->setIsGeneratedContent();
@@ -363,6 +365,8 @@
 
 void RenderElement::initializeStyle()
 {
+    Style::loadPendingResources(m_style, document(), element());
+
     styleWillChange(StyleDifferenceNewStyle, style());
 
     m_hasInitializedStyle = true;
@@ -402,6 +406,8 @@
 
     diff = adjustStyleDifference(diff, contextSensitiveProperties);
 
+    Style::loadPendingResources(style, document(), element());
+
     styleWillChange(diff, style);
 
     auto oldStyle = WTFMove(m_style);
@@ -1571,13 +1577,17 @@
 
     auto& styleResolver = element()->styleResolver();
 
+    std::unique_ptr<RenderStyle> style;
     if (pseudoStyleRequest.pseudoId == FIRST_LINE_INHERITED) {
-        auto result = styleResolver.styleForElement(*element(), parentStyle).renderStyle;
-        result->setStyleType(FIRST_LINE_INHERITED);
-        return result;
-    }
+        style = styleResolver.styleForElement(*element(), parentStyle).renderStyle;
+        style->setStyleType(FIRST_LINE_INHERITED);
+    } else
+        style = styleResolver.pseudoStyleForElement(*element(), pseudoStyleRequest, *parentStyle);
 
-    return styleResolver.pseudoStyleForElement(*element(), pseudoStyleRequest, *parentStyle);
+    if (style)
+        Style::loadPendingResources(*style, document(), element());
+
+    return style;
 }
 
 Color RenderElement::selectionColor(int colorProperty) const

Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/rendering/RenderImage.cpp (207062 => 207063)


--- releases/WebKitGTK/webkit-2.14/Source/WebCore/rendering/RenderImage.cpp	2016-10-11 08:05:04 UTC (rev 207062)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/rendering/RenderImage.cpp	2016-10-11 08:08:39 UTC (rev 207063)
@@ -131,7 +131,6 @@
     , m_imageDevicePixelRatio(imageDevicePixelRatio)
 {
     updateAltText();
-    imageResource().initialize(this);
     if (is<HTMLImageElement>(element))
         m_hasShadowControls = downcast<HTMLImageElement>(element).hasShadowControls();
 }
@@ -140,7 +139,6 @@
     : RenderReplaced(document, WTFMove(style), IntSize())
     , m_imageResource(styleImage ? std::make_unique<RenderImageResourceStyleImage>(*styleImage) : std::make_unique<RenderImageResource>())
 {
-    imageResource().initialize(this);
 }
 
 RenderImage::~RenderImage()
@@ -201,6 +199,13 @@
     return ImageSizeChangeForAltText;
 }
 
+void RenderImage::styleWillChange(StyleDifference diff, const RenderStyle& newStyle)
+{
+    if (!hasInitializedStyle())
+        imageResource().initialize(this);
+    RenderReplaced::styleWillChange(diff, newStyle);
+}
+
 void RenderImage::styleDidChange(StyleDifference diff, const RenderStyle* oldStyle)
 {
     RenderReplaced::styleDidChange(diff, oldStyle);

Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/rendering/RenderImage.h (207062 => 207063)


--- releases/WebKitGTK/webkit-2.14/Source/WebCore/rendering/RenderImage.h	2016-10-11 08:05:04 UTC (rev 207062)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/rendering/RenderImage.h	2016-10-11 08:08:39 UTC (rev 207063)
@@ -77,6 +77,7 @@
     void computeIntrinsicRatioInformation(FloatSize& intrinsicSize, double& intrinsicRatio) const final;
     bool foregroundIsKnownToBeOpaqueInRect(const LayoutRect& localRect, unsigned maxDepthToTest) const override;
 
+    void styleWillChange(StyleDifference, const RenderStyle& newStyle) override;
     void styleDidChange(StyleDifference, const RenderStyle*) override;
 
     void imageChanged(WrappedImagePtr, const IntRect* = nullptr) override;

Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/rendering/style/StyleCachedImage.cpp (207062 => 207063)


--- releases/WebKitGTK/webkit-2.14/Source/WebCore/rendering/style/StyleCachedImage.cpp	2016-10-11 08:05:04 UTC (rev 207062)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/rendering/style/StyleCachedImage.cpp	2016-10-11 08:08:39 UTC (rev 207063)
@@ -40,8 +40,11 @@
     m_isCachedImage = true;
 
     // CSSImageValue doesn't get invalidated so we can grab the CachedImage immediately if it exists.
-    if (is<CSSImageValue>(m_cssValue))
+    if (is<CSSImageValue>(m_cssValue)) {
         m_cachedImage = downcast<CSSImageValue>(m_cssValue.get()).cachedImage();
+        if (m_cachedImage)
+            m_isPending = false;
+    }
 }
 
 StyleCachedImage::~StyleCachedImage()
@@ -66,7 +69,8 @@
 
 void StyleCachedImage::load(CachedResourceLoader& loader, const ResourceLoaderOptions& options)
 {
-    ASSERT(isPending());
+    ASSERT(m_isPending);
+    m_isPending = false;
 
     if (is<CSSImageValue>(m_cssValue)) {
         auto& imageValue = downcast<CSSImageValue>(m_cssValue.get());
@@ -106,7 +110,7 @@
 
 bool StyleCachedImage::isPending() const
 {
-    return !m_cachedImage;
+    return m_isPending;
 }
 
 bool StyleCachedImage::isLoaded() const
@@ -169,6 +173,7 @@
 
 void StyleCachedImage::addClient(RenderElement* renderer)
 {
+    ASSERT(!m_isPending);
     if (!m_cachedImage)
         return;
     m_cachedImage->addClient(renderer);
@@ -176,6 +181,7 @@
 
 void StyleCachedImage::removeClient(RenderElement* renderer)
 {
+    ASSERT(!m_isPending);
     if (!m_cachedImage)
         return;
     m_cachedImage->removeClient(renderer);
@@ -183,6 +189,7 @@
 
 RefPtr<Image> StyleCachedImage::image(RenderElement* renderer, const FloatSize&) const
 {
+    ASSERT(!m_isPending);
     if (!m_cachedImage)
         return nullptr;
     return m_cachedImage->imageForRenderer(renderer);

Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/rendering/style/StyleCachedImage.h (207062 => 207063)


--- releases/WebKitGTK/webkit-2.14/Source/WebCore/rendering/style/StyleCachedImage.h	2016-10-11 08:05:04 UTC (rev 207062)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/rendering/style/StyleCachedImage.h	2016-10-11 08:08:39 UTC (rev 207063)
@@ -69,6 +69,7 @@
     StyleCachedImage(CSSValue&);
 
     Ref<CSSValue> m_cssValue;
+    bool m_isPending { true };
     mutable float m_scaleFactor { 1 };
     mutable CachedResourceHandle<CachedImage> m_cachedImage;
 };
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to