[Typed CSSOM] Get computed values for custom properties

[Typed CSSOM]Get computed values for registered custom properties

Previously StylePropertyMap::get used to throw for custom properties. That exception has been moved into InlineStylePropertyMap::getAllInternal(AtomicString customPropertyName ...), and StylePropertMap::get now works for ComputedStylePropertyMap.

I've simplified StylePropertyMap::get & StylePropertyMap::has somewhat. StylePropertyMap::getAll is now the only StylePropertyMap method that directly calls getAllInternal.

Relative URLs do not work for custom properties, so the layout test has to work around that. Bug for the issue is here https://bugs.chromium.org/p/chromium/issues/detail?id=618165

Computed style for custom properties that are URL images seems to have null values for all its attributes, but a CSS property (e.g. background-image) that is specified as var(<that custom property>) has proper values for its attributes in the computed style value. See LayoutTests/typedcssom/computedstyle/custom-properties.html, test case t2. I don't know if that is a bug or not.

BUG=648495

[rjwright, 2017-02-24 02:37:49]

Description was changed from

==========
[Typed CSSOM][WIP] Get computed values for custom properties
BUG=648495
==========

to

==========
[Typed CSSOM]Get computed values for registered custom properties

Previously StylePropertyMap::get used to throw for custom properties. That
exception has been moved into
InlineStylePropertyMap::getAllInternal(AtomicString customPropertyName ...), and
StylePropertMap::get now works for ComputedStylePropertyMap.

I've simplified StylePropertyMap::get & StylePropertyMap::has somewhat.
StylePropertyMap::getAll is now the only StylePropertyMap method that directly
calls getAllInternal.

Relative URLs do not work for custom properties, so the layout test has to work
around that. Bug for the issue is here
https://bugs.chromium.org/p/chromium/issues/detail?id=618165

Computed style for custom properties that are URL images seems to have null
values for all its attributes, but a CSS property (e.g. background-image) that
is specified as var(<that custom property>) has proper values for its attributes
in the computed style value. See
LayoutTests/typedcssom/computedstyle/custom-properties.html, test case t2. I
don't know if that is a bug or not.

BUG=648495
==========

[rjwright, 2017-02-24 02:37:49]

rjwright@chromium.org changed reviewers:
+ alancutter@chromium.org, meade@chromium.org

[rjwright, 2017-02-24 03:07:27]

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

[commit-bot: I haz the power, 2017-02-24 03:07:52]

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-02-24 04:46:50]

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

[commit-bot: I haz the power, 2017-02-24 04:46:51]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[alancutter (OOO until 2018), 2017-02-24 05:11:48]

https://codereview.chromium.org/2695093007/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/typedcssom/computedstyle/custom-properties.html
(right):

https://codereview.chromium.org/2695093007/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/typedcssom/computedstyle/custom-properties.html:75:
}
FYI you can avoid the incrementing variable name with:
async_test(function(t) {
  t.step_and_done(function() {
    assert_equals(actual, expected);
  });
}, 'description');

[alancutter (OOO until 2018), 2017-02-24 05:13:06]

https://codereview.chromium.org/2695093007/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/typedcssom/computedstyle/custom-properties.html
(right):

https://codereview.chromium.org/2695093007/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/typedcssom/computedstyle/custom-properties.html:75:
}
On 2017/02/24 at 05:11:48, alancutter wrote:
> FYI you can avoid the incrementing variable name with:
> async_test(function(t) {
>   t.step_and_done(function() {
>     assert_equals(actual, expected);
>   });
> }, 'description');

Oops my bad. Got to the bottom and saw the necessity. (:

[alancutter (OOO until 2018), 2017-02-24 05:57:21]

https://codereview.chromium.org/2695093007/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/typedcssom/computedstyle/custom-properties.html
(right):

https://codereview.chromium.org/2695093007/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/typedcssom/computedstyle/custom-properties.html:88:
// assert_equals(result.state, "loaded");
Leave these asserts in and check in the failing test expectations.

https://codereview.chromium.org/2695093007/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/typedcssom/computedstyle/custom-properties.html:167:
assert_equals(result[0].cssText, '100px');
Add instanceof check.

https://codereview.chromium.org/2695093007/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/typedcssom/computedstyle/custom-properties.html:175:
var childComputedStyleMap = getComputedStyleMap(childTestElement);
It shouldn't be necessary to wait for the document to be ready. Calls to
getComputedStyle() force recalcs, I presume the same would apply for
getComputedStyleMap(). These tests can be synchronous and you can make just the
background-image test async by itself.

https://codereview.chromium.org/2695093007/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.cpp
(right):

https://codereview.chromium.org/2695093007/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.cpp:90: const
ComputedStyle* style = this->updateStyle();
No need for "this->" where it's not ambiguous. Style guide doesn't say anything
about it but I would avoid it for consistency with the rest of the code base.

https://codereview.chromium.org/2695093007/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.cpp:114:
break;
FYI there's a LengthPropertyFunctions class with a "static bool
getLength(CSSPropertyID, const ComputedStyle&, Length& result);" method that
could save a bit of repetition here.

https://codereview.chromium.org/2695093007/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.cpp:139:
propertyID, *style, nullptr, this->node(), false);
I don't think it's necessary to pass in the node here, if I'm wrong then there
should be a TODO to remove the dependency. Everything you need should be on the
ComputedStyle.

https://codereview.chromium.org/2695093007/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.cpp:160:
m_node->document().propertyRegistry()->registration(customPropertyName);
The property registry is nullptr in stable Chrome.

https://codereview.chromium.org/2695093007/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/cssom/ComputedStylePropertyMap.cpp:167:
return StyleValueFactory::cssValueToStyleValueVector(CSSPropertyInvalid,
No need to pass in CSSPropertyInvalid here, there's a single argument overload
of this function.

[rjwright, 2017-02-27 05:37:03]

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

[commit-bot: I haz the power, 2017-02-27 05:37:11]

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-02-27 07:06:17]

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

[commit-bot: I haz the power, 2017-02-27 07:06:18]

Dry run: This issue passed the CQ dry run.

[meade_UTC10, 2017-03-06 03:16:04]

Is this CL deprecated in favour of the other one you were working on?

[rjwright, 2017-03-16 03:55:47]

On 2017/03/06 03:16:04, Eddy wrote:
> Is this CL deprecated in favour of the other one you were working on?

Yes, the implementation will be a merger of this code and the code you sent me.

[meade_UTC10, 2017-03-16 05:27:36]

meade@chromium.org changed reviewers:
- alancutter@chromium.org, meade@chromium.org