[webkit-reviews] review granted: [Bug 80054] [chromium] Expose compositor filters to Aura through WebLayer : [Attachment 137454] Patch

[bugzilla-daemon at webkit.org]

James Robinson <jamesr at chromium.org> has granted Dana Jansens
<danakj at chromium.org>'s request for review:
Bug 80054: [chromium] Expose compositor filters to Aura through WebLayer
https://bugs.webkit.org/show_bug.cgi?id=80054

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

------- Additional Comments from James Robinson <jamesr at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=137454&action=review


R=me with a few minor things to address before landing

> Source/Platform/chromium/public/WebFilterOperation.h:30
> +#include "WebColor.h"
> +#include "WebCommon.h"

WebCommon.h should always come first (sort of like config.h in .cpp files)

> Source/Platform/chromium/public/WebFilterOperation.h:96
> +    WebBlurFilterOperation(int pixelRadius) : pixelRadius(pixelRadius) { }

explicit on 1-arg c'tors, please

> Source/Platform/chromium/public/WebLayer.h:106
> +    // Clear the filters in use by passing in a newly instantiated
> +    // WebFilterOperations object.
> +    WEBKIT_EXPORT void setFilters(const WebFilterOperations&);
> +
> +    // Background filters are only possible on layers that are drawn
directly
> +    // to the root render surface. This means if an ancestor of the
background-
> +    // filtered layer sets certain properties (opacity, transforms), it may
> +    // conflict and hide the background filters.

Instead of just describing the exceptional cases, can you please describe what
these APIs do?	setFilters is fairly obvious, but for background filter it's
not really clear at all what the actual result of this is.  I think a short
description would go a long way