Bug 102453

Summary: When releasing a CGImage, we should remove it from the subimage cache too.
Product: WebKit Reporter: Yongjun Zhang <yongjun_zhang>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, buildbot, darin, ddkilzer, dglazkov, rniwa, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Macintosh   
OS: Unspecified   
Attachments:
Description Flags
Move SubimageCacheWithTimer to separate files.
buildbot: commit-queue-
Fix win build break.
webkit.review.bot: commit-queue-
Re-submit the patch for bots to pick up.
none
Move subimage cache code into separate h/cpp file.
ddkilzer: review-, buildbot: commit-queue-
remove the image from subimage cache when we releasing the CGImageRef none

Description Yongjun Zhang 2012-11-15 18:17:02 PST
Currently SubimageCacheWithTimer is inside GraphicsContextCG.cpp.  It would be nice to move it out to a separate file and we can access the subimage cache if needed.
Comment 1 Yongjun Zhang 2012-11-15 18:27:56 PST
Created attachment 174586 [details]
Move SubimageCacheWithTimer to separate files.
Comment 2 Build Bot 2012-11-15 21:30:12 PST
Comment on attachment 174586 [details]
Move SubimageCacheWithTimer to separate files.

Attachment 174586 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/14857504
Comment 3 Yongjun Zhang 2012-11-15 22:16:43 PST
Created attachment 174607 [details]
Fix win build break.
Comment 4 WebKit Review Bot 2012-11-16 00:42:28 PST
Comment on attachment 174607 [details]
Fix win build break.

Attachment 174607 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14844802

New failing tests:
inspector-protocol/debugger-terminate-dedicated-worker-while-paused.html
Comment 5 Yongjun Zhang 2012-11-16 12:21:50 PST
Created attachment 174739 [details]
Re-submit the patch for bots to pick up.
Comment 6 Yongjun Zhang 2012-11-26 09:59:29 PST
Part of <rdar://problem/12701759>
Comment 7 Yongjun Zhang 2013-02-06 15:29:59 PST
Created attachment 186934 [details]
Move subimage cache code into separate h/cpp file.
Comment 8 Build Bot 2013-02-06 16:35:43 PST
Comment on attachment 186934 [details]
Move subimage cache code into separate h/cpp file.

Attachment 186934 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/16398419

New failing tests:
http/tests/cache/cached-main-resource.html
Comment 9 Benjamin Poulain 2013-02-12 00:48:33 PST
Comment on attachment 186934 [details]
Move subimage cache code into separate h/cpp file.

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

I think it is misleading to have stuff like subimage(CGImageRef, const FloatRect&) in the new files. The new files should only contain the minimum for the definition and implementation of SubimageCacheWithTimer.

I would also promote SubimageCacheWithTimer to a real class with real encapsulation now that it is promoted from a tool to a header.

> Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.cpp:2
> + * Copyright (C) 2013 Apple Inc. All Rights Reserved.

You need to preserve the full original copyright too (or hunt who did the changes).

> Source/WebCore/platform/graphics/cg/SubimageCacheWithTimer.h:2
> + * Copyright (C) 2013 Apple Inc. All Rights Reserved.

Ditto for copyright.
Comment 10 David Kilzer (:ddkilzer) 2013-03-27 08:59:41 PDT
Comment on attachment 186934 [details]
Move subimage cache code into separate h/cpp file.

r- based on Benjamin's reply in Comment #9.
Comment 11 Yongjun Zhang 2013-03-29 14:20:11 PDT
We should also remove the image from subimage cache if we are going to release the CGImage.  Change the title to reflect that.
Comment 12 Yongjun Zhang 2013-03-29 14:20:49 PDT
<rdar://problem/13526138>
Comment 13 Yongjun Zhang 2013-03-29 14:45:02 PDT
Created attachment 195802 [details]
remove the image from subimage cache when we releasing the CGImageRef
Comment 14 WebKit Review Bot 2013-03-29 17:50:15 PDT
Comment on attachment 195802 [details]
remove the image from subimage cache when we releasing the CGImageRef

Clearing flags on attachment: 195802

Committed r147265: <http://trac.webkit.org/changeset/147265>
Comment 15 WebKit Review Bot 2013-03-29 17:50:19 PDT
All reviewed patches have been landed.  Closing bug.