New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 824684 link

Starred by 8 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature

Blocked on:
issue 392771



Sign in to add a comment

Provide a lightweight mechanism to add styles to a custom element

Project Member Reported by rakina@chromium.org, Mar 22 2018

Issue description

From discussion in https://github.com/w3c/webcomponents/issues/468#issuecomment-370665411, we are providing a way to assign a CSSStyleSheet to a custom element definition.

1) existing API
class MyElement extends HTMLElement {
  constructor() {
    super();
    this.attachShadow({mode: 'open'});
    this.shadowRoot.innerHTML = `<style>:host { color: blue }</style><slot></slot>`;
  }
}
window.customElements.define('my-element', MyElement);

(2) proposed API
var css = new CSSStyleSheet(':host { color: blue}');
class MyElement extends HTMLElement {}
window.customElements.define('my-element', MyElement, css);


Every instance of my-element will behave as if there is a shadow root with |css| as its only stylesheet, but there's no actual shadow roots and the custom element can add its own shadow root and styles to override the default style from |css|.
 

Comment 1 by rakina@chromium.org, Mar 22 2018

Description: Show this description
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 29 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/59284db4e1d974b6a08f6d96b9092f1639b93aaf

commit 59284db4e1d974b6a08f6d96b9092f1639b93aaf
Author: Rakina Zata Amni <rakina@chromium.org>
Date: Thu Mar 29 08:42:44 2018

Add CSSStyleSheet to CustomElementDefinition

This change is the first part of implementation for https://github.com/w3c/webcomponents/issues/468
This change adds an optional CSSStyleSheet in CustomElementOptions in
CustomElementRegistry.define() behind a flag.
The resulting CustomElementDefinition will contain the CSSStyleSheet.
Currently the stylesheet in the definition doesn't affect anything yet.

Intent to implement:
https://groups.google.com/a/chromium.org/d/topic/blink-dev/RQVKGUjDz9U/discussion


Implementation plan doc: https://docs.google.com/document/d/1aA1emBlA20nVB0bmkcamjFdPEvcoBuRbp6p8QKcPz80/edit?usp=sharing

Bug: 824684
Change-Id: I4bf7c05700307d5e8e084c9893afb1779155c017
Reviewed-on: https://chromium-review.googlesource.com/975081
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Hayato Ito <hayato@chromium.org>
Reviewed-by: Takayoshi Kochi <kochi@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546773}
[modify] https://crrev.com/59284db4e1d974b6a08f6d96b9092f1639b93aaf/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinition.cpp
[modify] https://crrev.com/59284db4e1d974b6a08f6d96b9092f1639b93aaf/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinition.h
[modify] https://crrev.com/59284db4e1d974b6a08f6d96b9092f1639b93aaf/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinitionBuilder.cpp
[modify] https://crrev.com/59284db4e1d974b6a08f6d96b9092f1639b93aaf/third_party/WebKit/Source/bindings/core/v8/ScriptCustomElementDefinitionBuilder.h
[modify] https://crrev.com/59284db4e1d974b6a08f6d96b9092f1639b93aaf/third_party/WebKit/Source/core/dom/ElementDefinitionOptions.idl
[modify] https://crrev.com/59284db4e1d974b6a08f6d96b9092f1639b93aaf/third_party/WebKit/Source/core/html/custom/CustomElementDefinition.cpp
[modify] https://crrev.com/59284db4e1d974b6a08f6d96b9092f1639b93aaf/third_party/WebKit/Source/core/html/custom/CustomElementDefinition.h
[modify] https://crrev.com/59284db4e1d974b6a08f6d96b9092f1639b93aaf/third_party/WebKit/Source/core/html/custom/CustomElementDefinitionBuilder.h
[modify] https://crrev.com/59284db4e1d974b6a08f6d96b9092f1639b93aaf/third_party/WebKit/Source/core/html/custom/CustomElementRegistry.cpp
[modify] https://crrev.com/59284db4e1d974b6a08f6d96b9092f1639b93aaf/third_party/WebKit/Source/core/html/custom/CustomElementRegistryTest.cpp
[modify] https://crrev.com/59284db4e1d974b6a08f6d96b9092f1639b93aaf/third_party/WebKit/Source/core/html/custom/CustomElementTestHelpers.cpp
[modify] https://crrev.com/59284db4e1d974b6a08f6d96b9092f1639b93aaf/third_party/WebKit/Source/core/html/custom/CustomElementTestHelpers.h
[modify] https://crrev.com/59284db4e1d974b6a08f6d96b9092f1639b93aaf/third_party/WebKit/Source/platform/runtime_enabled_features.json5

Comment 3 by hayato@chromium.org, Mar 29 2018

Blockedon: 392771

Comment 4 by rakina@chromium.org, May 29 2018

As discussed in https://github.com/w3c/webcomponents/issues/468, we are trying to implement custom element default style in two versions: a "fake shadow root" version, and "User Agent" version. Implementation is on crrev.com/c/992074 and crrev.com/c/1068562.

I've compared them using the attached test based on crbug.com/789729 (open uarunner.html or hostrunner.html using a build of the CLs linked before) and written about it on https://docs.google.com/document/d/1aA1emBlA20nVB0bmkcamjFdPEvcoBuRbp6p8QKcPz80/edit?usp=sharing. Both of them are around the same speed with the UA version somehow a bit slower, but both of them are much faster than just using a shadow root + style element.


CompareHostUA.zip
8.5 KB Download
I've redone the tests, with varying number of elements and done 1000 runs for each case. It seems like my initial impression was wrong - there isn't a significant difference in performance between the UA vs fake-shadow-root version when the number of elements are in the hundreds. When they reach thousands the UA version is a bit slower but not by much, around < 3% slower, so I guess it is because of the difference between ElementRuleCollector::CollectMatchingRules and ElementRuleCollector::CollectMatchingShadowHostRules. They're both still way faster than using shadow root + <style>.

I've updated the doc at 
https://docs.google.com/document/d/1aA1emBlA20nVB0bmkcamjFdPEvcoBuRbp6p8QKcPz80/edit#heading=h.vmv4o980xwad and also linked to the raw data from there.

Cc: momon@google.com
Project Member

Comment 7 by bugdroid1@chromium.org, Sep 13

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/86fc000c6580377ea7904fbcb0906d83becef828

commit 86fc000c6580377ea7904fbcb0906d83becef828
Author: Momoko Sumida <momon@google.com>
Date: Thu Sep 13 02:25:33 2018

Apply custom element default style using simple selectors

When a custom element is created and its definition has a default style,
apply the style as an author style and use simple selectors to match
custom element. This style will only style the element itself. (Note that
style inheritance will work as usual)

<my-element class="el1" class="el2"></my-element>
- The following match this custom element: *, .el1, .el1.el2, my-element
- The following match nothing: div, * > *, * *, * ~ *, foo-element

<my-el class="el1" id="el"><div class="el1" class="el2"></div></my-el>
- The following match this custom element: *, .el1, #el, my-el
- The following match nothing: div, * > *, * *, * ~ *, .el1.el2

Note:
when we implement updates of default style later on, using constructable
style sheets, there will be changes on how default style is passed to
custom element definition (at the moment we pass a copy)

Link to the Design doc(Google internal only):
https://docs.google.com/document/d/1el494btZRK7tBw2h7wIVZe4QgxRI0xNLBJqVHg2ilG8/edit?usp=sharing

Link to the issue:
https://github.com/w3c/webcomponents/issues/468#issuecomment-400905342

Intent to Implement:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/RQVKGUjDz9U/ltwrLQ1HAQAJ

Related CLs:
crrev.com/c/975081
^ Adding CSSStyleSheet to CustomElementDefinition and ElementDefinitionOptions
crrev.com/c/1070830
^ Copy the passed CSSStyleSheet instead of using the same instance
crrev.com/c/1068562
^ Apply custom element default style (UA version)
crrev.com/c/992074
^ Apply custom element default style (:host version)
crrev.com/c/1179440
^ Implement :element selector

Bug: 824684
Change-Id: I712402ccdb94dd1e16d126962738c2c9fe7b03d1
Reviewed-on: https://chromium-review.googlesource.com/1198885
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Commit-Queue: Momoko Sumida <momon@google.com>
Cr-Commit-Position: refs/heads/master@{#590898}
[add] https://crrev.com/86fc000c6580377ea7904fbcb0906d83becef828/third_party/WebKit/LayoutTests/fast/dom/custom/define-with-default-style.html
[modify] https://crrev.com/86fc000c6580377ea7904fbcb0906d83becef828/third_party/blink/renderer/bindings/core/v8/script_custom_element_definition.cc
[modify] https://crrev.com/86fc000c6580377ea7904fbcb0906d83becef828/third_party/blink/renderer/core/css/resolver/style_resolver.cc
[modify] https://crrev.com/86fc000c6580377ea7904fbcb0906d83becef828/third_party/blink/renderer/core/css/style_engine.cc
[modify] https://crrev.com/86fc000c6580377ea7904fbcb0906d83becef828/third_party/blink/renderer/core/css/style_engine.h
[modify] https://crrev.com/86fc000c6580377ea7904fbcb0906d83becef828/third_party/blink/renderer/core/html/custom/custom_element_definition.cc
[modify] https://crrev.com/86fc000c6580377ea7904fbcb0906d83becef828/third_party/blink/renderer/core/html/custom/custom_element_definition.h
[modify] https://crrev.com/86fc000c6580377ea7904fbcb0906d83becef828/third_party/blink/renderer/core/html/custom/custom_element_registry_test.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Sep 21

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/77d1f8feba799fdd804438cd30ef080e54aa893b

commit 77d1f8feba799fdd804438cd30ef080e54aa893b
Author: Momoko Sumida <momon@google.com>
Date: Fri Sep 21 08:08:00 2018

Implement updates of default style

This CL implements updates of default style, by marking elements for
styleRecalc when default style is mutated.
Custom elements are marked for styleRecalc by creating an invalidation
set with custom element tag name.

Link to the Design doc(Google internal only):
https://docs.google.com/document/d/1el494btZRK7tBw2h7wIVZe4QgxRI0xNLBJqVHg2ilG8/edit?usp=sharing

Link to the issue:
https://github.com/w3c/webcomponents/issues/468#issuecomment-400905342

Intent to Implement:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/RQVKGUjDz9U/ltwrLQ1HAQAJ

Link to related CL:
crrev.com/c/1198885
^ Apply custom element default style using simple selectors
crrev.com/c/1220527
^ Restrict the use of CSSStyleSheets to only one Document

Bug: 824684
Change-Id: Ibbd3ab78b4c3a61afe169078d8d45c9a2d684774
Reviewed-on: https://chromium-review.googlesource.com/1224314
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Commit-Queue: Momoko Sumida <momon@google.com>
Cr-Commit-Position: refs/heads/master@{#593112}
[add] https://crrev.com/77d1f8feba799fdd804438cd30ef080e54aa893b/third_party/WebKit/LayoutTests/fast/dom/custom/define-with-default-style.css
[modify] https://crrev.com/77d1f8feba799fdd804438cd30ef080e54aa893b/third_party/WebKit/LayoutTests/fast/dom/custom/define-with-default-style.html
[modify] https://crrev.com/77d1f8feba799fdd804438cd30ef080e54aa893b/third_party/blink/renderer/core/css/css_style_sheet.cc
[modify] https://crrev.com/77d1f8feba799fdd804438cd30ef080e54aa893b/third_party/blink/renderer/core/css/css_style_sheet.h
[modify] https://crrev.com/77d1f8feba799fdd804438cd30ef080e54aa893b/third_party/blink/renderer/core/css/style_engine.cc
[modify] https://crrev.com/77d1f8feba799fdd804438cd30ef080e54aa893b/third_party/blink/renderer/core/css/style_engine.h
[modify] https://crrev.com/77d1f8feba799fdd804438cd30ef080e54aa893b/third_party/blink/renderer/core/html/custom/custom_element_definition.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Sep 21

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1f89dd16297edba18f6fd19a1cc0bfb48564934f

commit 1f89dd16297edba18f6fd19a1cc0bfb48564934f
Author: Momoko Sumida <momon@google.com>
Date: Fri Sep 21 09:03:42 2018

PerfTest for Custom Element Default Style (simple selector version)

Add PerfTest to compare the time it takes to set default style with and
without ShadowRoot

Link to the Design doc(Google internal only):
https://docs.google.com/document/d/1el494btZRK7tBw2h7wIVZe4QgxRI0xNLBJqVHg2ilG8/edit?usp=sharing

Link to the issue:
https://github.com/w3c/webcomponents/issues/468#issuecomment-400905342

Intent to Implement:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/RQVKGUjDz9U/ltwrLQ1HAQAJ

Link to related CL:
crrev.com/c/1198885
^ Apply custom element default style using simple selectors
crrev.com/c/1220527
^ Restrict the use of CSSStyleSheets to only one Document
crrev.com/c/1224314
^ Implement updates of default style

Bug: 824684
Change-Id: Ib721764c86e2c757840d0c4629921a5273dbbfe3
Reviewed-on: https://chromium-review.googlesource.com/1233593
Commit-Queue: Momoko Sumida <momon@google.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Hayato Ito <hayato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593128}
[add] https://crrev.com/1f89dd16297edba18f6fd19a1cc0bfb48564934f/third_party/blink/perf_tests/dom/custom-element-default-style-with-shadow.html
[add] https://crrev.com/1f89dd16297edba18f6fd19a1cc0bfb48564934f/third_party/blink/perf_tests/dom/custom-element-default-style.html

Project Member

Comment 10 by bugdroid1@chromium.org, Oct 2

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/20807fcbbc9b6f4c2937566e9f43ae653ce5c05c

commit 20807fcbbc9b6f4c2937566e9f43ae653ce5c05c
Author: Rakina Zata Amni <rakina@chromium.org>
Date: Tue Oct 02 13:38:57 2018

Move setting of custom element default style outside of constructor

Previously custom element default style is passed to the constructor,
adding a CSSStyleSheet parameter to multiple parts of code. This change
removes that and instead sets the default style after the definition
has been built, in CustomElementRegistry::DefineInternal

Bug: 824684
Change-Id: Ic544561830cced77e946ae43491e6ee989511117
Reviewed-on: https://chromium-review.googlesource.com/1256263
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Hayato Ito <hayato@chromium.org>
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595821}
[modify] https://crrev.com/20807fcbbc9b6f4c2937566e9f43ae653ce5c05c/third_party/blink/renderer/bindings/core/v8/script_custom_element_definition.cc
[modify] https://crrev.com/20807fcbbc9b6f4c2937566e9f43ae653ce5c05c/third_party/blink/renderer/bindings/core/v8/script_custom_element_definition.h
[modify] https://crrev.com/20807fcbbc9b6f4c2937566e9f43ae653ce5c05c/third_party/blink/renderer/bindings/core/v8/script_custom_element_definition_builder.cc
[modify] https://crrev.com/20807fcbbc9b6f4c2937566e9f43ae653ce5c05c/third_party/blink/renderer/bindings/core/v8/script_custom_element_definition_builder.h
[modify] https://crrev.com/20807fcbbc9b6f4c2937566e9f43ae653ce5c05c/third_party/blink/renderer/core/html/custom/custom_element_definition.cc
[modify] https://crrev.com/20807fcbbc9b6f4c2937566e9f43ae653ce5c05c/third_party/blink/renderer/core/html/custom/custom_element_definition.h
[modify] https://crrev.com/20807fcbbc9b6f4c2937566e9f43ae653ce5c05c/third_party/blink/renderer/core/html/custom/custom_element_registry.cc
[modify] https://crrev.com/20807fcbbc9b6f4c2937566e9f43ae653ce5c05c/third_party/blink/renderer/core/html/custom/custom_element_registry_test.cc
[modify] https://crrev.com/20807fcbbc9b6f4c2937566e9f43ae653ce5c05c/third_party/blink/renderer/core/html/custom/custom_element_test_helpers.cc
[modify] https://crrev.com/20807fcbbc9b6f4c2937566e9f43ae653ce5c05c/third_party/blink/renderer/core/html/custom/custom_element_test_helpers.h

Project Member

Comment 11 by bugdroid1@chromium.org, Oct 6

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b062ae3753f451a573e32c85ebffd8df212d19ab

commit b062ae3753f451a573e32c85ebffd8df212d19ab
Author: Rakina Zata Amni <rakina@chromium.org>
Date: Sat Oct 06 10:41:11 2018

Make custom element default style accept array of stylesheets

After developer feedback on ergonomics, having a way to add multiple
stylesheets as the default style for a custom element seems to be
preferred. This CL changes custom element default style to handle
multiple stylesheets.

Bug: 824684
Change-Id: Ie0a130db42425396132ed48237e4b24c34243031
Reviewed-on: https://chromium-review.googlesource.com/c/1257468
Reviewed-by: Hayato Ito <hayato@chromium.org>
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597425}
[modify] https://crrev.com/b062ae3753f451a573e32c85ebffd8df212d19ab/third_party/WebKit/LayoutTests/fast/dom/custom/define-with-default-style.html
[modify] https://crrev.com/b062ae3753f451a573e32c85ebffd8df212d19ab/third_party/blink/renderer/bindings/core/v8/script_custom_element_definition.cc
[modify] https://crrev.com/b062ae3753f451a573e32c85ebffd8df212d19ab/third_party/blink/renderer/core/css/resolver/style_resolver.cc
[modify] https://crrev.com/b062ae3753f451a573e32c85ebffd8df212d19ab/third_party/blink/renderer/core/css/style_engine.cc
[modify] https://crrev.com/b062ae3753f451a573e32c85ebffd8df212d19ab/third_party/blink/renderer/core/css/style_engine.h
[modify] https://crrev.com/b062ae3753f451a573e32c85ebffd8df212d19ab/third_party/blink/renderer/core/dom/element_definition_options.idl
[modify] https://crrev.com/b062ae3753f451a573e32c85ebffd8df212d19ab/third_party/blink/renderer/core/html/custom/custom_element_definition.cc
[modify] https://crrev.com/b062ae3753f451a573e32c85ebffd8df212d19ab/third_party/blink/renderer/core/html/custom/custom_element_definition.h
[modify] https://crrev.com/b062ae3753f451a573e32c85ebffd8df212d19ab/third_party/blink/renderer/core/html/custom/custom_element_registry.cc

Just wanna clarify the microbenchmark result:

Using custom element default style + no shadow root is 70%-80% faster than the old way of using shadow root + style element.

Sign in to add a comment