[webkit-reviews] review granted: [Bug 108899] Coordinated Graphics : Refactor CoordinatedSurface to manage the lifecycle of GraphicsContext : [Attachment 203911] Patch

[bugzilla-daemon at webkit.org]

Noam Rosenthal <noam at webkit.org> has granted Jae Hyun Park
<jae.park at company100.net>'s request for review:
Bug 108899: Coordinated Graphics : Refactor CoordinatedSurface to manage the
lifecycle of GraphicsContext
https://bugs.webkit.org/show_bug.cgi?id=108899

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

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


LGTM with nitpicks

> Source/WebCore/ChangeLog:12
> +	   CoordinatedImageBacking and UpdateAtlas does not asks for the
ownership

does not asks -> do not ask

> Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedTile.h:81
> +    virtual bool paintToSurface(const IntSize&, uint32_t& /* atlasID */,
IntPoint&, CoordinatedSurface::Client*) = 0;

No need to comment atlasID

> Source/WebCore/platform/graphics/texmap/coordinated/UpdateAtlas.cpp:57
> +

Remove new line

> Source/WebCore/platform/graphics/texmap/coordinated/UpdateAtlas.h:49
> +    bool paintOnAvailableBuffer(const IntSize&, uint32_t& /* atlasID */,
IntPoint& /* offset */, CoordinatedSurface::Client*);

No need to comment out parameter names

> Source/WebKit2/Shared/CoordinatedGraphics/WebCoordinatedSurface.cpp:190
> +    if (!client)
> +	   return;
> +

We should actually assert for the client.