Allow custom property variant of setProperty to find existing references

Allow custom property variant of setProperty to find existing references
to the property name.

Prior to this patch, multiple calls to inline style setProperty with the
same custom property name cause multiple entries to be created in the
StylePropertySet. This causes inline style getPropertyValue calls to
return the wrong value.

Committed: https://crrev.com/e769dbdd327317bbf03d2275ccf57dc4d2f91d38
Cr-Commit-Position: refs/heads/master@{#376096}

[shans, 2016-02-16 04:38:35]

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

[Timothy Loh, 2016-02-16 23:49:39]

https://codereview.chromium.org/1697233003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/fast/css/variables/multiple-writes-to-inline-style.html
(right):

https://codereview.chromium.org/1697233003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/css/variables/multiple-writes-to-inline-style.html:15:
// assert_equals(getComputedStyle(testElem).getPropertyValue('--foo'),
'second');
uncomment this?

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

https://codereview.chromium.org/1697233003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/StylePropertySet.cpp:464:
ASSERT(customPropertyName != nullAtom);
!x.isNull() maybe

https://codereview.chromium.org/1697233003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/StylePropertySet.cpp:468: } else if
(customPropertyName != nullAtom) {
Isn't this backwards - when we have a regular property, the string will be null?

[shans, 2016-02-17 02:41:35]

https://codereview.chromium.org/1697233003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/fast/css/variables/multiple-writes-to-inline-style.html
(right):

https://codereview.chromium.org/1697233003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/css/variables/multiple-writes-to-inline-style.html:15:
// assert_equals(getComputedStyle(testElem).getPropertyValue('--foo'),
'second');
On 2016/02/16 at 23:49:39, Timothy Loh wrote:
> uncomment this?

It will fail, due to another bug I'm currently working on.

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

https://codereview.chromium.org/1697233003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/StylePropertySet.cpp:464:
ASSERT(customPropertyName != nullAtom);
On 2016/02/16 at 23:49:39, Timothy Loh wrote:
> !x.isNull() maybe

Done.

https://codereview.chromium.org/1697233003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/StylePropertySet.cpp:468: } else if
(customPropertyName != nullAtom) {
On 2016/02/16 at 23:49:39, Timothy Loh wrote:
> Isn't this backwards - when we have a regular property, the string will be
null?

uh .. yeah.

[Timothy Loh, 2016-02-17 23:54:10]

lgtm, but leave out empty BUG= lines

https://codereview.chromium.org/1697233003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/fast/css/variables/multiple-writes-to-inline-style.html
(right):

https://codereview.chromium.org/1697233003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/css/variables/multiple-writes-to-inline-style.html:15:
// assert_equals(getComputedStyle(testElem).getPropertyValue('--foo'),
'second');
On 2016/02/17 02:41:35, shans wrote:
> On 2016/02/16 at 23:49:39, Timothy Loh wrote:
> > uncomment this?
> 
> It will fail, due to another bug I'm currently working on.

Maybe better to uncomment and check in a failing expectation then?

[shans, 2016-02-18 00:02:27]

On 2016/02/17 at 23:54:10, timloh wrote:
> lgtm, but leave out empty BUG= lines
> 
>
https://codereview.chromium.org/1697233003/diff/1/third_party/WebKit/LayoutTe...
> File
third_party/WebKit/LayoutTests/fast/css/variables/multiple-writes-to-inline-style.html
(right):
> 
>
https://codereview.chromium.org/1697233003/diff/1/third_party/WebKit/LayoutTe...
>
third_party/WebKit/LayoutTests/fast/css/variables/multiple-writes-to-inline-style.html:15:
// assert_equals(getComputedStyle(testElem).getPropertyValue('--foo'),
'second');
> On 2016/02/17 02:41:35, shans wrote:
> > On 2016/02/16 at 23:49:39, Timothy Loh wrote:
> > > uncomment this?
> > 
> > It will fail, due to another bug I'm currently working on.
> 
> Maybe better to uncomment and check in a failing expectation then?

I think it's better either to just remove it, or for me to uncomment as part of
fixing the other bug.

[shans, 2016-02-18 01:17:02]

Description was changed from

==========
Allow custom property variant of setProperty to find existing references
to the property name.

Prior to this patch, multiple calls to inline style setProperty with the
same custom property name cause multiple entries to be created in the
StylePropertySet. This causes inline style getPropertyValue calls to
return the wrong value.

BUG=
==========

to

==========
Allow custom property variant of setProperty to find existing references
to the property name.

Prior to this patch, multiple calls to inline style setProperty with the
same custom property name cause multiple entries to be created in the
StylePropertySet. This causes inline style getPropertyValue calls to
return the wrong value.
==========

[shans, 2016-02-18 01:18:14]

The CQ bit was checked by shans@chromium.org

[shans, 2016-02-18 01:18:14]

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

[commit-bot: I haz the power, 2016-02-18 01:18:57]

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

[commit-bot: I haz the power, 2016-02-18 01:36:38]

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

[commit-bot: I haz the power, 2016-02-18 01:36:39]

Try jobs failed on following builders:
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)

[shans, 2016-02-18 04:37:31]

The CQ bit was checked by shans@chromium.org

[commit-bot: I haz the power, 2016-02-18 04:37:54]

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

[commit-bot: I haz the power, 2016-02-18 04:44:21]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-02-18 04:45:26]

Description was changed from

==========
Allow custom property variant of setProperty to find existing references
to the property name.

Prior to this patch, multiple calls to inline style setProperty with the
same custom property name cause multiple entries to be created in the
StylePropertySet. This causes inline style getPropertyValue calls to
return the wrong value.
==========

to

==========
Allow custom property variant of setProperty to find existing references
to the property name.

Prior to this patch, multiple calls to inline style setProperty with the
same custom property name cause multiple entries to be created in the
StylePropertySet. This causes inline style getPropertyValue calls to
return the wrong value.

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

[commit-bot: I haz the power, 2016-02-18 04:45:27]

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