https://bugs.webkit.org/show_bug.cgi?id=93203
Reviewed by Simon Fraser.
1) Go to apple.com
2) Reload repeatedly
Eventually you can get into state where some images don't load.
The problem is that a cached stylesheet may end up pointing to image resources that have been canceled (by the reload).
If this happens they stay in the canceled state even when the stylesheet is applied to a new document.
Fix by checking if all loads are complete (or pending) when restoring a cached stylesheet. The sheet is only used
if there are no failed or canceled loads. There are potential more sophisticated fixes but this is simple and safe.
Walking the sheet is fast and since it is only done on cache restore the cost is minimal.
No regression test yet though the new code does get exercised by the existing tests.
* css/CSSCrossfadeValue.cpp:
(WebCore::CSSCrossfadeValue::hasFailedOrCanceledSubresources):
(WebCore):
* css/CSSCrossfadeValue.h:
(CSSCrossfadeValue):
* css/CSSFontFaceSrcValue.cpp:
(WebCore::CSSFontFaceSrcValue::hasFailedOrCanceledSubresources):
(WebCore):
* css/CSSFontFaceSrcValue.h:
(CSSFontFaceSrcValue):
* css/CSSImageSetValue.cpp:
(WebCore::CSSImageSetValue::hasFailedOrCanceledSubresources):
(WebCore):
* css/CSSImageSetValue.h:
(CSSImageSetValue):
* css/CSSImageValue.cpp:
(WebCore::CSSImageValue::hasFailedOrCanceledSubresources):
(WebCore):
* css/CSSImageValue.h:
(CSSImageValue):
* css/CSSValue.cpp:
(WebCore::CSSValue::hasFailedOrCanceledSubresources):
(WebCore):
* css/CSSValue.h:
(CSSValue):
* css/CSSValueList.cpp:
(WebCore::CSSValueList::hasFailedOrCanceledSubresources):
(WebCore):
* css/CSSValueList.h:
(CSSValueList):
* css/StylePropertySet.cpp:
(WebCore::StylePropertySet::hasFailedOrCanceledSubresources):
(WebCore):
* css/StylePropertySet.h:
(StylePropertySet):
* css/StyleSheetContents.cpp:
(WebCore::childRulesHaveFailedOrCanceledSubresources):
(WebCore):
(WebCore::StyleSheetContents::hasFailedOrCanceledSubresources):
* css/StyleSheetContents.h:
(StyleSheetContents):
* loader/cache/CachedCSSStyleSheet.cpp:
(WebCore::CachedCSSStyleSheet::restoreParsedStyleSheet):
* loader/cache/CachedResource.h:
(WebCore::CachedResource::loadFailedOrCanceled):
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@124720
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
+2012-08-05 Antti Koivisto <antti@apple.com>
+
+ Don't reuse cached stylesheet with failed or canceled resource loads
+ https://bugs.webkit.org/show_bug.cgi?id=93203
+
+ Reviewed by Simon Fraser.
+
+ 1) Go to apple.com
+ 2) Reload repeatedly
+
+ Eventually you can get into state where some images don't load.
+
+ The problem is that a cached stylesheet may end up pointing to image resources that have been canceled (by the reload).
+ If this happens they stay in the canceled state even when the stylesheet is applied to a new document.
+
+ Fix by checking if all loads are complete (or pending) when restoring a cached stylesheet. The sheet is only used
+ if there are no failed or canceled loads. There are potential more sophisticated fixes but this is simple and safe.
+ Walking the sheet is fast and since it is only done on cache restore the cost is minimal.
+
+ No regression test yet though the new code does get exercised by the existing tests.
+
+ * css/CSSCrossfadeValue.cpp:
+ (WebCore::CSSCrossfadeValue::hasFailedOrCanceledSubresources):
+ (WebCore):
+ * css/CSSCrossfadeValue.h:
+ (CSSCrossfadeValue):
+ * css/CSSFontFaceSrcValue.cpp:
+ (WebCore::CSSFontFaceSrcValue::hasFailedOrCanceledSubresources):
+ (WebCore):
+ * css/CSSFontFaceSrcValue.h:
+ (CSSFontFaceSrcValue):
+ * css/CSSImageSetValue.cpp:
+ (WebCore::CSSImageSetValue::hasFailedOrCanceledSubresources):
+ (WebCore):
+ * css/CSSImageSetValue.h:
+ (CSSImageSetValue):
+ * css/CSSImageValue.cpp:
+ (WebCore::CSSImageValue::hasFailedOrCanceledSubresources):
+ (WebCore):
+ * css/CSSImageValue.h:
+ (CSSImageValue):
+ * css/CSSValue.cpp:
+ (WebCore::CSSValue::hasFailedOrCanceledSubresources):
+ (WebCore):
+ * css/CSSValue.h:
+ (CSSValue):
+ * css/CSSValueList.cpp:
+ (WebCore::CSSValueList::hasFailedOrCanceledSubresources):
+ (WebCore):
+ * css/CSSValueList.h:
+ (CSSValueList):
+ * css/StylePropertySet.cpp:
+ (WebCore::StylePropertySet::hasFailedOrCanceledSubresources):
+ (WebCore):
+ * css/StylePropertySet.h:
+ (StylePropertySet):
+ * css/StyleSheetContents.cpp:
+ (WebCore::childRulesHaveFailedOrCanceledSubresources):
+ (WebCore):
+ (WebCore::StyleSheetContents::hasFailedOrCanceledSubresources):
+ * css/StyleSheetContents.h:
+ (StyleSheetContents):
+ * loader/cache/CachedCSSStyleSheet.cpp:
+ (WebCore::CachedCSSStyleSheet::restoreParsedStyleSheet):
+ * loader/cache/CachedResource.h:
+ (WebCore::CachedResource::loadFailedOrCanceled):
+
2012-08-05 Kentaro Hara <haraken@chromium.org>
[V8] Move V8Proxy methods that set DOM attributes/callbacks to V8Binding
m_ownerValue->crossfadeChanged(*rect);
}
+bool CSSCrossfadeValue::hasFailedOrCanceledSubresources() const
+{
+ if (m_cachedFromImage && m_cachedFromImage->loadFailedOrCanceled())
+ return true;
+ if (m_cachedToImage && m_cachedToImage->loadFailedOrCanceled())
+ return true;
+ return false;
+}
+
} // namespace WebCore
void setPercentage(PassRefPtr<CSSPrimitiveValue> percentageValue) { m_percentageValue = percentageValue; }
+ bool hasFailedOrCanceledSubresources() const;
+
private:
CSSCrossfadeValue(PassRefPtr<CSSValue> fromValue, PassRefPtr<CSSValue> toValue)
: CSSImageGeneratorValue(CrossfadeClass)
addSubresourceURL(urls, styleSheet->completeURL(m_resource));
}
+bool CSSFontFaceSrcValue::hasFailedOrCanceledSubresources() const
+{
+ if (!m_cachedFont)
+ return false;
+ return m_cachedFont->loadFailedOrCanceled();
+}
+
CachedFont* CSSFontFaceSrcValue::cachedFont(Document* document)
{
if (!m_cachedFont) {
void addSubresourceStyleURLs(ListHashSet<KURL>&, const StyleSheetContents*) const;
+ bool hasFailedOrCanceledSubresources() const;
+
CachedFont* cachedFont(Document*);
private:
return "-webkit-image-set(" + CSSValueList::customCssText() + ")";
}
+bool CSSImageSetValue::hasFailedOrCanceledSubresources() const
+{
+ if (!m_imageSet || !m_imageSet->isCachedImageSet())
+ return false;
+ CachedResource* cachedResource = static_cast<StyleCachedImageSet*>(m_imageSet.get())->cachedImage();
+ if (!cachedResource)
+ return true;
+ return cachedResource->loadFailedOrCanceled();
+}
+
CSSImageSetValue::CSSImageSetValue(const CSSImageSetValue& cloneFrom)
: CSSValueList(cloneFrom)
, m_accessedBestFitImage(false)
float scaleFactor;
};
+ bool hasFailedOrCanceledSubresources() const;
+
PassRefPtr<CSSImageSetValue> cloneForCSSOM() const;
protected:
m_accessedImage = false;
}
+bool CSSImageValue::hasFailedOrCanceledSubresources() const
+{
+ if (!m_image || !m_image->isCachedImage())
+ return false;
+ CachedResource* cachedResource = static_cast<StyleCachedImage*>(m_image.get())->cachedImage();
+ if (!cachedResource)
+ return true;
+ return cachedResource->loadFailedOrCanceled();
+}
+
String CSSImageValue::customCssText() const
{
return "url(" + quoteCSSURLIfNeeded(m_url) + ")";
PassRefPtr<CSSValue> cloneForCSSOM() const;
+ bool hasFailedOrCanceledSubresources() const;
+
protected:
CSSImageValue(ClassType, const String& url);
static_cast<const CSSReflectValue*>(this)->addSubresourceStyleURLs(urls, styleSheet);
}
+bool CSSValue::hasFailedOrCanceledSubresources() const
+{
+ // This should get called for internal instances only.
+ ASSERT(!isCSSOMSafe());
+
+ if (isValueList())
+ return static_cast<const CSSValueList*>(this)->hasFailedOrCanceledSubresources();
+ if (classType() == FontFaceSrcClass)
+ return static_cast<const CSSFontFaceSrcValue*>(this)->hasFailedOrCanceledSubresources();
+ if (classType() == ImageClass)
+ return static_cast<const CSSImageValue*>(this)->hasFailedOrCanceledSubresources();
+ if (classType() == CrossfadeClass)
+ return static_cast<const CSSCrossfadeValue*>(this)->hasFailedOrCanceledSubresources();
+#if ENABLE(CSS_IMAGE_SET)
+ if (classType() == ImageSetClass)
+ return static_cast<const CSSImageSetValue*>(this)->hasFailedOrCanceledSubresources();
+#endif
+ return false;
+}
+
String CSSValue::cssText() const
{
if (m_isTextClone) {
void addSubresourceStyleURLs(ListHashSet<KURL>&, const StyleSheetContents*) const;
+ bool hasFailedOrCanceledSubresources() const;
+
protected:
static const size_t ClassTypeBits = 5;
m_values[i]->addSubresourceStyleURLs(urls, styleSheet);
}
+bool CSSValueList::hasFailedOrCanceledSubresources() const
+{
+ for (unsigned i = 0; i < m_values.size(); ++i) {
+ if (m_values[i]->hasFailedOrCanceledSubresources())
+ return true;
+ }
+ return false;
+}
+
CSSValueList::CSSValueList(const CSSValueList& cloneFrom)
: CSSValue(cloneFrom.classType(), /* isCSSOMSafe */ true)
{
#endif
void addSubresourceStyleURLs(ListHashSet<KURL>&, const StyleSheetContents*) const;
+
+ bool hasFailedOrCanceledSubresources() const;
PassRefPtr<CSSValueList> cloneForCSSOM() const;
propertyAt(i).value()->addSubresourceStyleURLs(urls, contextStyleSheet);
}
+bool StylePropertySet::hasFailedOrCanceledSubresources() const
+{
+ unsigned size = propertyCount();
+ for (unsigned i = 0; i < size; ++i) {
+ if (propertyAt(i).value()->hasFailedOrCanceledSubresources())
+ return true;
+ }
+ return false;
+}
+
// This is the list of properties we want to copy in the copyBlockProperties() function.
// It is the list of CSS properties that apply specially to block-level elements.
static const CSSPropertyID blockProperties[] = {
bool isMutable() const { return m_isMutable; }
+ bool hasFailedOrCanceledSubresources() const;
+
static unsigned averageSizeInBytes();
void reportMemoryUsage(MemoryObjectInfo*) const;
}
}
+static bool childRulesHaveFailedOrCanceledSubresources(const Vector<RefPtr<StyleRuleBase> >& rules)
+{
+ for (unsigned i = 0; i < rules.size(); ++i) {
+ const StyleRuleBase* rule = rules[i].get();
+ switch (rule->type()) {
+ case StyleRuleBase::Style:
+ if (static_cast<const StyleRule*>(rule)->properties()->hasFailedOrCanceledSubresources())
+ return true;
+ break;
+ case StyleRuleBase::FontFace:
+ if (static_cast<const StyleRuleFontFace*>(rule)->properties()->hasFailedOrCanceledSubresources())
+ return true;
+ break;
+ case StyleRuleBase::Media:
+ if (childRulesHaveFailedOrCanceledSubresources(static_cast<const StyleRuleMedia*>(rule)->childRules()))
+ return true;
+ break;
+ case StyleRuleBase::Region:
+ if (childRulesHaveFailedOrCanceledSubresources(static_cast<const StyleRuleRegion*>(rule)->childRules()))
+ return true;
+ break;
+ case StyleRuleBase::Import:
+ ASSERT_NOT_REACHED();
+ case StyleRuleBase::Page:
+ case StyleRuleBase::Keyframes:
+ case StyleRuleBase::Unknown:
+ case StyleRuleBase::Charset:
+ case StyleRuleBase::Keyframe:
+ break;
+ }
+ }
+ return false;
+}
+
+bool StyleSheetContents::hasFailedOrCanceledSubresources() const
+{
+ ASSERT(isCacheable());
+ return childRulesHaveFailedOrCanceledSubresources(m_childRules);
+}
+
StyleSheetContents* StyleSheetContents::parentStyleSheet() const
{
return m_ownerRule ? m_ownerRule->parentStyleSheet() : 0;
const String& charset() const { return m_parserContext.charset; }
bool loadCompleted() const { return m_loadCompleted; }
+ bool hasFailedOrCanceledSubresources() const;
KURL completeURL(const String& url) const;
void addSubresourceStyleURLs(ListHashSet<KURL>&);
{
if (!m_parsedStyleSheetCache)
return 0;
+ if (m_parsedStyleSheetCache->hasFailedOrCanceledSubresources()) {
+ m_parsedStyleSheetCache->removedFromMemoryCache();
+ m_parsedStyleSheetCache.clear();
+ return 0;
+ }
+
ASSERT(m_parsedStyleSheetCache->isCacheable());
ASSERT(m_parsedStyleSheetCache->isInMemoryCache());
bool wasCanceled() const { return m_status == Canceled; }
bool errorOccurred() const { return (m_status == LoadError || m_status == DecodeError); }
+ bool loadFailedOrCanceled() { return m_status == Canceled || m_status == LoadError; }
bool sendResourceLoadCallbacks() const { return m_options.sendLoadCallbacks == SendCallbacks; }