Move implementation of WebFilterOperations into cc

Move implementation of WebFilterOperations into cc

This moves the implementation of WebFilterOperations into cc, and
(behind an #ifdef) defines a WebFilterOperationsImpl class that
implements the WebFilterOperations interface by wrapping a 
cc::FilterOperations.

With this change, cc and ui/compositor no longer need to include
WebFilterOperations.h.

BUG=181613
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=208531

[ajuma, 2013-06-18 14:56:32]

The Blink side of this (which makes WebFilterOperations pure virtual) is
http://crrev.com/17286006.

This CL is the first step in the plan described at http://crbug.com/181613#c12
for threading filter animations.

https://codereview.chromium.org/16968002/diff/15001/cc/output/filter_operatio...
File cc/output/filter_operations.cc (right):

https://codereview.chromium.org/16968002/diff/15001/cc/output/filter_operatio...
cc/output/filter_operations.cc:109: matrix[19];
This early return seems wrong, since it means we can return false here even if
later filters in the sequence do affect opacity. Not sure if I should fix this
here or in a follow-up (to avoid introducing behavior changes in this CL).

[Stephen White, 2013-06-18 22:58:07]

This looks good, but I'm leaving for James.

Could you run layout tests trybots on this please (all 6 flavours)? Thanks.

[jamesr, 2013-06-18 23:08:52]

Very nice!  Needs a bit more chromification.

The conversion is slightly expensive/awkward, so we should try to avoid getting
stuck with it at the branch point.  I think we can complete it before we cut.

https://codereview.chromium.org/16968002/diff/15001/cc/output/filter_operatio...
File cc/output/filter_operation.cc (right):

https://codereview.chromium.org/16968002/diff/15001/cc/output/filter_operatio...
cc/output/filter_operation.cc:28: DCHECK(type_ != FilterTypeDropShadow && type_
!= FilterTypeColorMatrix);
would be better as two checks using DCHECK_NE

https://codereview.chromium.org/16968002/diff/15001/cc/output/filter_operation.h
File cc/output/filter_operation.h (right):

https://codereview.chromium.org/16968002/diff/15001/cc/output/filter_operatio...
cc/output/filter_operation.h:19: FilterTypeGrayscale,
This isn't the correct enum style for Chromium
(http://www.chromium.org/developers/coding-style#Naming).  In chromium, enum
types should be SHOUTING_STYLE and do not need to include the type name in the
enum.  These should probably be GRAYSCALE, SEPIA, SATURATE, etc

https://codereview.chromium.org/16968002/diff/15001/cc/output/filter_operatio...
cc/output/filter_operation.h:37: DCHECK(type_ != FilterTypeColorMatrix);
DCHECK_NE

https://codereview.chromium.org/16968002/diff/15001/cc/output/filter_operatio...
cc/output/filter_operation.h:42: DCHECK(type_ == FilterTypeDropShadow);
DCHECK_EQ, etc

https://codereview.chromium.org/16968002/diff/15001/cc/output/filter_operatio...
File cc/output/filter_operations.cc (right):

https://codereview.chromium.org/16968002/diff/15001/cc/output/filter_operatio...
cc/output/filter_operations.cc:13: }
this can go on the previous line with no space between the "{}"s

https://codereview.chromium.org/16968002/diff/15001/cc/output/filter_operatio...
File cc/output/filter_operations.h (right):

https://codereview.chromium.org/16968002/diff/15001/cc/output/filter_operatio...
cc/output/filter_operations.h:48: DCHECK(index < size());
DCHECK_LT

https://codereview.chromium.org/16968002/diff/15001/cc/trees/damage_tracker.h
File cc/trees/damage_tracker.h (right):

https://codereview.chromium.org/16968002/diff/15001/cc/trees/damage_tracker.h...
cc/trees/damage_tracker.h:43: const cc::FilterOperations& filters,
you're already in namespace cc, no need for "cc::"

https://codereview.chromium.org/16968002/diff/15001/cc/trees/layer_tree_host_...
File cc/trees/layer_tree_host_pixeltest_filters.cc (right):

https://codereview.chromium.org/16968002/diff/15001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_pixeltest_filters.cc:30:
filters.Append(FilterOperation::CreateBlurFilter(2));
this is a float constant, should be "2.f"

https://codereview.chromium.org/16968002/diff/15001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_pixeltest_filters.cc:55:
filters.Append(FilterOperation::CreateBlurFilter(5));
ditto

https://codereview.chromium.org/16968002/diff/15001/cc/trees/layer_tree_host_...
File cc/trees/layer_tree_host_unittest_occlusion.cc (right):

https://codereview.chromium.org/16968002/diff/15001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_unittest_occlusion.cc:353:
filters.Append(FilterOperation::CreateOpacityFilter(0.5));
0.5f

https://codereview.chromium.org/16968002/diff/15001/webkit/renderer/composito...
File webkit/renderer/compositor_bindings/web_layer_impl.cc (right):

https://codereview.chromium.org/16968002/diff/15001/webkit/renderer/composito...
webkit/renderer/compositor_bindings/web_layer_impl.cc:161:
filters).AsFilterOperations());
odd wrapping and casting. this probably deserves an intermediate variable to
hold the result of the downcast then the actual call

[ajuma, 2013-06-19 17:08:30]

PTAL.

https://codereview.chromium.org/16968002/diff/15001/cc/output/filter_operatio...
File cc/output/filter_operation.cc (right):

https://codereview.chromium.org/16968002/diff/15001/cc/output/filter_operatio...
cc/output/filter_operation.cc:28: DCHECK(type_ != FilterTypeDropShadow && type_
!= FilterTypeColorMatrix);
On 2013/06/18 23:08:52, jamesr wrote:
> would be better as two checks using DCHECK_NE

Done.

https://codereview.chromium.org/16968002/diff/15001/cc/output/filter_operation.h
File cc/output/filter_operation.h (right):

https://codereview.chromium.org/16968002/diff/15001/cc/output/filter_operatio...
cc/output/filter_operation.h:19: FilterTypeGrayscale,
On 2013/06/18 23:08:52, jamesr wrote:
> This isn't the correct enum style for Chromium
> (http://www.chromium.org/developers/coding-style#Naming).  In chromium, enum
> types should be SHOUTING_STYLE and do not need to include the type name in the
> enum.  These should probably be GRAYSCALE, SEPIA, SATURATE, etc

Done.

https://codereview.chromium.org/16968002/diff/15001/cc/output/filter_operatio...
cc/output/filter_operation.h:37: DCHECK(type_ != FilterTypeColorMatrix);
On 2013/06/18 23:08:52, jamesr wrote:
> DCHECK_NE

Done.

https://codereview.chromium.org/16968002/diff/15001/cc/output/filter_operatio...
cc/output/filter_operation.h:42: DCHECK(type_ == FilterTypeDropShadow);
On 2013/06/18 23:08:52, jamesr wrote:
> DCHECK_EQ, etc

Done.

https://codereview.chromium.org/16968002/diff/15001/cc/output/filter_operatio...
File cc/output/filter_operations.cc (right):

https://codereview.chromium.org/16968002/diff/15001/cc/output/filter_operatio...
cc/output/filter_operations.cc:13: }
On 2013/06/18 23:08:52, jamesr wrote:
> this can go on the previous line with no space between the "{}"s

Done.

https://codereview.chromium.org/16968002/diff/15001/cc/output/filter_operatio...
File cc/output/filter_operations.h (right):

https://codereview.chromium.org/16968002/diff/15001/cc/output/filter_operatio...
cc/output/filter_operations.h:48: DCHECK(index < size());
On 2013/06/18 23:08:52, jamesr wrote:
> DCHECK_LT

Done.

https://codereview.chromium.org/16968002/diff/15001/cc/trees/damage_tracker.h
File cc/trees/damage_tracker.h (right):

https://codereview.chromium.org/16968002/diff/15001/cc/trees/damage_tracker.h...
cc/trees/damage_tracker.h:43: const cc::FilterOperations& filters,
On 2013/06/18 23:08:52, jamesr wrote:
> you're already in namespace cc, no need for "cc::"

Done.

https://codereview.chromium.org/16968002/diff/15001/cc/trees/layer_tree_host_...
File cc/trees/layer_tree_host_pixeltest_filters.cc (right):

https://codereview.chromium.org/16968002/diff/15001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_pixeltest_filters.cc:30:
filters.Append(FilterOperation::CreateBlurFilter(2));
On 2013/06/18 23:08:52, jamesr wrote:
> this is a float constant, should be "2.f"

Done.

https://codereview.chromium.org/16968002/diff/15001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_pixeltest_filters.cc:55:
filters.Append(FilterOperation::CreateBlurFilter(5));
On 2013/06/18 23:08:52, jamesr wrote:
> ditto

Done.

https://codereview.chromium.org/16968002/diff/15001/cc/trees/layer_tree_host_...
File cc/trees/layer_tree_host_unittest_occlusion.cc (right):

https://codereview.chromium.org/16968002/diff/15001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_unittest_occlusion.cc:353:
filters.Append(FilterOperation::CreateOpacityFilter(0.5));
On 2013/06/18 23:08:52, jamesr wrote:
> 0.5f

Done.

https://codereview.chromium.org/16968002/diff/15001/webkit/renderer/composito...
File webkit/renderer/compositor_bindings/web_layer_impl.cc (right):

https://codereview.chromium.org/16968002/diff/15001/webkit/renderer/composito...
webkit/renderer/compositor_bindings/web_layer_impl.cc:161:
filters).AsFilterOperations());
On 2013/06/18 23:08:52, jamesr wrote:
> odd wrapping and casting. this probably deserves an intermediate variable to
> hold the result of the downcast then the actual call

Done.

[jamesr, 2013-06-19 17:15:03]

lgtm

https://codereview.chromium.org/16968002/diff/35001/cc/output/filter_operation.h
File cc/output/filter_operation.h (right):

https://codereview.chromium.org/16968002/diff/35001/cc/output/filter_operatio...
cc/output/filter_operation.h:123: return FilterOperation(GRAYSCALE, 0.0);
0.f

https://codereview.chromium.org/16968002/diff/35001/cc/output/filter_operatio...
File cc/output/filter_operations.cc (right):

https://codereview.chromium.org/16968002/diff/35001/cc/output/filter_operatio...
cc/output/filter_operations.cc:50: float d = floorf(std_deviation * 3 * sqrt(8.f
* atan(1.f)) / 4 + 0.5);
4.f, 0.5f

[ajuma, 2013-06-19 17:41:26]

+piman for ui/compositor/ and content/ OWNERS
+jschuh for IPC security review

https://codereview.chromium.org/16968002/diff/35001/cc/output/filter_operation.h
File cc/output/filter_operation.h (right):

https://codereview.chromium.org/16968002/diff/35001/cc/output/filter_operatio...
cc/output/filter_operation.h:123: return FilterOperation(GRAYSCALE, 0.0);
On 2013/06/19 17:15:04, jamesr wrote:
> 0.f

Done.

https://codereview.chromium.org/16968002/diff/35001/cc/output/filter_operatio...
File cc/output/filter_operations.cc (right):

https://codereview.chromium.org/16968002/diff/35001/cc/output/filter_operatio...
cc/output/filter_operations.cc:50: float d = floorf(std_deviation * 3 * sqrt(8.f
* atan(1.f)) / 4 + 0.5);
On 2013/06/19 17:15:04, jamesr wrote:
> 4.f, 0.5f

Done.

[piman, 2013-06-19 18:27:10]

LGTM \o/

[ajuma, 2013-06-24 13:35:44]

+palmer for IPC security review

Looks like jschuh@ is away, so switching IPC review to palmer@.

[palmer, 2013-06-24 19:11:44]

IPC security LGTM

[commit-bot: I haz the power, 2013-06-25 16:50:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ajuma@chromium.org/16968002/36049

[commit-bot: I haz the power, 2013-06-25 18:49:34]

Change committed as 208531