Add some additional SMIL use counters

Add some additional SMIL use counters

To gain a better understanding of the usage of particular features of SMIL
animations, this patch adds use counters for the following conditions:
- event-value used in 'begin' or 'end' property,
- syncbase-value used in 'begin' or 'end' property, and
- SMIL element inserted into document after load event has fired.

Committed: https://crrev.com/42fe281c43e4315970ec38b408f7622881271a87
Cr-Commit-Position: refs/heads/master@{#406800}

[suzyh_UTC10 (ex-contributor), 2016-07-11 07:23:20]

suzyh@chromium.org changed reviewers:
+ shans@chromium.org

[suzyh_UTC10 (ex-contributor), 2016-07-11 07:24:16]

With thanks to ericwilligers for the tests.

Shane, can you take the first look at this patch? Also intending to pass it by
pdr and then get any remaining OWNERS approvals.

[shans, 2016-07-11 22:16:10]

On 2016/07/11 at 07:24:16, suzyh wrote:
> With thanks to ericwilligers for the tests.
> 
> Shane, can you take the first look at this patch? Also intending to pass it by
pdr and then get any remaining OWNERS approvals.

LGTM so far, added a nit but I'm fine if you want to ignore it.

Thanks for this! Exciting!

[suzyh_UTC10 (ex-contributor), 2016-07-11 22:30:13]

On 2016/07/11 at 22:16:10, shans wrote:
> On 2016/07/11 at 07:24:16, suzyh wrote:
> > With thanks to ericwilligers for the tests.
> > 
> > Shane, can you take the first look at this patch? Also intending to pass it
by pdr and then get any remaining OWNERS approvals.
> 
> LGTM so far, added a nit but I'm fine if you want to ignore it.
> 
> Thanks for this! Exciting!

Nit isn't showing up for me?

[shans, 2016-07-11 22:49:25]

https://codereview.chromium.org/2132323002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/svg/animation/SVGSMILElement.cpp (right):

https://codereview.chromium.org/2132323002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/animation/SVGSMILElement.cpp:477: return
false;
Would it be worth counting after this point? Seems like declarations where
baseID.isEmpty() is true don't have any effect?

https://codereview.chromium.org/2132323002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/animation/SVGSMILElement.cpp:483:
UseCounter::count(&document(), UseCounter::SVGSMILBeginOrEndEventValue);
Interesting that this is the catch-all condition. It means we might slightly
over-count, but that's probably OK.

[suzyh_UTC10 (ex-contributor), 2016-07-13 06:04:27]

suzyh@chromium.org changed reviewers:
+ pdr@chromium.org

[suzyh_UTC10 (ex-contributor), 2016-07-13 06:04:44]

+pdr for second review

https://codereview.chromium.org/2132323002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/svg/animation/SVGSMILElement.cpp (right):

https://codereview.chromium.org/2132323002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/animation/SVGSMILElement.cpp:477: return
false;
On 2016/07/11 at 22:49:24, shans wrote:
> Would it be worth counting after this point? Seems like declarations where
baseID.isEmpty() is true don't have any effect?

Done.

[Mike Lawther (Google), 2016-07-13 07:30:54]

mikelawther@chromium.org changed reviewers:
+ mikelawther@chromium.org

[Mike Lawther (Google), 2016-07-13 07:30:55]

Drive by review! Zoooom!

(just nits, please consider but nothing earth shattering, does not affect
Shane's earlier l.g.t.m)

https://codereview.chromium.org/2132323002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/svg/animations/add-after-load-use-counter.html
(right):

https://codereview.chromium.org/2132323002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/animations/add-after-load-use-counter.html:13:
var SVGInsertAnimationValue = 1442;
nit: I'd give this the same name as from the .h file (ie
SVGSMILElementInsertedAfterLoad), as it's a pretty good name already, and
matches better with the comment on line 12.

https://codereview.chromium.org/2132323002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/svg/animations/begin-use-counters.html
(right):

https://codereview.chromium.org/2132323002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/animations/begin-use-counters.html:12: var
SVGSyncbaseValue = 1441;
nit: consider using the same names as the enum from the .h file?

[suzyh_UTC10 (ex-contributor), 2016-07-13 23:08:07]

https://codereview.chromium.org/2132323002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/svg/animations/add-after-load-use-counter.html
(right):

https://codereview.chromium.org/2132323002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/animations/add-after-load-use-counter.html:13:
var SVGInsertAnimationValue = 1442;
On 2016/07/13 at 07:30:54, Mike Lawther (Google) wrote:
> nit: I'd give this the same name as from the .h file (ie
SVGSMILElementInsertedAfterLoad), as it's a pretty good name already, and
matches better with the comment on line 12.

Good call, done.

https://codereview.chromium.org/2132323002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/svg/animations/begin-use-counters.html
(right):

https://codereview.chromium.org/2132323002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/svg/animations/begin-use-counters.html:12: var
SVGSyncbaseValue = 1441;
On 2016/07/13 at 07:30:54, Mike Lawther (Google) wrote:
> nit: consider using the same names as the enum from the .h file?

Ditto.

[suzyh_UTC10 (ex-contributor), 2016-07-15 05:58:12]

pdr, ping?

[suzyh_UTC10 (ex-contributor), 2016-07-21 01:38:24]

suzyh@chromium.org changed reviewers:
+ dcheng@chromium.org, isherman@chromium.org

[suzyh_UTC10 (ex-contributor), 2016-07-21 01:38:45]

+dcheng,isherman for UseCounter/histograms OWNERS

[pdr., 2016-07-21 01:50:08]

On 2016/07/15 at 05:58:12, suzyh wrote:
> pdr, ping?

LGTM, sorry for the slow reply, first day back.

[dcheng, 2016-07-21 01:54:36]

rs lgtm for UseCounter

[Ilya Sherman, 2016-07-21 02:05:09]

histograms.xml lgtm

[suzyh_UTC10 (ex-contributor), 2016-07-21 03:51:56]

The CQ bit was checked by suzyh@chromium.org

[suzyh_UTC10 (ex-contributor), 2016-07-21 03:51:57]

The patchset sent to the CQ was uploaded after l-g-t-m from shans@chromium.org
Link to the patchset: https://codereview.chromium.org/2132323002/#ps60001
(title: "Rebase")

[commit-bot: I haz the power, 2016-07-21 03:52:23]

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-07-21 04:08:25]

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

[commit-bot: I haz the power, 2016-07-21 04:08:27]

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

[suzyh_UTC10 (ex-contributor), 2016-07-21 06:47:31]

The CQ bit was checked by suzyh@chromium.org

[commit-bot: I haz the power, 2016-07-21 06:47:50]

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

[suzyh_UTC10 (ex-contributor), 2016-07-21 06:59:12]

The CQ bit was unchecked by suzyh@chromium.org

[suzyh_UTC10 (ex-contributor), 2016-07-21 06:59:16]

The CQ bit was checked by suzyh@chromium.org

[commit-bot: I haz the power, 2016-07-21 06:59:34]

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-07-21 07:54:24]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-07-21 07:55:42]

Description was changed from

==========
Add some additional SMIL use counters

To gain a better understanding of the usage of particular features of SMIL
animations, this patch adds use counters for the following conditions:
- event-value used in 'begin' or 'end' property,
- syncbase-value used in 'begin' or 'end' property, and
- SMIL element inserted into document after load event has fired.
==========

to

==========
Add some additional SMIL use counters

To gain a better understanding of the usage of particular features of SMIL
animations, this patch adds use counters for the following conditions:
- event-value used in 'begin' or 'end' property,
- syncbase-value used in 'begin' or 'end' property, and
- SMIL element inserted into document after load event has fired.

Committed: https://crrev.com/42fe281c43e4315970ec38b408f7622881271a87
Cr-Commit-Position: refs/heads/master@{#406800}
==========

[commit-bot: I haz the power, 2016-07-21 07:55:44]

Patchset 4 (id:??) landed as
https://crrev.com/42fe281c43e4315970ec38b408f7622881271a87
Cr-Commit-Position: refs/heads/master@{#406800}