Minor UseCounter clean-ups

Minor UseCounter clean-ups

Remove the mostly useless CountBits abstraction and better align the
generic feature and CSS property usage tracking.  Also a couple other
minor cleanups and comments warning of outstanding issues.

BUG=597963
Committed: https://crrev.com/ef896c91bab517924c6658af5f71ee3d8511b3b8
Cr-Commit-Position: refs/heads/master@{#414753}

[Rick Byers, 2016-08-25 21:10:29]

rbyers@chromium.org changed reviewers:
+ dtapuska@chromium.org

[Rick Byers, 2016-08-25 21:10:29]

Hey Dave, I factored out the clean-up I did from
https://codereview.chromium.org/2023903003/ to land first.  Can you please take
a look?  There should be no semantics/behavior changes here, just code cleanup.

Thanks,
   Rick

[dtapuska, 2016-08-26 14:37:22]

lgtm but I had a few nits

https://codereview.chromium.org/2284503002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/frame/UseCounter.cpp (right):

https://codereview.chromium.org/2284503002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/UseCounter.cpp:646: for (unsigned i = 0; i
< NumberOfFeatures; ++i) {
should likely be a size_t

https://codereview.chromium.org/2284503002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/UseCounter.cpp:658: for (int i =
firstCSSProperty; i <= lastUnresolvedCSSProperty; ++i) {
samething; BitVector works on size_t.

Sigh; this entire class uses ints :-( Maybe it isn't worth it.

https://codereview.chromium.org/2284503002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/workers/WorkerGlobalScope.cpp (right):

https://codereview.chromium.org/2284503002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/workers/WorkerGlobalScope.cpp:93: if
(!m_deprecationWarningBits.quickGet(feature)) {
Do you need the DCHECKS here you lose without using CountBits now?

[Rick Byers, 2016-08-26 14:55:48]

Thanks!

https://codereview.chromium.org/2284503002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/frame/UseCounter.cpp (right):

https://codereview.chromium.org/2284503002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/UseCounter.cpp:646: for (unsigned i = 0; i
< NumberOfFeatures; ++i) {
On 2016/08/26 14:37:22, dtapuska wrote:
> should likely be a size_t

Done.

https://codereview.chromium.org/2284503002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/UseCounter.cpp:658: for (int i =
firstCSSProperty; i <= lastUnresolvedCSSProperty; ++i) {
On 2016/08/26 14:37:22, dtapuska wrote:
> samething; BitVector works on size_t.
> 
> Sigh; this entire class uses ints :-( Maybe it isn't worth it.

It's only these two you've pointed out that are used for looping over the
BitVector.  I've fixed them.  The others make more sense as an int to me -
they're histogram sample values which is an int32_t.

https://codereview.chromium.org/2284503002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/workers/WorkerGlobalScope.cpp (right):

https://codereview.chromium.org/2284503002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/workers/WorkerGlobalScope.cpp:93: if
(!m_deprecationWarningBits.quickGet(feature)) {
On 2016/08/26 14:37:22, dtapuska wrote:
> Do you need the DCHECKS here you lose without using CountBits now?

Sure.  It seems pretty unlikely to be an issue but I'll add them.

[Rick Byers, 2016-08-26 14:56:05]

The CQ bit was checked by rbyers@chromium.org

[Rick Byers, 2016-08-26 14:56:06]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtapuska@chromium.org
Link to the patchset: https://codereview.chromium.org/2284503002/#ps20001
(title: "Tweaks for CR")

[commit-bot: I haz the power, 2016-08-26 14:56:24]

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

[commit-bot: I haz the power, 2016-08-26 14:56:25]

This CL has an open dependency (Issue 2284503002 Patch 1). Please resolve the
dependency and try again.
If you are sure that there is no real dependency, please use one of the options
listed in https://goo.gl/9Es4OR to land the CL.

[dtapuska, 2016-08-26 15:00:05]

On 2016/08/26 at 14:56:25, commit-bot wrote:
> This CL has an open dependency (Issue 2284503002 Patch 1). Please resolve the
dependency and try again.
> If you are sure that there is no real dependency, please use one of the
options listed in https://goo.gl/9Es4OR to land the CL.

still lgtm

[Rick Byers, 2016-08-26 15:38:46]

The CQ bit was checked by rbyers@chromium.org

[Rick Byers, 2016-08-26 15:38:47]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtapuska@chromium.org
Link to the patchset: https://codereview.chromium.org/2284503002/#ps40001
(title: "Fix dependency")

[commit-bot: I haz the power, 2016-08-26 15:39:11]

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-08-26 18:27:47]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-26 18:30:24]

Description was changed from

==========
Minor UseCounter clean-ups

Remove the mostly useless CountBits abstraction and better align the
generic feature and CSS property usage tracking.  Also a couple other
minor cleanups and comments warning of outstanding issues.

BUG=597963
==========

to

==========
Minor UseCounter clean-ups

Remove the mostly useless CountBits abstraction and better align the
generic feature and CSS property usage tracking.  Also a couple other
minor cleanups and comments warning of outstanding issues.

BUG=597963

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

[commit-bot: I haz the power, 2016-08-26 18:30:25]

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