Make a function to query whether a CSSPropertyID is valid and whether it has a name.

Make a function to query whether a CSSPropertyID is valid and whether it has a name.

This just tidies up and clarifies what is being checked for in a bunch of DCHECKs.

I manually looked for asserts, DCHECKs or if statements containing one of
id >= firstCSSProperty && id <= lastUnresolvedCSSProperty
or
id < firstCSSProperty || id > lastCSSProperty
and replaced them with a new method
isPropertyIDWithName

Also manually replaced things like
DCHECK_NE(property, CSSPropertyInvalid)
with
DCHECK(isValidCSSPropertyID(property));

Committed: https://crrev.com/c833755c1ac3b1e409fc1b5bbb3c40ad76d5f5ee
Cr-Commit-Position: refs/heads/master@{#423451}

[meade_UTC10, 2016-08-10 06:48:42]

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

[commit-bot: I haz the power, 2016-08-10 06:49:01]

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

[meade_UTC10, 2016-08-10 07:02:30]

meade@chromium.org changed reviewers:
+ sashab@chromium.org

[meade_UTC10, 2016-08-10 07:02:31]

This widens the allowable values of property.id() in
StylePropertySerializer::StylePropertySetForSerializer::StylePropertySetForSerializer
and
StylePropertySerializer::StylePropertySetForSerializer::shouldProcessPropertyAt,
but I'm not sure if that's a problem here. Do you know?

I'm also going to rename the function, this is experimental for now :)

[meade_UTC10, 2016-08-10 07:04:15]

On 2016/08/10 07:02:31, Eddy wrote:
> This widens the allowable values of property.id() in
>
StylePropertySerializer::StylePropertySetForSerializer::StylePropertySetForSerializer
> and
>
StylePropertySerializer::StylePropertySetForSerializer::shouldProcessPropertyAt,
> but I'm not sure if that's a problem here. Do you know?
> 
> I'm also going to rename the function, this is experimental for now :)

Scratch that, it doesn't do that. Sooo wdyt other than the function name?

[commit-bot: I haz the power, 2016-08-10 09:09:07]

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

[commit-bot: I haz the power, 2016-08-10 09:09:08]

Dry run: This issue passed the CQ dry run.

[sashab, 2016-08-15 07:23:13]

Totally lost track of this sorry!! Yes this definitely looks great :) How about
isPropertyWithName()? So we can have isPropertyValid(), isPropertyWithName(),
isInheritedProperty(), etc

Also sorry for the lateness on this!! Only saw this because of the dashboard
haha :) Next time ping me if I'm so late to review! :)

https://codereview.chromium.org/2228313002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/build/scripts/make_css_property_names.py (right):

https://codereview.chromium.org/2228313002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/build/scripts/make_css_property_names.py:56:
DCHECK_LE(value, lastCSSProperty);
Maybe DCHECK(propertyHasName() || property == CSSPropertyInvalid)?

https://codereview.chromium.org/2228313002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/animation/css/CSSAnimations.cpp (right):

https://codereview.chromium.org/2228313002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/css/CSSAnimations.cpp:118:
DCHECK_NE(property, CSSPropertyInvalid);
Maybe this could use propertyHasName() too? Should we have a isValidPropertyId()
method?

https://codereview.chromium.org/2228313002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/css/CSSAnimations.cpp:525:
DCHECK_NE(id, CSSPropertyInvalid);
Same comment as above

[sashab, 2016-09-23 03:02:01]

Are you still working on this eddy? I think its a great patch :)

[meade_UTC10, 2016-09-29 07:39:56]

Oh, thanks Sasha :)

https://codereview.chromium.org/2228313002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/build/scripts/make_css_property_names.py (right):

https://codereview.chromium.org/2228313002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/build/scripts/make_css_property_names.py:56:
DCHECK_LE(value, lastCSSProperty);
On 2016/08/15 07:23:13, sashab wrote:
> Maybe DCHECK(propertyHasName() || property == CSSPropertyInvalid)?

Done.

https://codereview.chromium.org/2228313002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/animation/css/CSSAnimations.cpp (right):

https://codereview.chromium.org/2228313002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/css/CSSAnimations.cpp:118:
DCHECK_NE(property, CSSPropertyInvalid);
On 2016/08/15 07:23:13, sashab wrote:
> Maybe this could use propertyHasName() too? Should we have a
isValidPropertyId()
> method?

Done.

https://codereview.chromium.org/2228313002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/animation/css/CSSAnimations.cpp:525:
DCHECK_NE(id, CSSPropertyInvalid);
On 2016/08/15 07:23:13, sashab wrote:
> Same comment as above

Done.

[meade_UTC10, 2016-09-29 07:44:12]

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

[commit-bot: I haz the power, 2016-09-29 07:44:50]

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

[meade_UTC10, 2016-09-29 08:09:44]

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

[commit-bot: I haz the power, 2016-09-29 08:10:04]

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

[meade_UTC10, 2016-09-29 08:10:27]

Description was changed from

==========
[EXPERIMENTAL] Make a function to encapsulate whether you can make a name out of
a CSSPropertyID

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

to

==========
Make a function to encapsulate whether you can make a name out of a
CSSPropertyID
==========

[meade_UTC10, 2016-09-29 08:10:58]

Description was changed from

==========
Make a function to encapsulate whether you can make a name out of a
CSSPropertyID
==========

to

==========
Make a function to encapsulate whether you can make a name out of a
CSSPropertyID

This just tidies up and clarifies what is being checked for in a bunch of
DCHECKs.
==========

[commit-bot: I haz the power, 2016-09-29 08:55:48]

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

[commit-bot: I haz the power, 2016-09-29 08:55:49]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[sashab, 2016-09-30 02:58:23]

lgtm

Maybe reword the patch description to be a bit clearer, and put the
regex/whatever you used to find these occurrences :) Other than that this looks
awesome.

[meade_UTC10, 2016-09-30 06:14:55]

Description was changed from

==========
Make a function to encapsulate whether you can make a name out of a
CSSPropertyID

This just tidies up and clarifies what is being checked for in a bunch of
DCHECKs.
==========

to

==========
Make a function to query whether you can make a name out of a CSSPropertyID

This just tidies up and clarifies what is being checked for in a bunch of
DCHECKs.
I manually looked for asserts, DCHECKs or if statements containing one of
id >= firstCSSProperty && id <= lastUnresolvedCSSProperty
or
id < firstCSSProperty || id > lastCSSProperty
and replaced them with a new method
isPropertyIDWithName

Also manually replaced things like
DCHECK_NE(property, CSSPropertyInvalid)
with
DCHECK(isValidCSSPropertyID(property));
==========

[meade_UTC10, 2016-09-30 06:15:14]

Description was changed from

==========
Make a function to query whether you can make a name out of a CSSPropertyID

This just tidies up and clarifies what is being checked for in a bunch of
DCHECKs.
I manually looked for asserts, DCHECKs or if statements containing one of
id >= firstCSSProperty && id <= lastUnresolvedCSSProperty
or
id < firstCSSProperty || id > lastCSSProperty
and replaced them with a new method
isPropertyIDWithName

Also manually replaced things like
DCHECK_NE(property, CSSPropertyInvalid)
with
DCHECK(isValidCSSPropertyID(property));
==========

to

==========
Make a function to query whether you can make a name out of a CSSPropertyID

This just tidies up and clarifies what is being checked for in a bunch of
DCHECKs.

I manually looked for asserts, DCHECKs or if statements containing one of
id >= firstCSSProperty && id <= lastUnresolvedCSSProperty
or
id < firstCSSProperty || id > lastCSSProperty
and replaced them with a new method
isPropertyIDWithName

Also manually replaced things like
DCHECK_NE(property, CSSPropertyInvalid)
with
DCHECK(isValidCSSPropertyID(property));
==========

[meade_UTC10, 2016-09-30 06:16:01]

Description was changed from

==========
Make a function to query whether you can make a name out of a CSSPropertyID

This just tidies up and clarifies what is being checked for in a bunch of
DCHECKs.

I manually looked for asserts, DCHECKs or if statements containing one of
id >= firstCSSProperty && id <= lastUnresolvedCSSProperty
or
id < firstCSSProperty || id > lastCSSProperty
and replaced them with a new method
isPropertyIDWithName

Also manually replaced things like
DCHECK_NE(property, CSSPropertyInvalid)
with
DCHECK(isValidCSSPropertyID(property));
==========

to

==========
Make a function to query whether a CSSPropertyID is valid and whether it has a
name.

This just tidies up and clarifies what is being checked for in a bunch of
DCHECKs.

I manually looked for asserts, DCHECKs or if statements containing one of
id >= firstCSSProperty && id <= lastUnresolvedCSSProperty
or
id < firstCSSProperty || id > lastCSSProperty
and replaced them with a new method
isPropertyIDWithName

Also manually replaced things like
DCHECK_NE(property, CSSPropertyInvalid)
with
DCHECK(isValidCSSPropertyID(property));
==========

[meade_UTC10, 2016-09-30 06:16:08]

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

[commit-bot: I haz the power, 2016-09-30 06:16:20]

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

[meade_UTC10, 2016-09-30 06:17:37]

Updated the CL description. Unfortunately it was not so fancy as to have regexes
:p

https://codereview.chromium.org/2228313002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/build/scripts/make_css_property_names.py (right):

https://codereview.chromium.org/2228313002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/build/scripts/make_css_property_names.py:56:
DCHECK_LE(value, lastCSSProperty);
On 2016/09/29 07:39:56, Eddy wrote:
> On 2016/08/15 07:23:13, sashab wrote:
> > Maybe DCHECK(propertyHasName() || property == CSSPropertyInvalid)?
> 
> Done.

Actually that doesn't work - lastCSSProperty != lastUnresolvedCSSProperty :)
I reverted my change for this comment.

[meade_UTC10, 2016-09-30 06:18:30]

meade@chromium.org changed reviewers:
+ rune@opera.com

[meade_UTC10, 2016-09-30 06:18:31]

+rune for OWNERS

PTAL, thanks!

[commit-bot: I haz the power, 2016-09-30 07:56:15]

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

[commit-bot: I haz the power, 2016-09-30 07:56:16]

Dry run: This issue passed the CQ dry run.

[rune, 2016-09-30 09:48:33]

lgtm with naming nit.

https://codereview.chromium.org/2228313002/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/build/scripts/make_css_property_names.py (right):

https://codereview.chromium.org/2228313002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/build/scripts/make_css_property_names.py:47: inline
bool isPropertyIDWithName(int id)
Should the name here also include "CSS"? isCSSPropertyIDWithName?

[meade_UTC10, 2016-10-06 03:49:39]

The CQ bit was checked by meade@chromium.org

[meade_UTC10, 2016-10-06 03:49:40]

The patchset sent to the CQ was uploaded after l-g-t-m from sashab@chromium.org
Link to the patchset: https://codereview.chromium.org/2228313002/#ps160001
(title: "Revert incorrect changes")

[meade_UTC10, 2016-10-06 03:49:42]

The CQ bit was unchecked by meade@chromium.org

[meade_UTC10, 2016-10-06 04:42:05]

https://codereview.chromium.org/2228313002/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/build/scripts/make_css_property_names.py (right):

https://codereview.chromium.org/2228313002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/build/scripts/make_css_property_names.py:47: inline
bool isPropertyIDWithName(int id)
On 2016/09/30 09:48:33, rune wrote:
> Should the name here also include "CSS"? isCSSPropertyIDWithName?

Done.

[meade_UTC10, 2016-10-06 04:56:46]

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

[commit-bot: I haz the power, 2016-10-06 04:57:06]

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

[meade_UTC10, 2016-10-06 05:04:49]

The CQ bit was checked by meade@chromium.org

[meade_UTC10, 2016-10-06 05:04:49]

The patchset sent to the CQ was uploaded after l-g-t-m from sashab@chromium.org,
rune@opera.com
Link to the patchset: https://codereview.chromium.org/2228313002/#ps200001
(title: "Revert spurious change")

[commit-bot: I haz the power, 2016-10-06 05:05:06]

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-10-06 06:26:32]

Description was changed from

==========
Make a function to query whether a CSSPropertyID is valid and whether it has a
name.

This just tidies up and clarifies what is being checked for in a bunch of
DCHECKs.

I manually looked for asserts, DCHECKs or if statements containing one of
id >= firstCSSProperty && id <= lastUnresolvedCSSProperty
or
id < firstCSSProperty || id > lastCSSProperty
and replaced them with a new method
isPropertyIDWithName

Also manually replaced things like
DCHECK_NE(property, CSSPropertyInvalid)
with
DCHECK(isValidCSSPropertyID(property));
==========

to

==========
Make a function to query whether a CSSPropertyID is valid and whether it has a
name.

This just tidies up and clarifies what is being checked for in a bunch of
DCHECKs.

I manually looked for asserts, DCHECKs or if statements containing one of
id >= firstCSSProperty && id <= lastUnresolvedCSSProperty
or
id < firstCSSProperty || id > lastCSSProperty
and replaced them with a new method
isPropertyIDWithName

Also manually replaced things like
DCHECK_NE(property, CSSPropertyInvalid)
with
DCHECK(isValidCSSPropertyID(property));
==========

[commit-bot: I haz the power, 2016-10-06 06:26:34]

Committed patchset #11 (id:200001)

[commit-bot: I haz the power, 2016-10-06 06:28:24]

Description was changed from

==========
Make a function to query whether a CSSPropertyID is valid and whether it has a
name.

This just tidies up and clarifies what is being checked for in a bunch of
DCHECKs.

I manually looked for asserts, DCHECKs or if statements containing one of
id >= firstCSSProperty && id <= lastUnresolvedCSSProperty
or
id < firstCSSProperty || id > lastCSSProperty
and replaced them with a new method
isPropertyIDWithName

Also manually replaced things like
DCHECK_NE(property, CSSPropertyInvalid)
with
DCHECK(isValidCSSPropertyID(property));
==========

to

==========
Make a function to query whether a CSSPropertyID is valid and whether it has a
name.

This just tidies up and clarifies what is being checked for in a bunch of
DCHECKs.

I manually looked for asserts, DCHECKs or if statements containing one of
id >= firstCSSProperty && id <= lastUnresolvedCSSProperty
or
id < firstCSSProperty || id > lastCSSProperty
and replaced them with a new method
isPropertyIDWithName

Also manually replaced things like
DCHECK_NE(property, CSSPropertyInvalid)
with
DCHECK(isValidCSSPropertyID(property));

Committed: https://crrev.com/c833755c1ac3b1e409fc1b5bbb3c40ad76d5f5ee
Cr-Commit-Position: refs/heads/master@{#423451}
==========

[commit-bot: I haz the power, 2016-10-06 06:28:27]

Patchset 11 (id:??) landed as
https://crrev.com/c833755c1ac3b1e409fc1b5bbb3c40ad76d5f5ee
Cr-Commit-Position: refs/heads/master@{#423451}