Summary: | Don't reuse cached stylesheet with failed or canceled resource loads | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Antti Koivisto <koivisto> | ||||||||
Component: | CSS | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | cmarcelo, gustavo, japhet, jonlee, kling, macpherson, menard, thorton, webkit.review.bot, xan.lopez | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Antti Koivisto
2012-08-05 11:14:17 PDT
Created attachment 156566 [details]
patch
Comment on attachment 156566 [details] patch Attachment 156566 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/13436501 Comment on attachment 156566 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=156566&action=review > Source/WebCore/css/CSSImageValue.cpp:105 > +bool CSSImageValue::hasFailedOrCanceledSubresources() const Do generated images need their own override too? (I'm thinking specifically for crossfade) Created attachment 156567 [details]
better patch
(In reply to comment #4) > Do generated images need their own override too? (I'm thinking specifically for crossfade) I don't see code there that references a CachedResource. Or maybe I'm not looking the right place? But it is definitely good to check this for completeness. (In reply to comment #6) > (In reply to comment #4) > > Do generated images need their own override too? (I'm thinking specifically for crossfade) > > I don't see code there that references a CachedResource. Or maybe I'm not looking the right place? > > But it is definitely good to check this for completeness. CSSCrossfadeValue (a CSSImageGeneratorValue, not a CSSImageValue) has two CachedResourceHandles. Comment on attachment 156567 [details] better patch Attachment 156567 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/13439404 Comment on attachment 156567 [details] better patch Attachment 156567 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/13442259 Comment on attachment 156567 [details] better patch Attachment 156567 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/13443180 Comment on attachment 156567 [details] better patch Attachment 156567 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/13447051 Created attachment 156568 [details]
update patch
Comment on attachment 156568 [details] update patch View in context: https://bugs.webkit.org/attachment.cgi?id=156568&action=review > Source/WebCore/css/CSSValue.cpp:140 > + 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 This would be one line if we used virtual functions :( > Source/WebCore/css/StylePropertySet.cpp:901 > + if (propertyAt(i).value()->hasFailedOrCanceledSubresources()) Is this guaranteed to be non-null? > Source/WebCore/css/StyleSheetContents.cpp:433 > + 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; We need casting functions like we have for renderers. |