CSS Properties and Values API: Support non-inherited custom properties

CSS Properties and Values API: Support non-inherited custom properties

This patch adds supports for registering non-inherited properties.
These properties are stored in a new class StyleNonInheritedVariables,
similar to StyleVariableData (now renamed to StyleInheritedVariables).
The new class differs in a couple of ways: it is not RefCounted and
there is no root optimisation (this wouldn't make sense anyway).

This patch fixes a bug in the getter on StyleInheritedVariables for
registered properties where we wouldn't check the root node. This is
covered by the added tests.

Non-inherited custom properties are not yet supported for the devtools
computed styles pane and @apply rules, and todos are added in relevant
places in the code.

BUG=641877
Committed: https://crrev.com/864454fb784c52c1202410dc28799c8dc0f9ad0c
Cr-Commit-Position: refs/heads/master@{#421776}

[Timothy Loh, 2016-09-27 07:49:38]

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

[meade_UTC10, 2016-09-28 04:23:29]

lgtm

https://codereview.chromium.org/2366313006/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/resolver/StyleBuilderCustom.cpp (right):

https://codereview.chromium.org/2366313006/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/resolver/StyleBuilderCustom.cpp:811: ||
(declaration.id() == CSSValueUnset && (!registration ||
registration->inherits()));
Can these be simplified by there being a method on CSSCustomPropertyDeclaration
and PropertyRegistry::Registration that's like "isInitial()" and "inherits()"?

https://codereview.chromium.org/2366313006/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/resolver/StyleBuilderCustom.cpp:812:
DCHECK(!initial || !inherit);
equivalent to !(initial && inherit), but either way is fine

https://codereview.chromium.org/2366313006/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/resolver/StyleBuilderCustom.cpp:844:
DCHECK(initial ^ inherit);
Shouldn't this be ||? I realise it has the same effect, but it can be confusing
to use bitwise operations on regular bools.

[Timothy Loh, 2016-09-29 06:18:09]

https://codereview.chromium.org/2366313006/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/resolver/StyleBuilderCustom.cpp (right):

https://codereview.chromium.org/2366313006/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/resolver/StyleBuilderCustom.cpp:811: ||
(declaration.id() == CSSValueUnset && (!registration ||
registration->inherits()));
On 2016/09/28 04:23:29, Eddy wrote:
> Can these be simplified by there being a method on
CSSCustomPropertyDeclaration
> and PropertyRegistry::Registration that's like "isInitial()" and "inherits()"?

Good idea -- I'll update CSSCustomPropertyDeclaration in a follow-up patch (this
one is big enough...) and I'll think about whether it makes sense to have a
default registration for unregistered properties.

https://codereview.chromium.org/2366313006/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/resolver/StyleBuilderCustom.cpp:812:
DCHECK(!initial || !inherit);
On 2016/09/28 04:23:29, Eddy wrote:
> equivalent to !(initial && inherit), but either way is fine

Changed (maybe it's marginally better with &&)

https://codereview.chromium.org/2366313006/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/resolver/StyleBuilderCustom.cpp:844:
DCHECK(initial ^ inherit);
On 2016/09/28 04:23:29, Eddy wrote:
> Shouldn't this be ||? I realise it has the same effect, but it can be
confusing
> to use bitwise operations on regular bools.

We want exactly one of the flags set here but || allows both set. I could write
initial != inherit but I think ^ is clearer.

[Timothy Loh, 2016-09-29 06:18:14]

The CQ bit was checked by timloh@chromium.org

[Timothy Loh, 2016-09-29 06:18:14]

The patchset sent to the CQ was uploaded after l-g-t-m from meade@chromium.org
Link to the patchset: https://codereview.chromium.org/2366313006/#ps40001
(title: "use de morgan's law")

[commit-bot: I haz the power, 2016-09-29 06:18:28]

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-09-29 07:45:38]

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

[commit-bot: I haz the power, 2016-09-29 07:45:39]

Exceeded global retry quota

[Timothy Loh, 2016-09-29 07:46:40]

The CQ bit was checked by timloh@chromium.org

[commit-bot: I haz the power, 2016-09-29 07:46:57]

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-09-29 08:00:41]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-09-29 08:03:39]

Description was changed from

==========
CSS Properties and Values API: Support non-inherited custom properties

This patch adds supports for registering non-inherited properties.
These properties are stored in a new class StyleNonInheritedVariables,
similar to StyleVariableData (now renamed to StyleInheritedVariables).
The new class differs in a couple of ways: it is not RefCounted and
there is no root optimisation (this wouldn't make sense anyway).

This patch fixes a bug in the getter on StyleInheritedVariables for
registered properties where we wouldn't check the root node. This is
covered by the added tests.

Non-inherited custom properties are not yet supported for the devtools
computed styles pane and @apply rules, and todos are added in relevant
places in the code.

BUG=641877
==========

to

==========
CSS Properties and Values API: Support non-inherited custom properties

This patch adds supports for registering non-inherited properties.
These properties are stored in a new class StyleNonInheritedVariables,
similar to StyleVariableData (now renamed to StyleInheritedVariables).
The new class differs in a couple of ways: it is not RefCounted and
there is no root optimisation (this wouldn't make sense anyway).

This patch fixes a bug in the getter on StyleInheritedVariables for
registered properties where we wouldn't check the root node. This is
covered by the added tests.

Non-inherited custom properties are not yet supported for the devtools
computed styles pane and @apply rules, and todos are added in relevant
places in the code.

BUG=641877

Committed: https://crrev.com/864454fb784c52c1202410dc28799c8dc0f9ad0c
Cr-Commit-Position: refs/heads/master@{#421776}
==========

[commit-bot: I haz the power, 2016-09-29 08:03:40]

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