Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(34)

Issue 1489433002: Support the essential part of Shadow DOM v1 (Closed)

Created:
5 years ago by hayato
Modified:
5 years ago
Reviewers:
tkent, kochi
CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-css, blink-reviews-dom_chromium.org, blink-reviews-html_chromium.org, blink-reviews-style_chromium.org, chromium-reviews, dglazkov+blink, eae+blinkwatch, rwlbuis, sof, webcomponents-bugzilla_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Support the essential part of Shadow DOM v1 The essential part which this CL supports are: - V1 shadow trees can be rendered correctly. - Slot elements are no longer dummy elements. It works as the Shadow DOM spec says [1]. - Composed Tree Traversal supports both v0 and v1 distribution. - v0 and v1 shadow trees can co-exist in the same tree of trees New APIs are: - NonDocumentTypeChildNode.assignedSlot - HTMLSlotElement.getAssingedNodes - HTMLSlotElement.getDistributedNodes - Element's slot attribute All new APIs are guarded by `ShadowDOMV1` runtime flag. The following features are not implemented: - Event path for v1 shadow trees. - The unified redistribution across v0 and v1 shadow trees. See [2] for details. - The fallback contents of slot elements. See [3] for details - Distribution algorithms for v1 shadow tree are not optimized yet. See TODOs in this CL for details. - All style related features which are specific to v1, such as `::slotted` pseudo elements. - Closed shadow trees of v1 are not considered yet. I'll add these missing features later as separate patches. [1]: http://w3c.github.io/webcomponents/spec/shadow/ [2]: https://github.com/w3c/webcomponents/blob/gh-pages/proposals/shadow-dom-v1-in-blink.md#user-content-unified-distribution [3]: https://github.com/w3c/webcomponents/issues/317 BUG=531990 Committed: https://crrev.com/071137e656e1ad78c5e8ce506346aa5d2eeef5f9 Cr-Commit-Position: refs/heads/master@{#364654}

Patch Set 1 #

Patch Set 2 : wip #

Patch Set 3 : wip #

Patch Set 4 : wip #

Patch Set 5 : wip #

Patch Set 6 : wip #

Total comments: 18

Patch Set 7 : update tests #

Patch Set 8 : rebase webexposed/ tests. Addressed the comments #

Total comments: 6

Patch Set 9 : Remove one TODO #

Total comments: 20

Patch Set 10 : Added unit tests. Fixed a minor bug in distribution. #

Total comments: 2

Patch Set 11 : Use the 3-line header #

Total comments: 2

Patch Set 12 : Revert Internals.* #

Unified diffs Side-by-side diffs Delta from patch set Stats (+832 lines, -33 lines) Patch
M third_party/WebKit/LayoutTests/fast/dom/shadow/resources/shadow-dom.js View 1 2 1 chunk +16 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/fast/dom/shadow/v1-default-slots.html View 1 2 3 4 5 6 7 8 9 1 chunk +44 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/fast/dom/shadow/v1-slots-1.html View 1 2 3 4 1 chunk +13 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/fast/dom/shadow/v1-slots-1-expected.html View 1 2 3 4 1 chunk +6 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/fast/dom/shadow/v1-slots-2.html View 1 2 3 4 1 chunk +27 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/fast/dom/shadow/v1-slots-2-expected.html View 1 2 3 4 1 chunk +9 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/fast/dom/shadow/v1-slots-api-1.html View 1 2 3 4 5 6 1 chunk +45 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/fast/dom/shadow/v1-slots-api-2.html View 1 2 3 4 5 6 1 chunk +81 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/fast/dom/shadow/v1-slots-dynamic.html View 1 2 3 4 5 6 1 chunk +12 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/fast/dom/shadow/v1-slots-dynamic-expected.html View 1 2 3 4 1 chunk +4 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/fast/dom/shadow/v1-slots-text-nodes.html View 1 2 3 4 5 6 1 chunk +51 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/webexposed/element-instance-property-listing-expected.txt View 1 2 3 4 5 6 7 8 9 5 chunks +6 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt View 1 2 3 4 5 6 7 8 9 10 11 5 chunks +6 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/core.gypi View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/css/resolver/ElementResolveContext.cpp View 1 2 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/dom/Element.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/Element.cpp View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/dom/Element.idl View 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/LayoutTreeBuilder.cpp View 1 2 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/dom/Node.h View 1 2 3 4 5 6 7 8 9 10 11 3 chunks +4 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/Node.cpp View 1 2 3 4 5 6 7 3 chunks +20 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/dom/NodeComputedStyle.h View 1 2 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/dom/NonDocumentTypeChildNode.h View 1 2 1 chunk +5 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/NonDocumentTypeChildNode.idl View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/shadow/ComposedTreeTraversal.h View 1 2 3 chunks +9 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/shadow/ComposedTreeTraversal.cpp View 1 2 3 4 5 6 7 8 5 chunks +101 lines, -14 lines 0 comments Download
M third_party/WebKit/Source/core/dom/shadow/ComposedTreeTraversalTest.cpp View 1 2 3 4 5 6 7 8 9 4 chunks +117 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/shadow/DistributedNodes.cpp View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/dom/shadow/ElementShadow.h View 1 2 3 4 5 6 7 6 chunks +26 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp View 1 2 3 4 5 2 chunks +7 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/dom/shadow/InsertionPoint.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/shadow/InsertionPoint.cpp View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/shadow/ShadowRoot.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -0 lines 0 comments Download
A third_party/WebKit/Source/core/dom/shadow/SlotAssignment.h View 1 2 3 4 5 6 7 8 9 10 1 chunk +31 lines, -0 lines 0 comments Download
A third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp View 1 2 3 4 5 6 7 8 9 1 chunk +94 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/editing/Position.cpp View 1 2 2 chunks +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLAttributeNames.in View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLSlotElement.h View 1 2 3 4 5 6 7 8 9 1 chunk +23 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLSlotElement.cpp View 1 2 3 4 5 6 7 8 9 1 chunk +55 lines, -5 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLSlotElement.idl View 1 chunk +2 lines, -0 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 100 (49 generated)
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1489433002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1489433002/1
5 years ago (2015-11-30 10:46:22 UTC) #2
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_rel/builds/36022)
5 years ago (2015-11-30 10:58:30 UTC) #4
hayato
wip
5 years ago (2015-12-01 06:16:29 UTC) #7
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1489433002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1489433002/20001
5 years ago (2015-12-01 06:17:12 UTC) #8
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/142021)
5 years ago (2015-12-01 07:26:18 UTC) #10
hayato
wip
5 years ago (2015-12-04 12:09:33 UTC) #14
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1489433002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1489433002/40001
5 years ago (2015-12-04 12:09:42 UTC) #15
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/150039)
5 years ago (2015-12-04 13:38:06 UTC) #20
hayato
wip
5 years ago (2015-12-08 09:33:11 UTC) #22
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1489433002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1489433002/60001
5 years ago (2015-12-08 09:33:29 UTC) #23
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: chromeos_amd64-generic_chromium_compile_only_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-generic_chromium_compile_only_ng/builds/67218) linux_chromium_chromeos_ozone_rel_ng on ...
5 years ago (2015-12-08 10:03:33 UTC) #25
hayato
wip
5 years ago (2015-12-08 11:30:34 UTC) #33
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1489433002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1489433002/80001
5 years ago (2015-12-08 11:30:40 UTC) #34
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_ozone_rel_ng/builds/98555)
5 years ago (2015-12-08 12:08:54 UTC) #36
hayato
wip
5 years ago (2015-12-09 06:45:40 UTC) #38
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1489433002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1489433002/100001
5 years ago (2015-12-09 06:45:43 UTC) #39
hayato
PTAL
5 years ago (2015-12-09 06:57:41 UTC) #43
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: linux_android_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_android_rel_ng/builds/107544)
5 years ago (2015-12-09 08:06:41 UTC) #48
hayato
update tests
5 years ago (2015-12-09 08:13:08 UTC) #50
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1489433002/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1489433002/120001
5 years ago (2015-12-09 08:13:36 UTC) #51
kochi
First round of comments. I haven't yet understood why new methods in ComposedTreeTraversal is required ...
5 years ago (2015-12-09 08:57:34 UTC) #52
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/146829)
5 years ago (2015-12-09 09:35:26 UTC) #54
hayato
rebase webexposed/ tests. Addressed the comments
5 years ago (2015-12-09 10:22:05 UTC) #56
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1489433002/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1489433002/140001
5 years ago (2015-12-09 10:22:19 UTC) #57
hayato
Thank you for the review. > I haven't yet understood why new methods in ComposedTreeTraversal ...
5 years ago (2015-12-09 10:25:40 UTC) #58
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years ago (2015-12-09 11:58:42 UTC) #60
kochi
Second round. I haven't looked into layout tests. https://codereview.chromium.org/1489433002/diff/140001/third_party/WebKit/Source/core/dom/Node.cpp File third_party/WebKit/Source/core/dom/Node.cpp (right): https://codereview.chromium.org/1489433002/diff/140001/third_party/WebKit/Source/core/dom/Node.cpp#newcode2219 third_party/WebKit/Source/core/dom/Node.cpp:2219: if ...
5 years ago (2015-12-09 12:02:27 UTC) #61
hayato
Remove one TODO
5 years ago (2015-12-10 02:16:39 UTC) #65
hayato
Thank you for the review. https://codereview.chromium.org/1489433002/diff/140001/third_party/WebKit/Source/core/dom/Node.cpp File third_party/WebKit/Source/core/dom/Node.cpp (right): https://codereview.chromium.org/1489433002/diff/140001/third_party/WebKit/Source/core/dom/Node.cpp#newcode2219 third_party/WebKit/Source/core/dom/Node.cpp:2219: if (shadow->isV1()) On 2015/12/09 ...
5 years ago (2015-12-10 02:17:31 UTC) #66
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1489433002/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1489433002/160001
5 years ago (2015-12-10 02:17:40 UTC) #67
tkent
https://codereview.chromium.org/1489433002/diff/160001/third_party/WebKit/LayoutTests/fast/dom/shadow/v1-composed-tree-traversal-1.html File third_party/WebKit/LayoutTests/fast/dom/shadow/v1-composed-tree-traversal-1.html (right): https://codereview.chromium.org/1489433002/diff/160001/third_party/WebKit/LayoutTests/fast/dom/shadow/v1-composed-tree-traversal-1.html#newcode27 third_party/WebKit/LayoutTests/fast/dom/shadow/v1-composed-tree-traversal-1.html:27: assert_equals(internals.nextInComposedTree(child1), child2); C++ unit test is more suitable because ...
5 years ago (2015-12-10 04:23:02 UTC) #68
kochi
Reviewed layout tests. https://codereview.chromium.org/1489433002/diff/160001/third_party/WebKit/LayoutTests/fast/dom/shadow/v1-slots-api-1.html File third_party/WebKit/LayoutTests/fast/dom/shadow/v1-slots-api-1.html (right): https://codereview.chromium.org/1489433002/diff/160001/third_party/WebKit/LayoutTests/fast/dom/shadow/v1-slots-api-1.html#newcode45 third_party/WebKit/LayoutTests/fast/dom/shadow/v1-slots-api-1.html:45: </script> Could you also add a ...
5 years ago (2015-12-10 05:57:23 UTC) #69
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years ago (2015-12-10 07:35:25 UTC) #71
hayato
Added unit tests. Fixed a minor bug in distribution.
5 years ago (2015-12-10 08:10:26 UTC) #73
hayato
Thank you for the reviews. https://codereview.chromium.org/1489433002/diff/160001/third_party/WebKit/LayoutTests/fast/dom/shadow/v1-composed-tree-traversal-1.html File third_party/WebKit/LayoutTests/fast/dom/shadow/v1-composed-tree-traversal-1.html (right): https://codereview.chromium.org/1489433002/diff/160001/third_party/WebKit/LayoutTests/fast/dom/shadow/v1-composed-tree-traversal-1.html#newcode27 third_party/WebKit/LayoutTests/fast/dom/shadow/v1-composed-tree-traversal-1.html:27: assert_equals(internals.nextInComposedTree(child1), child2); On 2015/12/10 ...
5 years ago (2015-12-10 08:11:21 UTC) #74
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1489433002/180001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1489433002/180001
5 years ago (2015-12-10 08:11:33 UTC) #75
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years ago (2015-12-10 11:51:11 UTC) #77
tkent
https://codereview.chromium.org/1489433002/diff/180001/third_party/WebKit/Source/core/dom/shadow/SlotAssignment.h File third_party/WebKit/Source/core/dom/shadow/SlotAssignment.h (right): https://codereview.chromium.org/1489433002/diff/180001/third_party/WebKit/Source/core/dom/shadow/SlotAssignment.h#newcode1 third_party/WebKit/Source/core/dom/shadow/SlotAssignment.h:1: /* Use the 3-line header.
5 years ago (2015-12-11 01:00:47 UTC) #78
hayato
Use the 3-line header
5 years ago (2015-12-11 02:04:36 UTC) #80
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1489433002/200001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1489433002/200001
5 years ago (2015-12-11 02:05:36 UTC) #81
hayato
https://codereview.chromium.org/1489433002/diff/180001/third_party/WebKit/Source/core/dom/shadow/SlotAssignment.h File third_party/WebKit/Source/core/dom/shadow/SlotAssignment.h (right): https://codereview.chromium.org/1489433002/diff/180001/third_party/WebKit/Source/core/dom/shadow/SlotAssignment.h#newcode1 third_party/WebKit/Source/core/dom/shadow/SlotAssignment.h:1: /* On 2015/12/11 01:00:47, tkent wrote: > Use the ...
5 years ago (2015-12-11 02:10:29 UTC) #82
kochi
lgtm Thanks for clarifying the CL description for missing parts for V1.
5 years ago (2015-12-11 04:46:08 UTC) #83
tkent
lgtm https://codereview.chromium.org/1489433002/diff/200001/third_party/WebKit/Source/core/testing/Internals.idl File third_party/WebKit/Source/core/testing/Internals.idl (right): https://codereview.chromium.org/1489433002/diff/200001/third_party/WebKit/Source/core/testing/Internals.idl#newcode90 third_party/WebKit/Source/core/testing/Internals.idl:90: [RaisesException] Node parentInComposedTree(Node node); Is this used now?
5 years ago (2015-12-11 04:49:37 UTC) #84
hayato
https://codereview.chromium.org/1489433002/diff/200001/third_party/WebKit/Source/core/testing/Internals.idl File third_party/WebKit/Source/core/testing/Internals.idl (right): https://codereview.chromium.org/1489433002/diff/200001/third_party/WebKit/Source/core/testing/Internals.idl#newcode90 third_party/WebKit/Source/core/testing/Internals.idl:90: [RaisesException] Node parentInComposedTree(Node node); On 2015/12/11 04:49:37, tkent wrote: ...
5 years ago (2015-12-11 04:51:08 UTC) #85
hayato
Revert Internals.*
5 years ago (2015-12-11 04:55:51 UTC) #87
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1489433002/220001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1489433002/220001
5 years ago (2015-12-11 04:56:44 UTC) #88
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1489433002/220001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1489433002/220001
5 years ago (2015-12-11 04:58:13 UTC) #92
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/148374)
5 years ago (2015-12-11 08:12:49 UTC) #94
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1489433002/220001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1489433002/220001
5 years ago (2015-12-11 08:13:57 UTC) #96
commit-bot: I haz the power
Committed patchset #12 (id:220001)
5 years ago (2015-12-11 10:24:57 UTC) #98
commit-bot: I haz the power
5 years ago (2015-12-11 10:26:00 UTC) #100
Message was sent while issue was closed.
Patchset 12 (id:??) landed as
https://crrev.com/071137e656e1ad78c5e8ce506346aa5d2eeef5f9
Cr-Commit-Position: refs/heads/master@{#364654}

Powered by Google App Engine
This is Rietveld 408576698