Skip to content

Conversation

@rniwa
Copy link
Member

@rniwa rniwa commented Sep 29, 2022

c0bda73

Prototype streaming for declarative shadow DOM
https://bugs.webkit.org/show_bug.cgi?id=245817

Reviewed by Chris Dumez.

Add a very experimental internal settings to make declarative shadow DOM stream by attaching shadow root
when the start tag of a template element is parsed instead of when the end tag is parsed as proposed.

Having this internal setting allow us to analyze the performance characteristics of both approaches.

* Source/WTF/Scripts/Preferences/WebPreferencesInternal.yaml: Added a new internal settings.
We don't use experimental features since this feature needs to be disabled by default in WKTR.

* Source/WebCore/html/HTMLTemplateElement.cpp:
(WebCore::HTMLTemplateElement::fragmentForInsertion const): Added. This is an abstraction around
regular template and template element, which is a parser macro for shadow root.
(WebCore::HTMLTemplateElement::content const): Assert that nobody calls this version unless we're
dealing with a regular template element (i.e. not a declarative shadow DOM).
(WebCore::HTMLTemplateElement::setDeclarativeShadowRoot): Added.
(WebCore::HTMLTemplateElement::attachAsDeclarativeShadowRootIfNeeded): Exit early if we had attached
the shadow root with the start tag.

* Source/WebCore/html/HTMLTemplateElement.h:

* Source/WebCore/html/parser/HTMLConstructionSite.cpp:
(WebCore::insert):
(WebCore::HTMLConstructionSite::insertHTMLTemplateElement): Added. This implements the main logic
of streaming declarative shadow DOM, which attaches shadow root as soon as the start tag is parsed.
(WebCore::HTMLConstructionSite::ownerDocumentForCurrentNode):

* Source/WebCore/html/parser/HTMLConstructionSite.h:

* Source/WebCore/html/parser/HTMLTreeBuilder.cpp:
(WebCore::HTMLTreeBuilder::processTemplateStartTag):

Canonical link: https://commits.webkit.org/255020@main

b2439e8

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ❌ 🛠 🧪 win
✅ 🧪 bindings ✅ 🛠 ios-sim 🛠 mac-debug ✅ 🛠 gtk 💥 🛠 wincairo
✅ 🧪 webkitperl ⏳ 🧪 ios-wk2 ✅ 🛠 mac-AS-debug ✅ 🧪 gtk-wk2
✅ 🧪 api-ios 🧪 api-mac ✅ 🧪 api-gtk
✅ 🛠 🧪 jsc ✅ 🛠 tv ✅ 🧪 mac-wk1 ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ✅ 🧪 mac-wk2 ✅ 🧪 jsc-armv7-tests
✅ 🛠 watch 🧪 mac-AS-debug-wk2 ✅ 🛠 jsc-mips
✅ 🛠 🧪 unsafe-merge ✅ 🛠 watch-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 jsc-mips-tests

@rniwa rniwa requested a review from cdumez as a code owner September 29, 2022 07:35
@rniwa rniwa self-assigned this Sep 29, 2022
@rniwa rniwa added DOM For bugs specific to XML/HTML DOM elements (including parsing). WebKit Nightly Build labels Sep 29, 2022
Copy link
Contributor

@cdumez cdumez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Do we really need the WeakPtr { }?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No needed after all. Will fix.

@rniwa rniwa added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Sep 30, 2022
https://bugs.webkit.org/show_bug.cgi?id=245817

Reviewed by Chris Dumez.

Add a very experimental internal settings to make declarative shadow DOM stream by attaching shadow root
when the start tag of a template element is parsed instead of when the end tag is parsed as proposed.

Having this internal setting allow us to analyze the performance characteristics of both approaches.

* Source/WTF/Scripts/Preferences/WebPreferencesInternal.yaml: Added a new internal settings.
We don't use experimental features since this feature needs to be disabled by default in WKTR.

* Source/WebCore/html/HTMLTemplateElement.cpp:
(WebCore::HTMLTemplateElement::fragmentForInsertion const): Added. This is an abstraction around
regular template and template element, which is a parser macro for shadow root.
(WebCore::HTMLTemplateElement::content const): Assert that nobody calls this version unless we're
dealing with a regular template element (i.e. not a declarative shadow DOM).
(WebCore::HTMLTemplateElement::setDeclarativeShadowRoot): Added.
(WebCore::HTMLTemplateElement::attachAsDeclarativeShadowRootIfNeeded): Exit early if we had attached
the shadow root with the start tag.

* Source/WebCore/html/HTMLTemplateElement.h:

* Source/WebCore/html/parser/HTMLConstructionSite.cpp:
(WebCore::insert):
(WebCore::HTMLConstructionSite::insertHTMLTemplateElement): Added. This implements the main logic
of streaming declarative shadow DOM, which attaches shadow root as soon as the start tag is parsed.
(WebCore::HTMLConstructionSite::ownerDocumentForCurrentNode):

* Source/WebCore/html/parser/HTMLConstructionSite.h:

* Source/WebCore/html/parser/HTMLTreeBuilder.cpp:
(WebCore::HTMLTreeBuilder::processTemplateStartTag):

Canonical link: https://commits.webkit.org/255020@main
@webkit-commit-queue
Copy link
Collaborator

Committed 255020@main (c0bda73): https://commits.webkit.org/255020@main

Reviewed commits have been landed. Closing PR #4822 and removing active labels.

@webkit-early-warning-system webkit-early-warning-system merged commit c0bda73 into WebKit:main Sep 30, 2022
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Sep 30, 2022
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 21, 2022
This follows the discussion on the HTML spec PR [1], and follows the
approach suggested by @rniwa [2]. A template element is still created
and added to the stack of open elements, but the template is *not*
added to the document. And then the template contents are parsed
directly into the open shadow root, so there is no moving of contents
or removal of the template element at the end.

The new behavior is behind the StreamingDeclarativeShadowDOM feature,
which is enabled for experimental only. A virtual suite tests the old
non-streaming behavior.

[1] whatwg/html#5465
[2] WebKit/WebKit#4822

Bug: 1042130
Change-Id: Iab9c9c1a2eb49eaa8f1f7c928f2477977e92b49b
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 28, 2022
A new feature flag, StreamingDeclarativeShadowDOM, guards the new
behavior in this CL. When that flag is enabled, upon seeing the
*opening* `<template>` tag, the parser will attach a shadow root to
the `<template>`'s parent element. All subsequent template content will
then be parsed *directly* into that shadow root. The closing
`</template>` tag is then basically a no-op: there is no more moving
of template content into shadow roots and removal of the `<template>`
element. In this new mode, there is never an actual `<template>`
element placed into the DOM tree. There is a "placeholder" template
that gets added to the parser's stack of open elements, just to keep
track of the parsing mode appropriately.

If the StreamingDeclarativeShadowDOM flag is disabled, the "old"
behavior is retained, of attaching the shadow root at the closing
`</template>` tag. A virtual suite tests this the old non-streaming
behavior.

This approach follows the discussion on the HTML spec PR [1], and
the approach suggested by @rniwa [2].

[1] whatwg/html#5465
[2] WebKit/WebKit#4822

Bug: 1042130
Change-Id: Iab9c9c1a2eb49eaa8f1f7c928f2477977e92b49b
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 28, 2022
A new feature flag, StreamingDeclarativeShadowDOM, guards the new
behavior in this CL. When that flag is enabled, upon seeing the
*opening* `<template>` tag, the parser will attach a shadow root to
the `<template>`'s parent element. All subsequent template content will
then be parsed *directly* into that shadow root. The closing
`</template>` tag is then basically a no-op: there is no more moving
of template content into shadow roots and removal of the `<template>`
element. In this new mode, there is never an actual `<template>`
element placed into the DOM tree. There is a "placeholder" template
that gets added to the parser's stack of open elements, just to keep
track of the parsing mode appropriately.

If the StreamingDeclarativeShadowDOM flag is disabled, the "old"
behavior is retained, of attaching the shadow root at the closing
`</template>` tag. A virtual suite tests this the old non-streaming
behavior.

This approach follows the discussion on the HTML spec PR [1], and
the approach suggested by @rniwa [2].

[1] whatwg/html#5465
[2] WebKit/WebKit#4822

Bug: 1042130,1379513
Change-Id: Iab9c9c1a2eb49eaa8f1f7c928f2477977e92b49b
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 29, 2022
A new feature flag, StreamingDeclarativeShadowDOM, guards the new
behavior in this CL. When that flag is enabled, upon seeing the
*opening* `<template>` tag, the parser will attach a shadow root to
the `<template>`'s parent element. All subsequent template content will
then be parsed *directly* into that shadow root. The closing
`</template>` tag is then basically a no-op: there is no more moving
of template content into shadow roots and removal of the `<template>`
element. In this new mode, there is never an actual `<template>`
element placed into the DOM tree. There is a "placeholder" template
that gets added to the parser's stack of open elements, just to keep
track of the parsing mode appropriately.

If the StreamingDeclarativeShadowDOM flag is disabled, the "old"
behavior is retained, of attaching the shadow root at the closing
`</template>` tag. A virtual suite tests this the old non-streaming
behavior.

This approach follows the discussion on the HTML spec PR [1], and
the approach suggested by @rniwa [2].

[1] whatwg/html#5465
[2] WebKit/WebKit#4822

Bug: 1042130,1379513
Change-Id: Iab9c9c1a2eb49eaa8f1f7c928f2477977e92b49b
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 31, 2022
A new feature flag, StreamingDeclarativeShadowDOM, guards the new
behavior in this CL. When that flag is enabled, upon seeing the
*opening* `<template>` tag, the parser will attach a shadow root to
the `<template>`'s parent element. All subsequent template content will
then be parsed *directly* into that shadow root. The closing
`</template>` tag is then basically a no-op: there is no more moving
of template content into shadow roots and removal of the `<template>`
element. In this new mode, there is never an actual `<template>`
element placed into the DOM tree. There is a "placeholder" template
that gets added to the parser's stack of open elements, just to keep
track of the parsing mode appropriately.

If the StreamingDeclarativeShadowDOM flag is disabled, the "old"
behavior is retained, of attaching the shadow root at the closing
`</template>` tag. A virtual suite tests this the old non-streaming
behavior.

This approach follows the discussion on the HTML spec PR [1], and
the approach suggested by @rniwa [2].

[1] whatwg/html#5465
[2] WebKit/WebKit#4822

Bug: 1042130,1379513
Change-Id: Iab9c9c1a2eb49eaa8f1f7c928f2477977e92b49b
aarongable pushed a commit to chromium/chromium that referenced this pull request Oct 31, 2022
A new feature flag, StreamingDeclarativeShadowDOM, guards the new
behavior in this CL. When that flag is enabled, upon seeing the
*opening* `<template>` tag, the parser will attach a shadow root to
the `<template>`'s parent element. All subsequent template content will
then be parsed *directly* into that shadow root. The closing
`</template>` tag is then basically a no-op: there is no more moving
of template content into shadow roots and removal of the `<template>`
element. In this new mode, there is never an actual `<template>`
element placed into the DOM tree. There is a "placeholder" template
that gets added to the parser's stack of open elements, just to keep
track of the parsing mode appropriately.

If the StreamingDeclarativeShadowDOM flag is disabled, the "old"
behavior is retained, of attaching the shadow root at the closing
`</template>` tag. A virtual suite tests this the old non-streaming
behavior.

This approach follows the discussion on the HTML spec PR [1], and
the approach suggested by @rniwa [2].

[1] whatwg/html#5465
[2] WebKit/WebKit#4822

Bug: 1042130,1379513
Change-Id: Iab9c9c1a2eb49eaa8f1f7c928f2477977e92b49b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3967571
Reviewed-by: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1065551}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 31, 2022
A new feature flag, StreamingDeclarativeShadowDOM, guards the new
behavior in this CL. When that flag is enabled, upon seeing the
*opening* `<template>` tag, the parser will attach a shadow root to
the `<template>`'s parent element. All subsequent template content will
then be parsed *directly* into that shadow root. The closing
`</template>` tag is then basically a no-op: there is no more moving
of template content into shadow roots and removal of the `<template>`
element. In this new mode, there is never an actual `<template>`
element placed into the DOM tree. There is a "placeholder" template
that gets added to the parser's stack of open elements, just to keep
track of the parsing mode appropriately.

If the StreamingDeclarativeShadowDOM flag is disabled, the "old"
behavior is retained, of attaching the shadow root at the closing
`</template>` tag. A virtual suite tests this the old non-streaming
behavior.

This approach follows the discussion on the HTML spec PR [1], and
the approach suggested by @rniwa [2].

[1] whatwg/html#5465
[2] WebKit/WebKit#4822

Bug: 1042130,1379513
Change-Id: Iab9c9c1a2eb49eaa8f1f7c928f2477977e92b49b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3967571
Reviewed-by: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1065551}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 31, 2022
A new feature flag, StreamingDeclarativeShadowDOM, guards the new
behavior in this CL. When that flag is enabled, upon seeing the
*opening* `<template>` tag, the parser will attach a shadow root to
the `<template>`'s parent element. All subsequent template content will
then be parsed *directly* into that shadow root. The closing
`</template>` tag is then basically a no-op: there is no more moving
of template content into shadow roots and removal of the `<template>`
element. In this new mode, there is never an actual `<template>`
element placed into the DOM tree. There is a "placeholder" template
that gets added to the parser's stack of open elements, just to keep
track of the parsing mode appropriately.

If the StreamingDeclarativeShadowDOM flag is disabled, the "old"
behavior is retained, of attaching the shadow root at the closing
`</template>` tag. A virtual suite tests this the old non-streaming
behavior.

This approach follows the discussion on the HTML spec PR [1], and
the approach suggested by @rniwa [2].

[1] whatwg/html#5465
[2] WebKit/WebKit#4822

Bug: 1042130,1379513
Change-Id: Iab9c9c1a2eb49eaa8f1f7c928f2477977e92b49b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3967571
Reviewed-by: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1065551}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Nov 12, 2022
…adow DOM, a=testonly

Automatic update from web-platform-tests
Add support for streaming Declarative Shadow DOM

A new feature flag, StreamingDeclarativeShadowDOM, guards the new
behavior in this CL. When that flag is enabled, upon seeing the
*opening* `<template>` tag, the parser will attach a shadow root to
the `<template>`'s parent element. All subsequent template content will
then be parsed *directly* into that shadow root. The closing
`</template>` tag is then basically a no-op: there is no more moving
of template content into shadow roots and removal of the `<template>`
element. In this new mode, there is never an actual `<template>`
element placed into the DOM tree. There is a "placeholder" template
that gets added to the parser's stack of open elements, just to keep
track of the parsing mode appropriately.

If the StreamingDeclarativeShadowDOM flag is disabled, the "old"
behavior is retained, of attaching the shadow root at the closing
`</template>` tag. A virtual suite tests this the old non-streaming
behavior.

This approach follows the discussion on the HTML spec PR [1], and
the approach suggested by @rniwa [2].

[1] whatwg/html#5465
[2] WebKit/WebKit#4822

Bug: 1042130,1379513
Change-Id: Iab9c9c1a2eb49eaa8f1f7c928f2477977e92b49b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3967571
Reviewed-by: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1065551}

--

wpt-commits: 80c57bdba06c47377a7abd550e3a371f8dd51b9f
wpt-pr: 36597
jamienicol pushed a commit to jamienicol/gecko that referenced this pull request Nov 14, 2022
…adow DOM, a=testonly

Automatic update from web-platform-tests
Add support for streaming Declarative Shadow DOM

A new feature flag, StreamingDeclarativeShadowDOM, guards the new
behavior in this CL. When that flag is enabled, upon seeing the
*opening* `<template>` tag, the parser will attach a shadow root to
the `<template>`'s parent element. All subsequent template content will
then be parsed *directly* into that shadow root. The closing
`</template>` tag is then basically a no-op: there is no more moving
of template content into shadow roots and removal of the `<template>`
element. In this new mode, there is never an actual `<template>`
element placed into the DOM tree. There is a "placeholder" template
that gets added to the parser's stack of open elements, just to keep
track of the parsing mode appropriately.

If the StreamingDeclarativeShadowDOM flag is disabled, the "old"
behavior is retained, of attaching the shadow root at the closing
`</template>` tag. A virtual suite tests this the old non-streaming
behavior.

This approach follows the discussion on the HTML spec PR [1], and
the approach suggested by @rniwa [2].

[1] whatwg/html#5465
[2] WebKit/WebKit#4822

Bug: 1042130,1379513
Change-Id: Iab9c9c1a2eb49eaa8f1f7c928f2477977e92b49b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3967571
Reviewed-by: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1065551}

--

wpt-commits: 80c57bdba06c47377a7abd550e3a371f8dd51b9f
wpt-pr: 36597
@rniwa rniwa deleted the fix245817 branch February 2, 2023 02:24
jwidar pushed a commit to jwidar/LatencyZeroGithub that referenced this pull request Sep 16, 2025
…adow DOM, a=testonly

Automatic update from web-platform-tests
Add support for streaming Declarative Shadow DOM

A new feature flag, StreamingDeclarativeShadowDOM, guards the new
behavior in this CL. When that flag is enabled, upon seeing the
*opening* `<template>` tag, the parser will attach a shadow root to
the `<template>`'s parent element. All subsequent template content will
then be parsed *directly* into that shadow root. The closing
`</template>` tag is then basically a no-op: there is no more moving
of template content into shadow roots and removal of the `<template>`
element. In this new mode, there is never an actual `<template>`
element placed into the DOM tree. There is a "placeholder" template
that gets added to the parser's stack of open elements, just to keep
track of the parsing mode appropriately.

If the StreamingDeclarativeShadowDOM flag is disabled, the "old"
behavior is retained, of attaching the shadow root at the closing
`</template>` tag. A virtual suite tests this the old non-streaming
behavior.

This approach follows the discussion on the HTML spec PR [1], and
the approach suggested by @rniwa [2].

[1] whatwg/html#5465
[2] WebKit/WebKit#4822

Bug: 1042130,1379513
Change-Id: Iab9c9c1a2eb49eaa8f1f7c928f2477977e92b49b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3967571
Reviewed-by: David Baron <dbaron@chromium.org>
Auto-Submit: Mason Freed <masonf@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1065551}

--

wpt-commits: 80c57bdba06c47377a7abd550e3a371f8dd51b9f
wpt-pr: 36597
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

DOM For bugs specific to XML/HTML DOM elements (including parsing).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants