[Typed OM] Implement FilteredComputedStylePropertyMap

[Typed OM] Implement FilteredComputedStylePropertyMap

This is needed for the CSS Paint API, as inside the paint callback only
properties listed as inputProperties should exist in the style map.

FilteredComputedStylePropertyMap takes a list of both the native and
custom properties and accessors will throw if the property isn't in this
list.

BUG=578252
Committed: https://crrev.com/4382d59bb2da192a3ee29f9ae454afe348c4056e
Cr-Commit-Position: refs/heads/master@{#398992}

[ikilpatrick, 2016-06-09 01:49:19]

Description was changed from

==========
first go...

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

to

==========
[Typed OM] Implement FilteredComputedStylePropertyMap


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

[ikilpatrick, 2016-06-09 02:23:09]

Description was changed from

==========
[Typed OM] Implement FilteredComputedStylePropertyMap


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

to

==========
[Typed OM] Implement FilteredComputedStylePropertyMap

This is needed for the CSS Paint API, as inside the paint callback only
properties listed as inputProproperties should exist in the style map.

FilteredComputedStylePropertyMap takes a list of both the native and custom
properties and accessors will throw if the property isn't in this list.

BUG=545318
==========

[ikilpatrick, 2016-06-09 02:23:10]

ikilpatrick@chromium.org changed reviewers:
+ meade@chromium.org, shans@chromium.org, timloh@chromium.org

[ikilpatrick, 2016-06-09 02:28:47]

In a follow up patch I'll actually hook it up to custom paint once Eddy's style
value change is in.

Just added a unit test at the moment to check for sanity.

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

https://codereview.chromium.org/2054633002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/cssom/FilteredComputedStylePropertyMap.cpp:15:
m_nativeProperties.add(nativeProperty);
These two HashSets could also just be vectors. At some point we probably want to
look at on how many properties are in these. HashSet might be overkill.

https://codereview.chromium.org/2054633002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/cssom/FilteredComputedStylePropertyMap.cpp:25:
{
get, has, getAll are basically the same as StylePropertyMap apart from the
filter check. This seemed better than adding virtual checkProperty method. But
up to y'all.

This could also check then delegate but that seemed too expensive.

https://codereview.chromium.org/2054633002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/cssom/StylePropertyMap.h (right):

https://codereview.chromium.org/2054633002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/cssom/StylePropertyMap.h:50: virtual
CSSStyleValueVector getAllInternal(AtomicString customPropertyName) = 0;
Renamed to internal as public virtual hiding.

[shans, 2016-06-09 03:49:05]

Approach LGTM.

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

https://codereview.chromium.org/2054633002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/cssom/FilteredComputedStylePropertyMap.cpp:25:
{
On 2016/06/09 at 02:28:47, ikilpatrick wrote:
> get, has, getAll are basically the same as StylePropertyMap apart from the
filter check. This seemed better than adding virtual checkProperty method. But
up to y'all.
> 
> This could also check then delegate but that seemed too expensive.

This seems fine to me. The check is there so we don't always need to invalidate,
not to keep map size down or anything.

[meade_UTC10, 2016-06-09 04:31:07]

lgtm, everything seems fine to me.

[Timothy Loh, 2016-06-09 07:53:38]

Just to confirm I'm reading the code correctly, this just adds bits of the stuff
needed for supporting custom properties in the existing SPMaps but doesn't add
support, right?

Please wrap patch descriptions to 72 chars and fix the inputProproperties typo
there. Otherwise this lgtm.

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

https://codereview.chromium.org/2054633002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/cssom/FilteredComputedStylePropertyMapTest.cpp:36:
Persistent<FilteredComputedStylePropertyMap> map =
FilteredComputedStylePropertyMap::create(declaration(), nativeProperties,
customProperties);
Filter..Map* instead of Persistent<..>

https://codereview.chromium.org/2054633002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/cssom/FilteredComputedStylePropertyMapTest.cpp:37:
ASSERT_TRUE(map->getProperties().contains("color"));
ASSERT->EXPECT everywhere. I think gtest style is to prefer EXPECT_* and only
use ASSERT_* when the test really shouldn't continue.

[ikilpatrick, 2016-06-09 17:18:37]

Description was changed from

==========
[Typed OM] Implement FilteredComputedStylePropertyMap

This is needed for the CSS Paint API, as inside the paint callback only
properties listed as inputProproperties should exist in the style map.

FilteredComputedStylePropertyMap takes a list of both the native and custom
properties and accessors will throw if the property isn't in this list.

BUG=545318
==========

to

==========
[Typed OM] Implement FilteredComputedStylePropertyMap

This is needed for the CSS Paint API, as inside the paint callback only 
properties listed as inputProperties should exist in the style map.

FilteredComputedStylePropertyMap takes a list of both the native and 
custom properties and accessors will throw if the property isn't in this
list.

BUG=578252
==========

[ikilpatrick, 2016-06-09 17:36:36]

On 2016/06/09 07:53:38, Timothy Loh wrote:
> Just to confirm I'm reading the code correctly, this just adds bits of the
stuff
> needed for supporting custom properties in the existing SPMaps but doesn't add
> support, right?
> 
> Please wrap patch descriptions to 72 chars and fix the inputProproperties typo
> there. Otherwise this lgtm.
> 
>
https://codereview.chromium.org/2054633002/diff/1/third_party/WebKit/Source/c...
> File
>
third_party/WebKit/Source/core/css/cssom/FilteredComputedStylePropertyMapTest.cpp
> (right):
> 
>
https://codereview.chromium.org/2054633002/diff/1/third_party/WebKit/Source/c...
>
third_party/WebKit/Source/core/css/cssom/FilteredComputedStylePropertyMapTest.cpp:36:
> Persistent<FilteredComputedStylePropertyMap> map =
> FilteredComputedStylePropertyMap::create(declaration(), nativeProperties,
> customProperties);
> Filter..Map* instead of Persistent<..>
> 
>
https://codereview.chromium.org/2054633002/diff/1/third_party/WebKit/Source/c...
>
third_party/WebKit/Source/core/css/cssom/FilteredComputedStylePropertyMapTest.cpp:37:
> ASSERT_TRUE(map->getProperties().contains("color"));
> ASSERT->EXPECT everywhere. I think gtest style is to prefer EXPECT_* and only
> use ASSERT_* when the test really shouldn't continue.

Yup thats right. Only FilteredComputedSPMap will support custom properties at
the moment with this patch. But should be fairly easy to add in support for
ComputedSPMap and InlineSPMap.

Done description wrapping.

[ikilpatrick, 2016-06-09 17:36:50]

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

https://codereview.chromium.org/2054633002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/cssom/FilteredComputedStylePropertyMapTest.cpp:36:
Persistent<FilteredComputedStylePropertyMap> map =
FilteredComputedStylePropertyMap::create(declaration(), nativeProperties,
customProperties);
On 2016/06/09 07:53:37, Timothy Loh wrote:
> Filter..Map* instead of Persistent<..>

Done.

https://codereview.chromium.org/2054633002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/cssom/FilteredComputedStylePropertyMapTest.cpp:37:
ASSERT_TRUE(map->getProperties().contains("color"));
On 2016/06/09 07:53:37, Timothy Loh wrote:
> ASSERT->EXPECT everywhere. I think gtest style is to prefer EXPECT_* and only
> use ASSERT_* when the test really shouldn't continue.

Done.

[ikilpatrick, 2016-06-09 18:51:28]

The CQ bit was checked by ikilpatrick@chromium.org

[ikilpatrick, 2016-06-09 18:51:29]

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

[commit-bot: I haz the power, 2016-06-09 18:52:08]

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

[commit-bot: I haz the power, 2016-06-09 20:13:19]

Description was changed from

==========
[Typed OM] Implement FilteredComputedStylePropertyMap

This is needed for the CSS Paint API, as inside the paint callback only 
properties listed as inputProperties should exist in the style map.

FilteredComputedStylePropertyMap takes a list of both the native and 
custom properties and accessors will throw if the property isn't in this
list.

BUG=578252
==========

to

==========
[Typed OM] Implement FilteredComputedStylePropertyMap

This is needed for the CSS Paint API, as inside the paint callback only 
properties listed as inputProperties should exist in the style map.

FilteredComputedStylePropertyMap takes a list of both the native and 
custom properties and accessors will throw if the property isn't in this
list.

BUG=578252
==========

[commit-bot: I haz the power, 2016-06-09 20:13:21]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-09 20:13:24]

CQ bit was unchecked

[commit-bot: I haz the power, 2016-06-09 20:14:46]

Description was changed from

==========
[Typed OM] Implement FilteredComputedStylePropertyMap

This is needed for the CSS Paint API, as inside the paint callback only 
properties listed as inputProperties should exist in the style map.

FilteredComputedStylePropertyMap takes a list of both the native and 
custom properties and accessors will throw if the property isn't in this
list.

BUG=578252
==========

to

==========
[Typed OM] Implement FilteredComputedStylePropertyMap

This is needed for the CSS Paint API, as inside the paint callback only
properties listed as inputProperties should exist in the style map.

FilteredComputedStylePropertyMap takes a list of both the native and
custom properties and accessors will throw if the property isn't in this
list.

BUG=578252

Committed: https://crrev.com/4382d59bb2da192a3ee29f9ae454afe348c4056e
Cr-Commit-Position: refs/heads/master@{#398992}
==========

[commit-bot: I haz the power, 2016-06-09 20:14:47]

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