Bug 91733 – Destroy CSS decoded data more eagerly once they become dead caches.

• Status: RESOLVED
• Resolution: FIXED
• Component: Page Loading

[Dongseong Hwang]

There are three CachedResources with decoded data: CachedImage, CachedScript
and CachedCSSStyleSheet. In the cases of CachedImage and CachedScript, we
eagerly destroy the decoded data using Timer in CacehdResource::allClientsRemoved().
We must apply the same policy here in CachedCSSStyleSheet because priority
inversion can occur. For example, we can't destroy the decoded data of CachedImages
when they are referenced by CachedCSSStyleSheet as background, mask or border
images.

[Dongseong Hwang]

Created attachment 153228 [details]
Patch

[Dongseong Hwang]

Bug 22394 made CacheScript destroy script decoded data eagerly.

Bug 22214 made the policy of CachedCSSStyleSheet::allClientsRemoved() and the policy continues to this day.

[Geoffrey Garen]

Comment on attachment 153228 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=153228&action=review

> Source/WebCore/loader/cache/CachedCSSStyleSheet.cpp:172
> +        makePurgeable(true);

Where is the makePurgeable(false) call to balance this? That's a good thing to comment on in your ChangeLog.

[Dongseong Hwang]

(In reply to comment #3)
> (From update of attachment 153228 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=153228&action=review
> 
> > Source/WebCore/loader/cache/CachedCSSStyleSheet.cpp:172
> > +        makePurgeable(true);
> 
> Where is the makePurgeable(false) call to balance this? That's a good thing to comment on in your ChangeLog.

Thank you for your review.

makePurgeable(false) is called by MemoryCache::resourceForURL() and DocumentLoader::subresource() like CachedImage and CachedScript.
CachedImage and CachedScript call only makePurgeable(true) instead of makePurgeable(false), too.
IMHO, "makePurgeable(false) call" may be not related to this patch because the previous CachedCSSStyleSheet did not call makePurgeable(false) and CachedCSSStyleSheet of this patch does also.

[Geoffrey Garen]

Comment on attachment 153228 [details]
Patch

In future, I'd like to see more code sharing for destroying decoded data on a timer.

[Kwang Yul Seo]

Comment on attachment 153228 [details]
Patch

Clearing flags on attachment: 153228

Committed r123385: <http://trac.webkit.org/changeset/123385>

[Kwang Yul Seo]

All reviewed patches have been landed.  Closing bug.

[Dongseong Hwang]

(In reply to comment #5)
> (From update of attachment 153228 [details])
> In future, I'd like to see more code sharing for destroying decoded data on a timer.

Thank you for your review. I think so. :)