Store CSSPropertyID in SVGAnimatedPropertyBase

Store CSSPropertyID in SVGAnimatedPropertyBase

With an increasing amount of SVG attributes being "promoted" to
presentation attributes, it makes sense to try to keep the property
mapping with the other attribute related data.
To make room for these additional bits in SVGAnimatedPropertyBase, pack
some of its fields into a bitfield:

 * m_isReadOnly only needs a single bit.
 * m_type only need room for 21 different values, so 5 bits should
   suffice.

With this new field in place, plumb it through for SVG element
attributes, then, as a start, use the SVG property map to simplify the
implementations of isPresentationAttributeWithSVGDOM and
isPresentationAttribute.

This could also be used to provide storage for attribute initial values
in the future (crbug.com/225807.)

BUG=225807, 369942
Committed: https://crrev.com/96564f72ad4d9b7cf8df82eb0fcd7741c6191631
Cr-Commit-Position: refs/heads/master@{#431229}

[fs, 2016-11-07 17:15:02]

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

[commit-bot: I haz the power, 2016-11-07 17:15:28]

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, 2016-11-07 18:45:50]

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

[commit-bot: I haz the power, 2016-11-07 18:45:51]

Dry run: This issue passed the CQ dry run.

[fs, 2016-11-08 09:52:46]

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

[fs, 2016-11-08 09:52:47]

I realized that having this would help with some other work ('transform' as
presentation attribute being at the fore.)

[pdr., 2016-11-08 19:44:12]

LGTM, two optional comments.

https://codereview.chromium.org/2485663002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/svg/properties/SVGAnimatedProperty.h
(right):

https://codereview.chromium.org/2485663002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/properties/SVGAnimatedProperty.h:98: enum {
kCssPropertyBits = 9 };
Can we use constexpr now?

https://groups.google.com/a/chromium.org/d/msg/chromium-dev/hDqJ4KBVqog/FLZVH...
was an old discussion about this, and it appears approved in
https://chromium-cpp.appspot.com.

https://codereview.chromium.org/2485663002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/svg/properties/SVGPropertyInfo.h (right):

https://codereview.chromium.org/2485663002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/properties/SVGPropertyInfo.h:47: // If/when
adding something to the end of this enumeration, also update the
WDYT of using the numbered enum pattern with NumberOfTypes as the last value?

E.g.,
enum AnimatedPropertyType {
  AnimatedUnknown = 0,
  AnimatedAngle = 1,
  ...
  NumberOfAnimatedPropertyTypes = 21
}

[fs, 2016-11-09 17:36:56]

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

[commit-bot: I haz the power, 2016-11-09 17:37:09]

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

[fs, 2016-11-09 17:37:42]

https://codereview.chromium.org/2485663002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/svg/properties/SVGAnimatedProperty.h
(right):

https://codereview.chromium.org/2485663002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/properties/SVGAnimatedProperty.h:98: enum {
kCssPropertyBits = 9 };
On 2016/11/08 at 19:44:12, pdr. wrote:
> Can we use constexpr now?
> 
>
https://groups.google.com/a/chromium.org/d/msg/chromium-dev/hDqJ4KBVqog/FLZVH...
was an old discussion about this, and it appears approved in
https://chromium-cpp.appspot.com.

Done.

https://codereview.chromium.org/2485663002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/svg/properties/SVGPropertyInfo.h (right):

https://codereview.chromium.org/2485663002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/properties/SVGPropertyInfo.h:47: // If/when
adding something to the end of this enumeration, also update the
On 2016/11/08 at 19:44:12, pdr. wrote:
> WDYT of using the numbered enum pattern with NumberOfTypes as the last value?
> 
> E.g.,
> enum AnimatedPropertyType {
>   AnimatedUnknown = 0,
>   AnimatedAngle = 1,
>   ...
>   NumberOfAnimatedPropertyTypes = 21
> }

I added a NumberOfAnimatedPropertyTypes sentinel, but I did not explicit values,
because would just add (unneeded) redundancy.

[commit-bot: I haz the power, 2016-11-09 17:49:44]

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

[commit-bot: I haz the power, 2016-11-09 17:49:45]

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

[pdr., 2016-11-09 18:42:55]

On 2016/11/09 at 17:37:42, fs wrote:
>
https://codereview.chromium.org/2485663002/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/svg/properties/SVGAnimatedProperty.h
(right):
> 
>
https://codereview.chromium.org/2485663002/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/svg/properties/SVGAnimatedProperty.h:98: enum {
kCssPropertyBits = 9 };
> On 2016/11/08 at 19:44:12, pdr. wrote:
> > Can we use constexpr now?
> > 
> >
https://groups.google.com/a/chromium.org/d/msg/chromium-dev/hDqJ4KBVqog/FLZVH...
was an old discussion about this, and it appears approved in
https://chromium-cpp.appspot.com.
> 
> Done.
> 
>
https://codereview.chromium.org/2485663002/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/svg/properties/SVGPropertyInfo.h (right):
> 
>
https://codereview.chromium.org/2485663002/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/svg/properties/SVGPropertyInfo.h:47: // If/when
adding something to the end of this enumeration, also update the
> On 2016/11/08 at 19:44:12, pdr. wrote:
> > WDYT of using the numbered enum pattern with NumberOfTypes as the last
value?
> > 
> > E.g.,
> > enum AnimatedPropertyType {
> >   AnimatedUnknown = 0,
> >   AnimatedAngle = 1,
> >   ...
> >   NumberOfAnimatedPropertyTypes = 21
> > }
> 
> I added a NumberOfAnimatedPropertyTypes sentinel, but I did not explicit
values, because would just add (unneeded) redundancy.

LGTM

[fs, 2016-11-10 08:47:22]

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

[commit-bot: I haz the power, 2016-11-10 08:47:30]

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, 2016-11-10 10:04:05]

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

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

Dry run: This issue passed the CQ dry run.

[fs, 2016-11-10 10:16:25]

The CQ bit was checked by fs@opera.com

[fs, 2016-11-10 10:16:25]

The patchset sent to the CQ was uploaded after l-g-t-m from pdr@chromium.org
Link to the patchset: https://codereview.chromium.org/2485663002/#ps40001
(title: "Add missing case")

[commit-bot: I haz the power, 2016-11-10 10:16:44]

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-11-10 10:21:59]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-11-10 10:23:44]

Description was changed from

==========
Store CSSPropertyID in SVGAnimatedPropertyBase

With an increasing amount of SVG attributes being "promoted" to
presentation attributes, it makes sense to try to keep the property
mapping with the other attribute related data.
To make room for these additional bits in SVGAnimatedPropertyBase, pack
some of its fields into a bitfield:

 * m_isReadOnly only needs a single bit.
 * m_type only need room for 21 different values, so 5 bits should
   suffice.

With this new field in place, plumb it through for SVG element
attributes, then, as a start, use the SVG property map to simplify the
implementations of isPresentationAttributeWithSVGDOM and
isPresentationAttribute.

This could also be used to provide storage for attribute initial values
in the future (crbug.com/225807.)

BUG=225807,369942
==========

to

==========
Store CSSPropertyID in SVGAnimatedPropertyBase

With an increasing amount of SVG attributes being "promoted" to
presentation attributes, it makes sense to try to keep the property
mapping with the other attribute related data.
To make room for these additional bits in SVGAnimatedPropertyBase, pack
some of its fields into a bitfield:

 * m_isReadOnly only needs a single bit.
 * m_type only need room for 21 different values, so 5 bits should
   suffice.

With this new field in place, plumb it through for SVG element
attributes, then, as a start, use the SVG property map to simplify the
implementations of isPresentationAttributeWithSVGDOM and
isPresentationAttribute.

This could also be used to provide storage for attribute initial values
in the future (crbug.com/225807.)

BUG=225807,369942

Committed: https://crrev.com/96564f72ad4d9b7cf8df82eb0fcd7741c6191631
Cr-Commit-Position: refs/heads/master@{#431229}
==========

[commit-bot: I haz the power, 2016-11-10 10:23:45]

Patchset 3 (id:??) landed as
https://crrev.com/96564f72ad4d9b7cf8df82eb0fcd7741c6191631
Cr-Commit-Position: refs/heads/master@{#431229}