[webkit-reviews] review denied: [Bug 100819] Coordinated Graphics: Remove the dependency of ShareableSurface from Coordinated Graphics. : [Attachment 177979] 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 177979: Patch
https://bugs.webkit.org/attachment.cgi?id=177979&action=review

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


This changes the behavior implicitly in a way that the bitmap handle is alive
for the lifetime of the surface. We should find a way to make this refactor
without changing this behavior.

> Source/WebKit2/ChangeLog:36
> +	   * Scripts/webkit2/messages.py:
> +	   (forward_declarations_and_headers):
> +	     This patch needs to use typedef as an argument of messages, so
this patch changes
> +	     forward_declarations_and_headers function to include headers for
types in special_cases set,
> +	     not to make forward declarations.
> +	   (headers_for_type):
> +	     CoordinatedSurfaceRef's header needs to be special cased to
> +	     CoordinatedSurface.h.

Please review this in a separate bug report, mentioning this one as a use case.


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

The const& thing is not needed... An IntSize is as expensive to copy as a
pointer. Keep it the same as WebCore::Image::size

> Source/WebKit2/Shared/CoordinatedGraphics/WebCoordinatedSurface.h:118
> +    enum Purpose {
> +	   Producer,
> +	   Consumer
> +    };
> +    // This member exists for only a debugging purpose. It is Producer in
Web Process, while Consumer in UI Process.
> +    Purpose m_purpose;

This should be in DEBUG ifdef.