Support Style Sharing for Shadow DOM V1

Support Style Sharing for Shadow DOM V1

Style sharing has been disabled for Shadow DOM v1 slotted
elements, but should be enabled.

There used to be a bug that ::slotted([attr]) rules did
not properly prevent styles from being shared and falsely
shared.
This was because the ::slotted() rule did not match
those elements which should and they were not rejected for
style sharing.

This CL fixes the bug to enable style sharing in Shadow DOM
v1 slotted elements and implement a function to check
if style can be shared between those elements.

BUG=671319
TEST=LayoutTests/shadow-dom/slotted-pseudo-element-shared-style.html

Review-Url: https://codereview.chromium.org/2613213002
Cr-Commit-Position: refs/heads/master@{#443785}
Committed: https://chromium.googlesource.com/chromium/src/+/92740ea13dcbe5af5afcbb84949d142e82ff6c6d

[kochi, 2017-01-06 07:16:50]

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

[commit-bot: I haz the power, 2017-01-06 07:17:03]

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-06 08:10:18]

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

[commit-bot: I haz the power, 2017-01-06 08:10:19]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[kochi, 2017-01-10 07:33:51]

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

[commit-bot: I haz the power, 2017-01-10 07:34:05]

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-10 09:10:14]

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

[commit-bot: I haz the power, 2017-01-10 09:10:15]

Dry run: This issue passed the CQ dry run.

[kochi, 2017-01-11 08:27:44]

Description was changed from

==========
WIP: Add support for Shadow V1 style sharing.


BUG=671319
==========

to

==========
Support Style Sharing for Shadow DOM V1

Style sharing has been disabled for Shadow DOM v1 slotted
elements, but should be enabled.

There used to be a bug that ::slotted([attr]) rules did
not properly prevent styles from being shared, because
the ::slotted() rule did not match the element and was
not rejected for sharing style, thus falsely shared style
and styled wrong.

This CL fixes the bug to enable style sharing in Shadow DOM
v1 slotted elements and implement a function to check
if style can be shared between those elements.

BUG=671319
==========

[kochi, 2017-01-11 08:27:44]

kochi@chromium.org changed reviewers:
+ rune@opera.com

[kochi, 2017-01-11 08:29:57]

Description was changed from

==========
Support Style Sharing for Shadow DOM V1

Style sharing has been disabled for Shadow DOM v1 slotted
elements, but should be enabled.

There used to be a bug that ::slotted([attr]) rules did
not properly prevent styles from being shared, because
the ::slotted() rule did not match the element and was
not rejected for sharing style, thus falsely shared style
and styled wrong.

This CL fixes the bug to enable style sharing in Shadow DOM
v1 slotted elements and implement a function to check
if style can be shared between those elements.

BUG=671319
==========

to

==========
Support Style Sharing for Shadow DOM V1

Style sharing has been disabled for Shadow DOM v1 slotted
elements, but should be enabled.

There used to be a bug that ::slotted([attr]) rules did
not properly prevent styles from being shared and falsely
shared.
This was because the ::slotted() rule did not match
those elements which should and they were not rejected for
style sharing.

This CL fixes the bug to enable style sharing in Shadow DOM
v1 slotted elements and implement a function to check
if style can be shared between those elements.

BUG=671319
==========

[kochi, 2017-01-11 08:30:24]

Description was changed from

==========
Support Style Sharing for Shadow DOM V1

Style sharing has been disabled for Shadow DOM v1 slotted
elements, but should be enabled.

There used to be a bug that ::slotted([attr]) rules did
not properly prevent styles from being shared and falsely
shared.
This was because the ::slotted() rule did not match
those elements which should and they were not rejected for
style sharing.

This CL fixes the bug to enable style sharing in Shadow DOM
v1 slotted elements and implement a function to check
if style can be shared between those elements.

BUG=671319
==========

to

==========
Support Style Sharing for Shadow DOM V1

Style sharing has been disabled for Shadow DOM v1 slotted
elements, but should be enabled.

There used to be a bug that ::slotted([attr]) rules did
not properly prevent styles from being shared and falsely
shared.
This was because the ::slotted() rule did not match
those elements which should and they were not rejected for
style sharing.

This CL fixes the bug to enable style sharing in Shadow DOM
v1 slotted elements and implement a function to check
if style can be shared between those elements.

BUG=671319
TEST=LayoutTests/shadow-dom/slotted-pseudo-element-shared-style.html
==========

[rune, 2017-01-11 11:34:27]

https://codereview.chromium.org/2613213002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/SelectorChecker.cpp (right):

https://codereview.chromium.org/2613213002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/SelectorChecker.cpp:122: return
context.element->assignedSlot();
Not sure I understand this one. This is fixing a known bug? Test?

https://codereview.chromium.org/2613213002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/resolver/SharedStyleFinder.cpp (right):

https://codereview.chromium.org/2613213002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/resolver/SharedStyleFinder.cpp:205: return
slot1 && slot2 && slot1 == slot2;
The most common case is that slot1 and slot2 are both null. Won't this skip
style sharing for the common case?

[kochi, 2017-01-12 09:23:17]

On 2017/01/11 11:34:27, rune wrote:
>
https://codereview.chromium.org/2613213002/diff/60001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/css/SelectorChecker.cpp (right):
> 
>
https://codereview.chromium.org/2613213002/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/css/SelectorChecker.cpp:122: return
> context.element->assignedSlot();
> Not sure I understand this one. This is fixing a known bug? Test?
> 
>
https://codereview.chromium.org/2613213002/diff/60001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/css/resolver/SharedStyleFinder.cpp
(right):
> 
>
https://codereview.chromium.org/2613213002/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/css/resolver/SharedStyleFinder.cpp:205: return
> slot1 && slot2 && slot1 == slot2;
> The most common case is that slot1 and slot2 are both null. Won't this skip
> style sharing for the common case?

Thanks for your early review!
I'll address the comments and update the CL.

[kochi, 2017-01-13 09:31:32]

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

[commit-bot: I haz the power, 2017-01-13 09:31:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[kochi, 2017-01-13 10:46:45]

PTAL

https://codereview.chromium.org/2613213002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/SelectorChecker.cpp (right):

https://codereview.chromium.org/2613213002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/SelectorChecker.cpp:122: return
context.element->assignedSlot();
On 2017/01/11 11:34:27, rune wrote:
> Not sure I understand this one. This is fixing a known bug? Test?

ElemenetRuleCollector::hasAnyMatchingRules() does scope-less
checking for rules, and SharedStyleFinder uses it for
m_siblingRuleSet and m_uncommonAttributeRuleSet.

A "::slotted([attr])" style rule is put to m_uncommonAttributeRuleSet
but fails to match because of this scope checking.
Therefore such element that should match the rule fails to match,
and falsely shares style with some sibling if exists.

[commit-bot: I haz the power, 2017-01-13 10:51:35]

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

[commit-bot: I haz the power, 2017-01-13 10:51:36]

Dry run: This issue passed the CQ dry run.

[rune, 2017-01-13 11:34:41]

lgtm

[kochi, 2017-01-14 04:06:52]

The CQ bit was checked by kochi@chromium.org

[commit-bot: I haz the power, 2017-01-14 04:07:06]

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-14 04:17:16]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1484366812312840,
"parent_rev": "4f2237ccffefb9642173b872d4a0aafbac57f65e", "commit_rev":
"92740ea13dcbe5af5afcbb84949d142e82ff6c6d"}

[commit-bot: I haz the power, 2017-01-14 04:18:48]

Description was changed from

==========
Support Style Sharing for Shadow DOM V1

Style sharing has been disabled for Shadow DOM v1 slotted
elements, but should be enabled.

There used to be a bug that ::slotted([attr]) rules did
not properly prevent styles from being shared and falsely
shared.
This was because the ::slotted() rule did not match
those elements which should and they were not rejected for
style sharing.

This CL fixes the bug to enable style sharing in Shadow DOM
v1 slotted elements and implement a function to check
if style can be shared between those elements.

BUG=671319
TEST=LayoutTests/shadow-dom/slotted-pseudo-element-shared-style.html
==========

to

==========
Support Style Sharing for Shadow DOM V1

Style sharing has been disabled for Shadow DOM v1 slotted
elements, but should be enabled.

There used to be a bug that ::slotted([attr]) rules did
not properly prevent styles from being shared and falsely
shared.
This was because the ::slotted() rule did not match
those elements which should and they were not rejected for
style sharing.

This CL fixes the bug to enable style sharing in Shadow DOM
v1 slotted elements and implement a function to check
if style can be shared between those elements.

BUG=671319
TEST=LayoutTests/shadow-dom/slotted-pseudo-element-shared-style.html

Review-Url: https://codereview.chromium.org/2613213002
Cr-Commit-Position: refs/heads/master@{#443785}
Committed:
https://chromium.googlesource.com/chromium/src/+/92740ea13dcbe5af5afcbb84949d...
==========

[commit-bot: I haz the power, 2017-01-14 04:18:49]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/92740ea13dcbe5af5afcbb84949d...