Cleanup CSSGradientValue

Cleanup CSSGradientValue

1) move member fields to subclasses, to ensure they're only used/stored
   by gradient types which need them

2) remove setters; instead of manipulating fields after instantiation,
   pass all needed params to the ctor

3) refactor the CSS parsing methods for #2: defer gradient value
   instantiation until all needed params are parsed

4) constify member value pointers


Refactor only, no functional changes.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
BUG=614368
Review-Url: https://codereview.chromium.org/2817523002
Cr-Commit-Position: refs/heads/master@{#464034}
Committed: https://chromium.googlesource.com/chromium/src/+/55633a0e8db901aa6ce0990c99352b5d77087d06

[f(malita), 2017-04-11 21:22:37]

Description was changed from

==========
Cleanup CSSGradientValue

WIP

BUG=
==========

to

==========
Cleanup CSSGradientValue

WIP

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

[f(malita), 2017-04-11 21:22:42]

The CQ bit was checked by fmalita@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-11 21:23:06]

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

[f(malita), 2017-04-11 21:36:30]

The CQ bit was checked by fmalita@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-11 21:37:15]

Dry run: 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, 2017-04-11 23:53:55]

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

[commit-bot: I haz the power, 2017-04-11 23:53:56]

Dry run: This issue passed the CQ dry run.

[f(malita), 2017-04-12 12:51:30]

Description was changed from

==========
Cleanup CSSGradientValue

WIP

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

to

==========
Cleanup CSSGradientValue

1) move member fields to subclasses, to ensure they're only used/stored
   by gradient types which need them

2) remove setters; instead of manipulating fields after instantiation,
   pass all needed params to the ctor

3) refactor the CSS parsing methods for #2: defer gradient value
   instantiation until all needed params are parsed

4) constify member value pointers


Refactor only, no functional changes.

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

[f(malita), 2017-04-12 12:51:30]

fmalita@chromium.org changed reviewers:
+ fs@opera.com

[f(malita), 2017-04-12 12:52:22]

Description was changed from

==========
Cleanup CSSGradientValue

1) move member fields to subclasses, to ensure they're only used/stored
   by gradient types which need them

2) remove setters; instead of manipulating fields after instantiation,
   pass all needed params to the ctor

3) refactor the CSS parsing methods for #2: defer gradient value
   instantiation until all needed params are parsed

4) constify member value pointers


Refactor only, no functional changes.

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

to

==========
Cleanup CSSGradientValue

1) move member fields to subclasses, to ensure they're only used/stored
   by gradient types which need them

2) remove setters; instead of manipulating fields after instantiation,
   pass all needed params to the ctor

3) refactor the CSS parsing methods for #2: defer gradient value
   instantiation until all needed params are parsed

4) constify member value pointers


Refactor only, no functional changes.

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

[fs, 2017-04-12 13:28:15]

lgtm

https://codereview.chromium.org/2817523002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/CSSGradientValue.h (right):

https://codereview.chromium.org/2817523002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSGradientValue.h:189: Member<const
CSSPrimitiveValue> angle_;  // may be null.
Could we try to use const CSSValue& as (argument) type where applicable?
(Member<> accepts references during construction IIRC.)

[f(malita), 2017-04-12 13:59:05]

https://codereview.chromium.org/2817523002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/CSSGradientValue.h (right):

https://codereview.chromium.org/2817523002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSGradientValue.h:189: Member<const
CSSPrimitiveValue> angle_;  // may be null.
On 2017/04/12 13:28:15, fs wrote:
> Could we try to use const CSSValue& as (argument) type where applicable?
> (Member<> accepts references during construction IIRC.)

Yeah - the problem is there's much implicit voodoo in CSSFooGradientValues and
most of these arguments are optional.

Actually, I can't find a single one for any of the gradient types that is
guaranteed to be non-null in all instantiations :)  I'll update the misleading
comment to no longer single-out angle_.

(maybe we could add some sanity if we further specialize the deprecated gradient
types - but that's a lot of code shuffling for little gain)

[fs, 2017-04-12 14:01:26]

https://codereview.chromium.org/2817523002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/CSSGradientValue.h (right):

https://codereview.chromium.org/2817523002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/CSSGradientValue.h:189: Member<const
CSSPrimitiveValue> angle_;  // may be null.
On 2017/04/12 at 13:59:05, f(malita) wrote:
> On 2017/04/12 13:28:15, fs wrote:
> > Could we try to use const CSSValue& as (argument) type where applicable?
> > (Member<> accepts references during construction IIRC.)
> 
> Yeah - the problem is there's much implicit voodoo in CSSFooGradientValues and
most of these arguments are optional.
> 
> Actually, I can't find a single one for any of the gradient types that is
guaranteed to be non-null in all instantiations :)  I'll update the misleading
comment to no longer single-out angle_.
> 
> (maybe we could add some sanity if we further specialize the deprecated
gradient types - but that's a lot of code shuffling for little gain)

Roger that.

[f(malita), 2017-04-12 14:04:10]

The CQ bit was checked by fmalita@chromium.org

[f(malita), 2017-04-12 14:04:11]

The patchset sent to the CQ was uploaded after l-g-t-m from fs@opera.com
Link to the patchset: https://codereview.chromium.org/2817523002/#ps40001
(title: "some more comments")

[commit-bot: I haz the power, 2017-04-12 14:04: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, 2017-04-12 15:34:56]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1492005850981890,
"parent_rev": "d871a9338a39d46cbb25ee6571a4e225f049bda1", "commit_rev":
"55633a0e8db901aa6ce0990c99352b5d77087d06"}

[commit-bot: I haz the power, 2017-04-12 15:35:46]

Description was changed from

==========
Cleanup CSSGradientValue

1) move member fields to subclasses, to ensure they're only used/stored
   by gradient types which need them

2) remove setters; instead of manipulating fields after instantiation,
   pass all needed params to the ctor

3) refactor the CSS parsing methods for #2: defer gradient value
   instantiation until all needed params are parsed

4) constify member value pointers


Refactor only, no functional changes.

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

to

==========
Cleanup CSSGradientValue

1) move member fields to subclasses, to ensure they're only used/stored
   by gradient types which need them

2) remove setters; instead of manipulating fields after instantiation,
   pass all needed params to the ctor

3) refactor the CSS parsing methods for #2: defer gradient value
   instantiation until all needed params are parsed

4) constify member value pointers


Refactor only, no functional changes.

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

Review-Url: https://codereview.chromium.org/2817523002
Cr-Commit-Position: refs/heads/master@{#464034}
Committed:
https://chromium.googlesource.com/chromium/src/+/55633a0e8db901aa6ce0990c9935...
==========

[commit-bot: I haz the power, 2017-04-12 15:35:47]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/55633a0e8db901aa6ce0990c9935...