Diff
Modified: trunk/Source/WebCore/ChangeLog (205700 => 205701)
--- trunk/Source/WebCore/ChangeLog 2016-09-09 07:01:59 UTC (rev 205700)
+++ trunk/Source/WebCore/ChangeLog 2016-09-09 07:42:56 UTC (rev 205701)
@@ -1,3 +1,65 @@
+2016-09-09 Antti Koivisto <[email protected]>
+
+ 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 <[email protected]>
ScriptRunner should be driven by PendingScript rather than ScriptElement
Modified: trunk/Source/WebCore/css/StyleResolver.cpp (205700 => 205701)
--- trunk/Source/WebCore/css/StyleResolver.cpp 2016-09-09 07:01:59 UTC (rev 205700)
+++ trunk/Source/WebCore/css/StyleResolver.cpp 2016-09-09 07:42:56 UTC (rev 205701)
@@ -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: trunk/Source/WebCore/css/StyleResolver.h (205700 => 205701)
--- trunk/Source/WebCore/css/StyleResolver.h 2016-09-09 07:01:59 UTC (rev 205700)
+++ trunk/Source/WebCore/css/StyleResolver.h 2016-09-09 07:42:56 UTC (rev 205701)
@@ -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: trunk/Source/WebCore/page/animation/ImplicitAnimation.cpp (205700 => 205701)
--- trunk/Source/WebCore/page/animation/ImplicitAnimation.cpp 2016-09-09 07:01:59 UTC (rev 205700)
+++ trunk/Source/WebCore/page/animation/ImplicitAnimation.cpp 2016-09-09 07:42:56 UTC (rev 205701)
@@ -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: trunk/Source/WebCore/page/animation/KeyframeAnimation.cpp (205700 => 205701)
--- trunk/Source/WebCore/page/animation/KeyframeAnimation.cpp 2016-09-09 07:01:59 UTC (rev 205700)
+++ trunk/Source/WebCore/page/animation/KeyframeAnimation.cpp 2016-09-09 07:42:56 UTC (rev 205701)
@@ -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: trunk/Source/WebCore/page/animation/KeyframeAnimation.h (205700 => 205701)
--- trunk/Source/WebCore/page/animation/KeyframeAnimation.h 2016-09-09 07:01:59 UTC (rev 205700)
+++ trunk/Source/WebCore/page/animation/KeyframeAnimation.h 2016-09-09 07:42:56 UTC (rev 205701)
@@ -81,6 +81,7 @@
bool computeExtentOfAnimationForMatchingTransformLists(const FloatRect& rendererBox, LayoutRect&) const;
+ void resolveKeyframeStyles();
void validateTransformFunctionList();
void checkForMatchingFilterFunctionLists();
#if ENABLE(FILTERS_LEVEL_2)
Modified: trunk/Source/WebCore/rendering/RenderElement.cpp (205700 => 205701)
--- trunk/Source/WebCore/rendering/RenderElement.cpp 2016-09-09 07:01:59 UTC (rev 205700)
+++ trunk/Source/WebCore/rendering/RenderElement.cpp 2016-09-09 07:42:56 UTC (rev 205701)
@@ -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: trunk/Source/WebCore/rendering/RenderImage.cpp (205700 => 205701)
--- trunk/Source/WebCore/rendering/RenderImage.cpp 2016-09-09 07:01:59 UTC (rev 205700)
+++ trunk/Source/WebCore/rendering/RenderImage.cpp 2016-09-09 07:42:56 UTC (rev 205701)
@@ -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: trunk/Source/WebCore/rendering/RenderImage.h (205700 => 205701)
--- trunk/Source/WebCore/rendering/RenderImage.h 2016-09-09 07:01:59 UTC (rev 205700)
+++ trunk/Source/WebCore/rendering/RenderImage.h 2016-09-09 07:42:56 UTC (rev 205701)
@@ -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: trunk/Source/WebCore/rendering/style/StyleCachedImage.cpp (205700 => 205701)
--- trunk/Source/WebCore/rendering/style/StyleCachedImage.cpp 2016-09-09 07:01:59 UTC (rev 205700)
+++ trunk/Source/WebCore/rendering/style/StyleCachedImage.cpp 2016-09-09 07:42:56 UTC (rev 205701)
@@ -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: trunk/Source/WebCore/rendering/style/StyleCachedImage.h (205700 => 205701)
--- trunk/Source/WebCore/rendering/style/StyleCachedImage.h 2016-09-09 07:01:59 UTC (rev 205700)
+++ trunk/Source/WebCore/rendering/style/StyleCachedImage.h 2016-09-09 07:42:56 UTC (rev 205701)
@@ -69,6 +69,7 @@
StyleCachedImage(CSSValue&);
Ref<CSSValue> m_cssValue;
+ bool m_isPending { true };
mutable float m_scaleFactor { 1 };
mutable CachedResourceHandle<CachedImage> m_cachedImage;
};
