Disallow setting invalid values for registered properties via CSSOM

Disallow setting invalid values for registered properties via CSSOM

This patch rejects setting invalid values for registered properties via
the CSSStyleDeclaration::setProperty function. The behaviour which we
decided on is that we will retain any values set prior to registration
and compute them as if they were the unset keyword if they are invalid
after registration, but disallow setting such values after registration.

CSS Typed OM is not yet hooked up aside from the filtered computed map
for custom paint, so we don't yet make any changes there.

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

[Timothy Loh, 2017-01-03 05:35:10]

The CQ bit was checked by timloh@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-03 05:35:18]

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, 2017-01-03 07:41:46]

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

[commit-bot: I haz the power, 2017-01-03 07:41:47]

Dry run: This issue passed the CQ dry run.

[Timothy Loh, 2017-01-04 00:40:07]

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

[alancutter (OOO until 2018), 2017-01-04 01:03:42]

https://codereview.chromium.org/2607403002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/parser/CSSParserImpl.cpp (right):

https://codereview.chromium.org/2607403002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/parser/CSSParserImpl.cpp:88: // to validate
but throw away the result.
General comment: A CSSValue cache (with a void* to the registry to invalidate
against) on CSSVariableData would probably be the solution here.

https://codereview.chromium.org/2607403002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/parser/CSSParserImpl.cpp:92: return
MutableStylePropertySet::SetResult{didParse, didChange};
Use braces here.

https://codereview.chromium.org/2607403002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/Document.h (right):

https://codereview.chromium.org/2607403002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/Document.h:1668: mutable
Member<PropertyRegistry> m_propertyRegistry;
Why is this mutable?

[Timothy Loh, 2017-01-04 02:00:59]

https://codereview.chromium.org/2607403002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/parser/CSSParserImpl.cpp (right):

https://codereview.chromium.org/2607403002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/parser/CSSParserImpl.cpp:92: return
MutableStylePropertySet::SetResult{didParse, didChange};
On 2017/01/04 01:03:42, alancutter wrote:
> Use braces here.

sure

https://codereview.chromium.org/2607403002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/Document.h (right):

https://codereview.chromium.org/2607403002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/Document.h:1668: mutable
Member<PropertyRegistry> m_propertyRegistry;
On 2017/01/04 01:03:42, alancutter wrote:
> Why is this mutable?

Oops, I was going to copy the logic in non-const propertyRegistry to the const
one, but then I just const_cast'd and called the non-const. Will remove.

[alancutter (OOO until 2018), 2017-01-04 02:01:17]

lgtm

[haraken, 2017-01-04 02:10:06]

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

[haraken, 2017-01-04 02:10:07]

bindings/ LGTM

[alancutter (OOO until 2018), 2017-01-04 02:22:10]

https://codereview.chromium.org/2607403002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/Document.h (right):

https://codereview.chromium.org/2607403002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/Document.h:1668: mutable
Member<PropertyRegistry> m_propertyRegistry;
On 2017/01/04 at 02:00:59, Timothy Loh wrote:
> On 2017/01/04 01:03:42, alancutter wrote:
> > Why is this mutable?
> 
> Oops, I was going to copy the logic in non-const propertyRegistry to the const
one, but then I just const_cast'd and called the non-const. Will remove.

I don't think mutable is required for your original plan anyway. (:

[Timothy Loh, 2017-01-04 02:25:50]

The CQ bit was checked by timloh@chromium.org

[Timothy Loh, 2017-01-04 02:25:50]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, alancutter@chromium.org
Link to the patchset: https://codereview.chromium.org/2607403002/#ps20001
(title: "fix comments")

[commit-bot: I haz the power, 2017-01-04 02:26:10]

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, 2017-01-04 03:46:56]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1483496750130120,
"parent_rev": "0e1f97f0c86191d22ad57046ded5004f7792c6a9", "commit_rev":
"039ff099ff8a285cb4923d79c8c8073a83694f06"}

[commit-bot: I haz the power, 2017-01-04 03:47:24]

Description was changed from

==========
Disallow setting invalid values for registered properties via CSSOM

This patch rejects setting invalid values for registered properties via
the CSSStyleDeclaration::setProperty function. The behaviour which we
decided on is that we will retain any values set prior to registration
and compute them as if they were the unset keyword if they are invalid
after registration, but disallow setting such values after registration.

CSS Typed OM is not yet hooked up aside from the filtered computed map
for custom paint, so we don't yet make any changes there.

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

to

==========
Disallow setting invalid values for registered properties via CSSOM

This patch rejects setting invalid values for registered properties via
the CSSStyleDeclaration::setProperty function. The behaviour which we
decided on is that we will retain any values set prior to registration
and compute them as if they were the unset keyword if they are invalid
after registration, but disallow setting such values after registration.

CSS Typed OM is not yet hooked up aside from the filtered computed map
for custom paint, so we don't yet make any changes there.

BUG=641877

Review-Url: https://codereview.chromium.org/2607403002
==========

[commit-bot: I haz the power, 2017-01-04 03:47:26]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2017-01-04 03:49:50]

Description was changed from

==========
Disallow setting invalid values for registered properties via CSSOM

This patch rejects setting invalid values for registered properties via
the CSSStyleDeclaration::setProperty function. The behaviour which we
decided on is that we will retain any values set prior to registration
and compute them as if they were the unset keyword if they are invalid
after registration, but disallow setting such values after registration.

CSS Typed OM is not yet hooked up aside from the filtered computed map
for custom paint, so we don't yet make any changes there.

BUG=641877

Review-Url: https://codereview.chromium.org/2607403002
==========

to

==========
Disallow setting invalid values for registered properties via CSSOM

This patch rejects setting invalid values for registered properties via
the CSSStyleDeclaration::setProperty function. The behaviour which we
decided on is that we will retain any values set prior to registration
and compute them as if they were the unset keyword if they are invalid
after registration, but disallow setting such values after registration.

CSS Typed OM is not yet hooked up aside from the filtered computed map
for custom paint, so we don't yet make any changes there.

BUG=641877

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

[commit-bot: I haz the power, 2017-01-04 03:49:53]

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