[webkit-reviews] review granted: [Bug 87878] [CSS Shaders] Create a shared object between FECustomFilter objects to store the shared resources : [Attachment 146067] Patch V2

[bugzilla-daemon at webkit.org]

Dean Jackson <dino at apple.com> has granted Chiculita Alexandru
<achicu at adobe.com>'s request for review:
Bug 87878: [CSS Shaders] Create a shared object between FECustomFilter objects
to store the shared resources
https://bugs.webkit.org/show_bug.cgi?id=87878

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

------- Additional Comments from Dean Jackson <dino at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=146067&action=review


I'm interested to see what you think about changing the name, so I'm not
marking as cq+. As I read more of the code, I don't think this new class
controls anything, so Controller is the wrong name. But I didn't have any good
ideas.

> Source/WebCore/ChangeLog:8
> +	   An object called CustomFilterController is created the first time a
new FECustomFilter is needed in a document.

I wonder if CustomFilterController is the best name. It's not really
controlling anything - it's just holding the objects that are needed to process
the custom filter. It could almost be called CustomFilterGlobalContext or
something, because it holds things related to drawing and tied to the context
that it creates.

> Source/WebCore/platform/graphics/filters/CustomFilterController.h:34
> +#include <wtf/PassRefPtr.h>

Do you need this include?

> Source/WebCore/platform/graphics/filters/CustomFilterController.h:50
> +    void prepareContextIfNeeded(HostWindow*);
> +private:

Need a space between here.