Let clients of FilterEffectBuilder compute/provide the reference-box

Let clients of FilterEffectBuilder compute/provide the reference-box

Rather than having code to compute the reference box for the various
clients that use FilterEffectBuilder in the class itself, let clients
compute the desired reference box themselves and pass it along.
This separates concerns, and makes code for computing the reference box
less defensive. It also eliminates the need to add even more cases in
the future (like for SVG shorthand support.)

Also push calls to resolveReferenceFilters() in PaintLayer closer to the
filter-building calls, and rename computeFilterOperations to
addReflectionToFilterOperations. This allows us to eliminate a redundant
call to resolveReferenceFilters() in the mapRectForFilter() code-path.

BUG=439970
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Committed: https://crrev.com/0dc62d2553f96900e50d4f43154435b601c281b6
Cr-Commit-Position: refs/heads/master@{#419290}

[fs, 2016-09-16 16:52:01]

Description was changed from

==========
Let clients of FilterEffectBuilder compute/provide the reference-box

Rather than having code to compute the reference box for the various
clients that use FilterEffectBuilder in the class itself, let clients
compute the desired reference box themselves and pass it along.
This separates concerns, and makes code for computing the reference box
less defensive. It also eliminates the need to add even more cases in
the future (like for SVG shorthand support.)

Also push calls to resolveReferenceFilters() in PaintLayer closer to the
filter-building calls, and rename computeFilterOperations to
addReflectionToFilterOperations. This allows us to eliminate a redundant
call to resolveReferenceFilters() in the mapRectForFilter() code-path.

BUG=439970
==========

to

==========
Let clients of FilterEffectBuilder compute/provide the reference-box

Rather than having code to compute the reference box for the various
clients that use FilterEffectBuilder in the class itself, let clients
compute the desired reference box themselves and pass it along.
This separates concerns, and makes code for computing the reference box
less defensive. It also eliminates the need to add even more cases in
the future (like for SVG shorthand support.)

Also push calls to resolveReferenceFilters() in PaintLayer closer to the
filter-building calls, and rename computeFilterOperations to
addReflectionToFilterOperations. This allows us to eliminate a redundant
call to resolveReferenceFilters() in the mapRectForFilter() code-path.

BUG=439970
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[fs, 2016-09-16 16:53:14]

The CQ bit was checked by fs@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2016-09-16 16:53:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[fs, 2016-09-16 18:35:10]

fs@opera.com changed reviewers:
+ chrishtr@chromium.org, senorblanco@chromium.org

[Stephen White, 2016-09-16 19:14:23]

I like this direction. Just one question.

https://codereview.chromium.org/2343173002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/PaintLayer.cpp (right):

https://codereview.chromium.org/2343173002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/PaintLayer.cpp:2823: FilterOperations
filterOperations = addReflectionToFilterOperations(layoutObject()->styleRef());
Isn't this already handled by the call to updateFilterEffectBuilder() call on
L2821? It now seems to call addReflectionToFilterOperations().

[Stephen White, 2016-09-16 19:16:54]

LGTM

https://codereview.chromium.org/2343173002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/PaintLayer.cpp (right):

https://codereview.chromium.org/2343173002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/PaintLayer.cpp:2823: FilterOperations
filterOperations = addReflectionToFilterOperations(layoutObject()->styleRef());
On 2016/09/16 19:14:23, Stephen White wrote:
> Isn't this already handled by the call to updateFilterEffectBuilder() call on
> L2821? It now seems to call addReflectionToFilterOperations().

Oh, I see now that the computed filter operations aren't retained. Never mind.

[fs, 2016-09-16 19:20:37]

https://codereview.chromium.org/2343173002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/PaintLayer.cpp (right):

https://codereview.chromium.org/2343173002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/PaintLayer.cpp:2823: FilterOperations
filterOperations = addReflectionToFilterOperations(layoutObject()->styleRef());
On 2016/09/16 at 19:14:23, Stephen White wrote:
> Isn't this already handled by the call to updateFilterEffectBuilder() call on
L2821? It now seems to call addReflectionToFilterOperations().

It isn't handled because there (in updateFilterEffectBuilder) we make a copy of
the Vector (FilterOperations) that we then build the FilterEffect chain from. So
because of that copy we have to start afresh here and re-add the reflection
filter. All very roundabout this, and I hope I'll be able to simplify it in
future CLs.

[commit-bot: I haz the power, 2016-09-16 19:56:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-16 19:56:42]

Dry run: This issue passed the CQ dry run.

[fs, 2016-09-16 21:55:37]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2016-09-16 21:56:26]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-16 22:04:07]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-09-16 22:06:31]

Description was changed from

==========
Let clients of FilterEffectBuilder compute/provide the reference-box

Rather than having code to compute the reference box for the various
clients that use FilterEffectBuilder in the class itself, let clients
compute the desired reference box themselves and pass it along.
This separates concerns, and makes code for computing the reference box
less defensive. It also eliminates the need to add even more cases in
the future (like for SVG shorthand support.)

Also push calls to resolveReferenceFilters() in PaintLayer closer to the
filter-building calls, and rename computeFilterOperations to
addReflectionToFilterOperations. This allows us to eliminate a redundant
call to resolveReferenceFilters() in the mapRectForFilter() code-path.

BUG=439970
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Let clients of FilterEffectBuilder compute/provide the reference-box

Rather than having code to compute the reference box for the various
clients that use FilterEffectBuilder in the class itself, let clients
compute the desired reference box themselves and pass it along.
This separates concerns, and makes code for computing the reference box
less defensive. It also eliminates the need to add even more cases in
the future (like for SVG shorthand support.)

Also push calls to resolveReferenceFilters() in PaintLayer closer to the
filter-building calls, and rename computeFilterOperations to
addReflectionToFilterOperations. This allows us to eliminate a redundant
call to resolveReferenceFilters() in the mapRectForFilter() code-path.

BUG=439970
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Committed: https://crrev.com/0dc62d2553f96900e50d4f43154435b601c281b6
Cr-Commit-Position: refs/heads/master@{#419290}
==========

[commit-bot: I haz the power, 2016-09-16 22:06:32]

Patchset 1 (id:??) landed as
https://crrev.com/0dc62d2553f96900e50d4f43154435b601c281b6
Cr-Commit-Position: refs/heads/master@{#419290}