Bug 100671

Summary: CoordinatedGraphics: Unrelease adopted images
Product: WebKit Reporter: Jocelyn Turcotte <jturcotte>
Component: New BugsAssignee: Jocelyn Turcotte <jturcotte>
Status: RESOLVED FIXED    
Severity: Normal CC: jturcotte, kenneth, noam, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Jocelyn Turcotte
Reported 2012-10-29 07:01:08 PDT
CoordinatedGraphics: Unrelease adopted images
Attachments
Patch (2.28 KB, patch)
2012-10-29 07:05 PDT, Jocelyn Turcotte
no flags
Patch (2.96 KB, patch)
2012-10-29 09:42 PDT, Jocelyn Turcotte
no flags
Jocelyn Turcotte
Comment 1 2012-10-29 07:05:17 PDT
Kenneth Rohde Christiansen
Comment 2 2012-10-29 07:14:36 PDT
Comment on attachment 171242 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=171242&action=review > Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/LayerTreeCoordinator.cpp:516 > + // Check if we were going to release this image on the next flush. during next flush. check whether > Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/LayerTreeCoordinator.cpp:517 > + size_t releasedPos = m_releasedDirectlyCompositedImages.find(key); Pos meaning what? position?
Jocelyn Turcotte
Comment 3 2012-10-29 07:37:36 PDT
Comment on attachment 171242 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=171242&action=review >> Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/LayerTreeCoordinator.cpp:516 >> + // Check if we were going to release this image on the next flush. > > during next flush. > check whether I don't understand what's wrong with "Check if" and "on the next". >> Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/LayerTreeCoordinator.cpp:517 >> + size_t releasedPos = m_releasedDirectlyCompositedImages.find(key); > > Pos meaning what? position? Yep that one is misleading, "index" would be clearer. I'll change it.
Kenneth Rohde Christiansen
Comment 4 2012-10-29 07:40:30 PDT
Isnt it called "during a flush", you dont say on a flush, at least it sounds weird to me.
Jocelyn Turcotte
Comment 5 2012-10-29 07:59:18 PDT
(In reply to comment #4) > Isnt it called "during a flush", you dont say on a flush, at least it sounds weird to me. Ok, according to the Internet "on" should only be used for specific days.
Noam Rosenthal
Comment 6 2012-10-29 08:02:05 PDT
(In reply to comment #5) > (In reply to comment #4) > > Isnt it called "during a flush", you dont say on a flush, at least it sounds weird to me. > > Ok, according to the Internet "on" should only be used for specific days. Usually "on" is for instantaneous reactions or events, and "during" is for operations that take time; e.g. "on the next timer event" or "during the next render phase". "if" makes a sentence into a conditional statement. In this case the statement is not conditional. </grammar nit>
Jocelyn Turcotte
Comment 7 2012-10-29 09:42:29 PDT
Created attachment 171264 [details] Patch Reuploading since I just figured out that the ShareableBitmap should also happen only if it wasn't in the release vector. I checked and 'if' should be fine here, just less formal. 'If' have the same meaning as 'whether' as long as it's not followed by a second alternative or an infinitive.
Kenneth Rohde Christiansen
Comment 8 2012-10-30 02:09:06 PDT
Comment on attachment 171264 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=171264&action=review > Source/WebKit2/ChangeLog:3 > + Coordinated Graphics: Unrelease adopted images unrelease? you really mean 'do not release'? Wouldn't it be cleared to use that wording, anyway nit.
WebKit Review Bot
Comment 9 2012-10-30 02:15:47 PDT
Comment on attachment 171264 [details] Patch Clearing flags on attachment: 171264 Committed r132883: <http://trac.webkit.org/changeset/132883>
WebKit Review Bot
Comment 10 2012-10-30 02:15:51 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.