Add internals.isCSSPropertyUseCounted functionality

Add internals.isCSSPropertyUseCounted functionality

This patch adds the functionality of
internals.isCSSPropertyUseCounted to test whether a particular CSSProperty
is in use in a file. This function is being added to help improve testing
for improvements made to UseCounter methods for counting CSSProperty.

There is a currently a internals.isUseCounted that is used for
testing whether a current feature is UseCounted.

BUG=589343
Committed: https://crrev.com/04de728dedc272262c320c5d24f5df8df2a0db7e
Cr-Commit-Position: refs/heads/master@{#377846}

[nainar, 2016-02-21 23:23:14]

Description was changed from

==========
WIP
BUG=
==========

to

==========
Add window.internals.isCSSPropertyUseCounted functionality

This patch adds the functionality of 
window.internal.isCSSPropertyUseCounted to test whether a particular 
CSSProperty is in use in a file.
==========

[nainar, 2016-02-21 23:23:14]

nainar@chromium.org changed reviewers:
+ alancutter@chromium.org

[alancutter (OOO until 2018), 2016-02-22 00:06:45]

Why do we need this function? Is there a patch that depends on it?

https://codereview.chromium.org/1711703002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/css/window-internals-isCSSPropertyUseCounted.html
(right):

https://codereview.chromium.org/1711703002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/css/window-internals-isCSSPropertyUseCounted.html:11:
opacity: 100px;
You should test setting shorthands + checking longhands.

https://codereview.chromium.org/1711703002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/css/window-internals-isCSSPropertyUseCounted.html:22:
var CSSPropertyPaddingLeft = 166; //taken from CSSPropertyNames.h
CSSPropertyNames is generated, these numbers can change at any time, pass
strings instead.

https://codereview.chromium.org/1711703002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/css/window-internals-isCSSPropertyUseCounted.html:29:
assert_false(window.internals.isCSSPropertyUseCounted(document,
CSSPropertyOpacity));
Why is this false?

[nainar, 2016-02-24 02:30:14]

Description was changed from

==========
Add window.internals.isCSSPropertyUseCounted functionality

This patch adds the functionality of 
window.internal.isCSSPropertyUseCounted to test whether a particular 
CSSProperty is in use in a file.
==========

to

==========
Add window.internals.isCSSPropertyUseCounted functionality

This patch adds the functionality of 
window.internal.isCSSPropertyUseCounted to test whether a particular 
CSSProperty is in use in a file. This function is being added to help 
improve testing for improvements made to UseCounter methods for 
counting CSSProperty.

There is a currently a window.internal.isUseCounted that is used for 
testing whether a current feature is UseCounted.
==========

[nainar, 2016-02-24 02:31:23]

@alancutter

I've added the reasoning behind the patch in the description. 

PTAL?

Thanks!

https://codereview.chromium.org/1711703002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/css/window-internals-isCSSPropertyUseCounted.html
(right):

https://codereview.chromium.org/1711703002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/css/window-internals-isCSSPropertyUseCounted.html:11:
opacity: 100px;
Done.

https://codereview.chromium.org/1711703002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/css/window-internals-isCSSPropertyUseCounted.html:22:
var CSSPropertyPaddingLeft = 166; //taken from CSSPropertyNames.h
Done.

https://codereview.chromium.org/1711703002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/css/window-internals-isCSSPropertyUseCounted.html:29:
assert_false(window.internals.isCSSPropertyUseCounted(document,
CSSPropertyOpacity));
Isn't specified in the author style sheet

[alancutter (OOO until 2018), 2016-02-24 02:45:52]

s/window.internal./internals./ in the description.

https://codereview.chromium.org/1711703002/diff/100001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/css/window-internals-isCSSPropertyUseCounted.html
(right):

https://codereview.chromium.org/1711703002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/css/window-internals-isCSSPropertyUseCounted.html:11:
opacity: 100px;
We should make invalid values obvious e.g. "opacity: invalid value;".

https://codereview.chromium.org/1711703002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/css/window-internals-isCSSPropertyUseCounted.html:20:
assert_true(window.internals.isCSSPropertyUseCounted(document, "color"));
No need for "window.".

https://codereview.chromium.org/1711703002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/css/window-internals-isCSSPropertyUseCounted.html:35:
}, "Test windows.internal.isCSSPropertyUseCounted functionality - a property not
set in author stylesheet should be false");
This description is a bit weird, it's actually testing a property with an
invalid value. Can we have both?

You should add a test for properties set only in the user agent stylesheet.

https://codereview.chromium.org/1711703002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/frame/UseCounter.cpp (right):

https://codereview.chromium.org/1711703002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/frame/UseCounter.cpp:697: return
host->useCounter().isCounted(cssPropertyID(string));
ASSERT not CSSPropertyInvalid.

[nainar, 2016-02-24 03:32:23]

Description was changed from

==========
Add window.internals.isCSSPropertyUseCounted functionality

This patch adds the functionality of 
window.internal.isCSSPropertyUseCounted to test whether a particular 
CSSProperty is in use in a file. This function is being added to help 
improve testing for improvements made to UseCounter methods for 
counting CSSProperty.

There is a currently a window.internal.isUseCounted that is used for 
testing whether a current feature is UseCounted.
==========

to

==========
Add internals.isCSSPropertyUseCounted functionality

This patch adds the functionality of 
internal.isCSSPropertyUseCounted to test whether a particular CSSProperty 
is in use in a file. This function is being added to help improve testing 
for improvements made to UseCounter methods for counting CSSProperty.

There is a currently a internal.isUseCounted that is used for 
testing whether a current feature is UseCounted.
==========

[nainar, 2016-02-24 03:34:26]

Made the changes in the description as well. 

PTAl?

Thanks!

https://codereview.chromium.org/1711703002/diff/100001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/css/window-internals-isCSSPropertyUseCounted.html
(right):

https://codereview.chromium.org/1711703002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/css/window-internals-isCSSPropertyUseCounted.html:11:
opacity: 100px;
Done.

https://codereview.chromium.org/1711703002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/css/window-internals-isCSSPropertyUseCounted.html:20:
assert_true(window.internals.isCSSPropertyUseCounted(document, "color"));
Done.

https://codereview.chromium.org/1711703002/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/css/window-internals-isCSSPropertyUseCounted.html:35:
}, "Test windows.internal.isCSSPropertyUseCounted functionality - a property not
set in author stylesheet should be false");
Done.

https://codereview.chromium.org/1711703002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/frame/UseCounter.cpp (right):

https://codereview.chromium.org/1711703002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/frame/UseCounter.cpp:697: return
host->useCounter().isCounted(cssPropertyID(string));
Done.

[alancutter (OOO until 2018), 2016-02-24 03:40:07]

lgtm with nit.

s/internal./internals./ in the description.

[Timothy Loh, 2016-02-24 03:40:35]

This needs a bug associated with it to provide context for why we're actually
doing this.

https://codereview.chromium.org/1711703002/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/css/window-internals-isCSSPropertyUseCounted.html
(right):

https://codereview.chromium.org/1711703002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/css/window-internals-isCSSPropertyUseCounted.html:28:
assert_false(internals.isCSSPropertyUseCounted(document, "border-color"));
should also check a few props like border-left, border-left-color and so on

https://codereview.chromium.org/1711703002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/frame/UseCounter.cpp (right):

https://codereview.chromium.org/1711703002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/frame/UseCounter.cpp:698: ASSERT(propertyID !=
CSSPropertyInvalid);
this needs to be defensive if you don't want clusterfuzz to yell at you

[nainar, 2016-02-24 03:43:12]

Description was changed from

==========
Add internals.isCSSPropertyUseCounted functionality

This patch adds the functionality of 
internal.isCSSPropertyUseCounted to test whether a particular CSSProperty 
is in use in a file. This function is being added to help improve testing 
for improvements made to UseCounter methods for counting CSSProperty.

There is a currently a internal.isUseCounted that is used for 
testing whether a current feature is UseCounted.
==========

to

==========
Add internals.isCSSPropertyUseCounted functionality

This patch adds the functionality of 
internals.isCSSPropertyUseCounted to test whether a particular CSSProperty 
is in use in a file. This function is being added to help improve testing 
for improvements made to UseCounter methods for counting CSSProperty.

There is a currently a internals.isUseCounted that is used for 
testing whether a current feature is UseCounted.
==========

[nainar, 2016-02-24 04:28:26]

Description was changed from

==========
Add internals.isCSSPropertyUseCounted functionality

This patch adds the functionality of 
internals.isCSSPropertyUseCounted to test whether a particular CSSProperty 
is in use in a file. This function is being added to help improve testing 
for improvements made to UseCounter methods for counting CSSProperty.

There is a currently a internals.isUseCounted that is used for 
testing whether a current feature is UseCounted.
==========

to

==========
Add internals.isCSSPropertyUseCounted functionality

This patch adds the functionality of 
internals.isCSSPropertyUseCounted to test whether a particular CSSProperty 
is in use in a file. This function is being added to help improve testing 
for improvements made to UseCounter methods for counting CSSProperty.

There is a currently a internals.isUseCounted that is used for 
testing whether a current feature is UseCounted.

BUG=589343
==========

[nainar, 2016-02-24 04:29:30]

nainar@chromium.org changed reviewers:
+ timloh@chromium.org

[nainar, 2016-02-24 04:29:53]

Added tracking bugs and made changes as per alan's comments

@timloh,

PTAL?

Thanks!

https://codereview.chromium.org/1711703002/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/css/window-internals-isCSSPropertyUseCounted.html
(right):

https://codereview.chromium.org/1711703002/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/css/window-internals-isCSSPropertyUseCounted.html:28:
assert_false(internals.isCSSPropertyUseCounted(document, "border-color"));
Done.

https://codereview.chromium.org/1711703002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/frame/UseCounter.cpp (right):

https://codereview.chromium.org/1711703002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/frame/UseCounter.cpp:698: ASSERT(propertyID !=
CSSPropertyInvalid);
Done.

[Timothy Loh, 2016-02-26 06:37:46]

lgtm

https://codereview.chromium.org/1711703002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/frame/UseCounter.cpp (right):

https://codereview.chromium.org/1711703002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/frame/UseCounter.cpp:699: if (propertyID !=
CSSPropertyInvalid)
better to rearrange this so the success case is at the very bottom. otherwise
right now the return statements from top to bottom are [fail, fail, success,
fail], which is weird.

[nainar, 2016-02-26 06:59:55]

@timloh, 

Made the changes. Thanks!

https://codereview.chromium.org/1711703002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/frame/UseCounter.cpp (right):

https://codereview.chromium.org/1711703002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/frame/UseCounter.cpp:699: if (propertyID !=
CSSPropertyInvalid)
Done.

[nainar, 2016-02-26 07:00:26]

The CQ bit was checked by nainar@chromium.org

[nainar, 2016-02-26 07:00:26]

The patchset sent to the CQ was uploaded after l-g-t-m from
alancutter@chromium.org, timloh@chromium.org
Link to the patchset: https://codereview.chromium.org/1711703002/#ps160001
(title: " ")

[commit-bot: I haz the power, 2016-02-26 07:00:48]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1711703002/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1711703002/160001

[commit-bot: I haz the power, 2016-02-26 07:19:01]

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

[commit-bot: I haz the power, 2016-02-26 07:19:01]

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

[nainar, 2016-02-26 07:20:52]

The CQ bit was checked by nainar@chromium.org

[commit-bot: I haz the power, 2016-02-26 07:21:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1711703002/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1711703002/160001

[commit-bot: I haz the power, 2016-02-26 08:13:50]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2016-02-26 08:15:28]

Description was changed from

==========
Add internals.isCSSPropertyUseCounted functionality

This patch adds the functionality of 
internals.isCSSPropertyUseCounted to test whether a particular CSSProperty 
is in use in a file. This function is being added to help improve testing 
for improvements made to UseCounter methods for counting CSSProperty.

There is a currently a internals.isUseCounted that is used for 
testing whether a current feature is UseCounted.

BUG=589343
==========

to

==========
Add internals.isCSSPropertyUseCounted functionality

This patch adds the functionality of
internals.isCSSPropertyUseCounted to test whether a particular CSSProperty
is in use in a file. This function is being added to help improve testing
for improvements made to UseCounter methods for counting CSSProperty.

There is a currently a internals.isUseCounted that is used for
testing whether a current feature is UseCounted.

BUG=589343

Committed: https://crrev.com/04de728dedc272262c320c5d24f5df8df2a0db7e
Cr-Commit-Position: refs/heads/master@{#377846}
==========

[commit-bot: I haz the power, 2016-02-26 08:15:29]

Patchset 9 (id:??) landed as
https://crrev.com/04de728dedc272262c320c5d24f5df8df2a0db7e
Cr-Commit-Position: refs/heads/master@{#377846}