Bug 100671 – CoordinatedGraphics: Unrelease adopted images

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jocelyn Turcotte]

CoordinatedGraphics: Unrelease adopted images

[Jocelyn Turcotte]

Created attachment 171242 [details]
Patch

[Kenneth Rohde Christiansen]

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 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]

Isnt it called "during a flush", you dont say on a flush, at least it sounds weird to me.

[Jocelyn Turcotte]

(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]

(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]

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 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 on attachment 171264 [details]
Patch

Clearing flags on attachment: 171264

Committed r132883: <http://trac.webkit.org/changeset/132883>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.