Refactor FilterData::state_ handling in SVGFilterPainter

Refactor FilterData::state_ handling in SVGFilterPainter

Make FilterData::state_ only be checked and updated within
SVGFilterPainter, and not by SVGFilterRecordingContext or the local
painting helper.
Instead SVGFilterRecordingContext only manages the recording state, and
gets passed bounds while returning a paint record.
This simplifies some of the corner-cases with regards to how "aborted"
filters are handled, getting rid of some "FilterData is null" in many
cases.

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

Review-Url: https://codereview.chromium.org/2851753002
Cr-Commit-Position: refs/heads/master@{#468085}
Committed: https://chromium.googlesource.com/chromium/src/+/aa5f200c624aeb429e0d8084a11670feccf4f540

[fs, 2017-04-28 14:02:46]

Description was changed from

==========
Refactor FilterData::state_ handling in SVGFilterPainter

Make FilterData::state_ only be checked and updated within
SVGFilterPainter, and not by SVGFilterRecordingContext or the local
painting helper.
Instead SVGFilterRecordingContext only manages the recording state, and
gets passed bounds while returning a paint record.
This simplifies some of the corner-cases with regards to how "aborted"
filters are handled, getting rid of some "FilterData is null" in many
cases.

BUG=713024
==========

to

==========
Refactor FilterData::state_ handling in SVGFilterPainter

Make FilterData::state_ only be checked and updated within
SVGFilterPainter, and not by SVGFilterRecordingContext or the local
painting helper.
Instead SVGFilterRecordingContext only manages the recording state, and
gets passed bounds while returning a paint record.
This simplifies some of the corner-cases with regards to how "aborted"
filters are handled, getting rid of some "FilterData is null" in many
cases.

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

[fs, 2017-04-28 14:02:51]

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

[commit-bot: I haz the power, 2017-04-28 14:03:56]

Dry run: CQ is trying da patch.

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

[fs, 2017-04-28 15:11:40]

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

[Stephen Chennney, 2017-04-28 16:02:42]

lgtm number 1

[commit-bot: I haz the power, 2017-04-28 16:10:27]

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

[commit-bot: I haz the power, 2017-04-28 16:10:28]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[fs, 2017-04-28 17:56:32]

The CQ bit was checked by fs@opera.com

[commit-bot: I haz the power, 2017-04-28 17:57:47]

CQ is trying da patch.

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

[chrishtr, 2017-04-28 18:20:56]

https://codereview.chromium.org/2851753002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/SVGFilterPainter.cpp (left):

https://codereview.chromium.org/2851753002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/SVGFilterPainter.cpp:156:
PaintFilteredContent(recording_context.PaintingContext(), object,
The old code would have executed this line in the case of a cycle being
detected.
Is the new early-out code correct in this case? Was it incorrect before?

[fs, 2017-04-28 18:40:52]

https://codereview.chromium.org/2851753002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/SVGFilterPainter.cpp (left):

https://codereview.chromium.org/2851753002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/SVGFilterPainter.cpp:156:
PaintFilteredContent(recording_context.PaintingContext(), object,
On 2017/04/28 at 18:20:56, chrishtr wrote:
> The old code would have executed this line in the case of a cycle being
detected.
> Is the new early-out code correct in this case? Was it incorrect before?

Previously it would've early-outed in PaintFilteredContent because of the
"filter_data->state_ != FilterData::kReadyToPaint", so produced an empty display
item - while now it will produce no display item. So the painted result should
be the same while consuming less space.

[chrishtr, 2017-04-28 18:45:49]

lgtm

https://codereview.chromium.org/2851753002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/SVGFilterPainter.cpp (left):

https://codereview.chromium.org/2851753002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/SVGFilterPainter.cpp:156:
PaintFilteredContent(recording_context.PaintingContext(), object,
On 2017/04/28 at 18:40:52, fs wrote:
> On 2017/04/28 at 18:20:56, chrishtr wrote:
> > The old code would have executed this line in the case of a cycle being
detected.
> > Is the new early-out code correct in this case? Was it incorrect before?
> 
> Previously it would've early-outed in PaintFilteredContent because of the
"filter_data->state_ != FilterData::kReadyToPaint", so produced an empty display
item - while now it will produce no display item. So the painted result should
be the same while consuming less space.

Ok great.

[commit-bot: I haz the power, 2017-04-28 19:09:46]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1493402192144570, "parent_rev":
"14ddf4a2b43673a91f552143adb6e7cf03acbb8b", "commit_rev":
"aa5f200c624aeb429e0d8084a11670feccf4f540"}

[commit-bot: I haz the power, 2017-04-28 19:10:28]

Description was changed from

==========
Refactor FilterData::state_ handling in SVGFilterPainter

Make FilterData::state_ only be checked and updated within
SVGFilterPainter, and not by SVGFilterRecordingContext or the local
painting helper.
Instead SVGFilterRecordingContext only manages the recording state, and
gets passed bounds while returning a paint record.
This simplifies some of the corner-cases with regards to how "aborted"
filters are handled, getting rid of some "FilterData is null" in many
cases.

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

to

==========
Refactor FilterData::state_ handling in SVGFilterPainter

Make FilterData::state_ only be checked and updated within
SVGFilterPainter, and not by SVGFilterRecordingContext or the local
painting helper.
Instead SVGFilterRecordingContext only manages the recording state, and
gets passed bounds while returning a paint record.
This simplifies some of the corner-cases with regards to how "aborted"
filters are handled, getting rid of some "FilterData is null" in many
cases.

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

Review-Url: https://codereview.chromium.org/2851753002
Cr-Commit-Position: refs/heads/master@{#468085}
Committed:
https://chromium.googlesource.com/chromium/src/+/aa5f200c624aeb429e0d8084a116...
==========

[commit-bot: I haz the power, 2017-04-28 19:10:28]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/aa5f200c624aeb429e0d8084a116...