[webkit-reviews] review denied: [Bug 100819] Coordinated Graphics: Remove the dependency of ShareableSurface from Coordinated Graphics. : [Attachment 178341] Patch

[bugzilla-daemon at webkit.org]

Noam Rosenthal <noam at webkit.org> has denied Huang Dongsung
<luxtella at company100.net>'s request for review:
Bug 100819: Coordinated Graphics: Remove the dependency of ShareableSurface
from Coordinated Graphics.
https://bugs.webkit.org/show_bug.cgi?id=100819

Attachment 178341: Patch
https://bugs.webkit.org/attachment.cgi?id=178341&action=review

------- Additional Comments from Noam Rosenthal <noam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=178341&action=review


I'm ok with the change, but the toFlags(...) thing is not an improvement IMO,
and the signature of the size() function wasn't changed.

> Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedSurface.h:58
> +    virtual const WebCore::IntSize& size() const = 0;

Didn't we agree to make this 
virtual WebCore::IntSize size() const = 0?

>
Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/CoordinatedGraphicsLayer.
cpp:743
> -    return m_coordinator->beginContentUpdate(size, contentsOpaque() ? 0 :
ShareableBitmap::SupportsAlpha, atlas, offset);
> +    return m_coordinator->beginContentUpdate(size,
CoordinatedSurface::toFlags(!contentsOpaque()), atlas, offset);

This toFlags change doesn't improve readability.

>
Source/WebKit2/WebProcess/WebPage/CoordinatedGraphics/LayerTreeCoordinator.cpp:
778
> -	   if (atlas->flags() == flags) {
> +	   if (atlas->supportsAlpha() ==
CoordinatedSurface::supportsAlpha(flags)) {

This doesn't improve readability.