Commit 56d5b293 authored by Yu Han's avatar Yu Han Committed by Commit Bot

Reland "Adds slotAssignmentMode to the shadow root bindings."

This is a reland of 63c4f9f5

The problem with the reverted CL is that a binding file needs updating.
third_party/blink/renderer/bindings/generated_in_core.gni should be
manually updated to point to the newly generated binding classes.
The reason CQ and my local didn't catch this is because the old
generated files v8_shadow_root_slotting(.h|.cc) still exists. But a
clean build will fail.

This CL is the reverted CL plus updates to generated_in_core.gni. See
patch 1 for the failed case. Patch 2 has the update and failed try job
passes.

Original change's description:
> Adds slotAssignmentMode to the shadow root bindings.
>
> Prior to this CL, the previous implementation of imperative slot API uses
> 'slotting' as an optional parameter for Element.attachShadow(). However,
> based on the feedback by JanMiksovsky@ in
> https://github.com/w3c/webcomponents/pull/866, the parameter should be
> changed to 'slotAssignment' to make it align with other members,
> assignNodes(), assignElements(). I agree with this feedback.
>
> This CL updates the following:
> 1. update bindings where the previous 'slotting' was used to
>  'slotAssignment'.
> 2. Updated the enum type of slotAssignment. It should be,
>  SlotAssignmentMode [auto | manual].
> 3. Added attribute 'slotAssignment' to Shadow_Root.idl and updated
>  global-interface-listing-expected.txt.
> 4. Modify existing tests from using slotting to slotAssignment.
> 5. Removed test that'll be covered by
>  external/wpt/shadow-dom/slots-imperative-slot-api.tentative.html.
> https://chromium-review.googlesource.com/c/chromium/src/+/2083742
> I'll update this CL with the correct
>  slots-imperative-slot-api.tentative-expected.txt once that CL lands.
>
> Bug: 869308
> Change-Id: I0fb1a9e11c88d52fc0b0570f3f3e1f5bf834f858
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2084376
> Commit-Queue: Yu Han <yuzhehan@chromium.org>
> Reviewed-by: Mason Freed <masonfreed@chromium.org>
> Reviewed-by: Hayato Ito <hayato@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#748116}

Bug: 869308
Change-Id: I26121ca421d707393afc720c1c5918ff36373f29
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2096784Reviewed-by: default avatarMason Freed <masonfreed@chromium.org>
Reviewed-by: default avatarHayato Ito <hayato@chromium.org>
Reviewed-by: default avatarYuki Shiino <yukishiino@chromium.org>
Commit-Queue: Yu Han <yuzhehan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#749215}
parent ccf21c2c
......@@ -11,7 +11,7 @@ customElements.define("my-summary", MySummaryElement);
customElements.define("my-detail", class extends HTMLElement {
constructor() {
super();
this.attachShadow({ mode: "open", slotting: "manual" });
this.attachShadow({ mode: "open", slotAssignment: "manual" });
}
connectedCallback() {
const target = this;
......
......@@ -4,7 +4,7 @@
</div>
<script>
const host = document.querySelector("#host");
const shadow_root = host.attachShadow({ mode: 'open', slotting: 'auto' });
const shadow_root = host.attachShadow({ mode: 'open', slotAssignment: 'auto' });
const slot1 = document.createElement("slot");
const child1 = document.createElement("child");
shadow_root.appendChild(slot1);
......
......@@ -4,7 +4,7 @@
</div>
<script>
const host = document.querySelector("#host");
const shadow_root = host.attachShadow({ mode: 'open', slotting: 'manual' });
const shadow_root = host.attachShadow({ mode: 'open', slotAssignment: 'manual' });
const slot1 = document.createElement("slot");
const child1 = document.createElement("child");
slot1.assign([child1]);
......
......@@ -4,7 +4,7 @@
</div>
<script>
const host = document.querySelector("#host");
const shadow_root = host.attachShadow({ mode: 'open', slotting: 'manual' });
const shadow_root = host.attachShadow({ mode: 'open', slotAssignment: 'manual' });
const slot1 = document.createElement("slot");
const child1 = document.createElement("child");
shadow_root.appendChild(slot1);
......
......@@ -4,7 +4,7 @@
</div>
<script>
const host = document.querySelector("#host");
const shadow_root = host.attachShadow({ mode: 'open', slotting: 'manual' });
const shadow_root = host.attachShadow({ mode: 'open', slotAssignment: 'manual' });
const slot1 = document.createElement("slot");
const child1 = document.createElement("child");
slot1.assign([child1]);
......
......@@ -4,7 +4,7 @@
</div>
<script>
const host = document.querySelector("#host");
const shadow_root = host.attachShadow({ mode: 'open', slotting: 'manual' });
const shadow_root = host.attachShadow({ mode: 'open', slotAssignment: 'manual' });
const slot1 = document.createElement("slot");
const child1 = document.createElement("child");
slot1.assign([child1]);
......
......@@ -5,7 +5,7 @@
</div>
<script>
const host = document.querySelector("#host");
const shadow_root = host.attachShadow({ mode: 'open', slotting: 'manual' });
const shadow_root = host.attachShadow({ mode: 'open', slotAssignment: 'manual' });
const slot1 = document.createElement("slot");
const child1 = document.createElement("child");
const child = document.querySelector("#child");
......
......@@ -6,7 +6,7 @@
<script>
const host = document.querySelector("#host");
const child1 = document.querySelector("#child1");
const shadow_root = host.attachShadow({ mode: "open", slotting: "manual" });
const shadow_root = host.attachShadow({ mode: "open", slotAssignment: "manual" });
window.onload = function() {
PerfTestRunner.measureTime({
description: "Measure performance of slot assignment in manual-slotting mode in shadow root.",
......
......@@ -107,8 +107,8 @@ generated_enumeration_sources_in_core = [
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_selection_mode.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_shadow_root_mode.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_shadow_root_mode.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_shadow_root_slotting_mode.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_shadow_root_slotting_mode.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_slot_assignment_mode.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_slot_assignment_mode.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_supported_type.cc",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_supported_type.h",
"$root_gen_dir/third_party/blink/renderer/bindings/core/v8/v8_text_track_kind.cc",
......
......@@ -3710,7 +3710,7 @@ ShadowRoot* Element::attachShadow(const ShadowRootInit* shadow_root_init_dict,
DCHECK(!shadow_root_init_dict->hasMode() || !GetShadowRoot());
bool delegates_focus = shadow_root_init_dict->hasDelegatesFocus() &&
shadow_root_init_dict->delegatesFocus();
bool manual_slotting = shadow_root_init_dict->slotting() == "manual";
bool manual_slotting = shadow_root_init_dict->slotAssignment() == "manual";
return &AttachShadowRootInternal(type, delegates_focus, manual_slotting);
}
......@@ -3740,8 +3740,9 @@ ShadowRoot& Element::AttachShadowRootInternal(ShadowRootType type,
GetDocument().SetShadowCascadeOrder(ShadowCascadeOrder::kShadowCascadeV1);
ShadowRoot& shadow_root = CreateAndAttachShadowRoot(type);
shadow_root.SetDelegatesFocus(delegates_focus);
shadow_root.SetSlotting(manual_slotting ? ShadowRootSlotting::kManual
: ShadowRootSlotting::kAuto);
shadow_root.SetSlotAssignmentMode(manual_slotting
? SlotAssignmentMode::kManual
: SlotAssignmentMode::kAuto);
return shadow_root;
}
......
......@@ -70,7 +70,7 @@ ShadowRoot::ShadowRoot(Document& document, ShadowRootType type)
type_(static_cast<unsigned>(type)),
registered_with_parent_shadow_root_(false),
delegates_focus_(false),
slotting_(static_cast<unsigned>(ShadowRootSlotting::kAuto)),
slot_assignment_mode_(static_cast<unsigned>(SlotAssignmentMode::kAuto)),
needs_distribution_recalc_(false),
unused_(0) {
if (IsV0())
......@@ -108,8 +108,8 @@ Node* ShadowRoot::Clone(Document&, CloneChildrenFlag) const {
return nullptr;
}
void ShadowRoot::SetSlotting(ShadowRootSlotting slotting) {
slotting_ = static_cast<unsigned>(slotting);
void ShadowRoot::SetSlotAssignmentMode(SlotAssignmentMode assignment_mode) {
slot_assignment_mode_ = static_cast<unsigned>(assignment_mode);
}
String ShadowRoot::innerHTML() const {
......
......@@ -46,7 +46,7 @@ class WhitespaceAttacher;
enum class ShadowRootType { V0, kOpen, kClosed, kUserAgent };
enum class ShadowRootSlotting { kManual, kAuto };
enum class SlotAssignmentMode { kManual, kAuto };
class CORE_EXPORT ShadowRoot final : public DocumentFragment, public TreeScope {
DEFINE_WRAPPERTYPEINFO();
......@@ -146,9 +146,16 @@ class CORE_EXPORT ShadowRoot final : public DocumentFragment, public TreeScope {
void SetDelegatesFocus(bool flag) { delegates_focus_ = flag; }
bool delegatesFocus() const { return delegates_focus_; }
void SetSlotting(ShadowRootSlotting slotting);
bool IsManualSlotting() {
return slotting_ == static_cast<unsigned>(ShadowRootSlotting::kManual);
void SetSlotAssignmentMode(SlotAssignmentMode assignment);
bool IsManualSlotting() const {
return slot_assignment_mode_ ==
static_cast<unsigned>(SlotAssignmentMode::kManual);
}
SlotAssignmentMode GetSlotAssignmentMode() const {
return static_cast<SlotAssignmentMode>(slot_assignment_mode_);
}
String slotAssignment() const {
return IsManualSlotting() ? "manual" : "auto";
}
bool ContainsShadowRoots() const { return child_shadow_root_count_; }
......@@ -181,7 +188,7 @@ class CORE_EXPORT ShadowRoot final : public DocumentFragment, public TreeScope {
unsigned type_ : 2;
unsigned registered_with_parent_shadow_root_ : 1;
unsigned delegates_focus_ : 1;
unsigned slotting_ : 1;
unsigned slot_assignment_mode_ : 1;
unsigned needs_distribution_recalc_ : 1;
unsigned unused_ : 10;
......
......@@ -34,6 +34,7 @@ interface ShadowRoot : DocumentFragment {
// https://crbug.com/1058762 has been fixed.
[CEReactions, CustomElementCallbacks, RaisesException=Setter] attribute [TreatNullAs=EmptyString, StringContext=TrustedHTML] DOMString innerHTML;
readonly attribute boolean delegatesFocus;
[RuntimeEnabled=ManualSlotting] readonly attribute SlotAssignmentMode slotAssignment;
};
ShadowRoot includes DocumentOrShadowRoot;
......@@ -5,10 +5,10 @@
// Spec: https://w3c.github.io/webcomponents/spec/shadow/#shadowrootinit-dictionary
enum ShadowRootMode { "open", "closed" };
enum ShadowRootSlottingMode { "manual", "auto" };
enum SlotAssignmentMode { "manual", "auto" };
dictionary ShadowRootInit {
required ShadowRootMode mode;
boolean delegatesFocus;
[RuntimeEnabled=ManualSlotting] ShadowRootSlottingMode slotting;
[RuntimeEnabled=ManualSlotting] SlotAssignmentMode slotAssignment;
};
This is a testharness.js-based test.
FAIL attachShadow can take slotAssignment parameter. assert_throws_js: others should throw exception function "() => {
tTree.host3.attachShadow({ mode: 'open', slotAssignment: 'exceptional' })}" did not throw
PASS attachShadow can take slotAssignment parameter.
FAIL Imperative slot API throws exception when not slotAssignment != 'manual'. assert_throws_dom: function "() => { tTree.s1.assign([]); }" did not throw
FAIL Imperative slot API throws exception when slotable parentNode != slot's host. assert_throws_dom: function "() => { tTree.s2.assign([tTree.c1]); }" did not throw
FAIL Imperative slot API can assign nodes in manual slot assignment. assert_equals: expected null but got Element node <slot id="s1"></slot>
......
......@@ -12,7 +12,7 @@ See https://crbug.com/869308
<script>
const host = document.querySelector("#host");
const child1 = document.querySelector("#child1");
const shadow_root = host.attachShadow({ mode: "open", slotting: "manual" });
const shadow_root = host.attachShadow({ mode: "open", slotAssignment: "manual" });
const slot1 = document.createElement("slot");
const slot2 = document.createElement("slot");
shadow_root.appendChild(slot1);
......
......@@ -6,27 +6,11 @@ See https://crbug.com/869308
<script src="../resources/testharness.js"></script>
<script src="../resources/testharnessreport.js"></script>
<script src="resources/shadow-dom.js"></script>
<div id="test1">
<div id="host"></div>
<div id="host1"></div>
<div id="host2"></div>
</div>
<script>
test(() => {
let n = createTestTree(test1);
assert_not_equals(n.host.attachShadow({ mode: 'open', slotting: 'manual' }),
null, 'slotting manual should work');
assert_not_equals(n.host1.attachShadow({ mode: 'open', slotting: 'auto' }),
null, 'slotting auto should work');
assert_throws_js(TypeError, () => {
n.host2.attachShadow({ mode: 'open', slotting: 'exceptional' })},
'others should throw exception');
}, 'attachShadow can take slotting parameter');
</script>
<div id="test2">
<div id="host">
<template id="shadow_root" data-mode="open" data-slotting="manual">
<template id="shadow_root" data-mode="open" data-slot-assignment="manual">
<slot id="s1"></slot>
<slot id="s2"></slot>
<slot id="s3"></slot>
......@@ -37,7 +21,7 @@ test(() => {
</div>
<div id="c4"></div>
<div id="host4">
<template id="shadow_root1" data-mode="open" data-slotting="manual">
<template id="shadow_root1" data-mode="open" data-slot-assignment="manual">
<slot id="s5" name="s5"></slot>
</template>
</div>
......
......@@ -11,7 +11,7 @@ customElements.define("my-summary", MySummaryElement);
customElements.define("my-detail", class extends HTMLElement {
constructor() {
super();
this.attachShadow({ mode: "open", slotting: "manual" });
this.attachShadow({ mode: "open", slotAssignment: "manual" });
}
connectedCallback() {
const target = this;
......
......@@ -76,10 +76,10 @@ function createTestTree(node) {
if (template.getAttribute('data-mode') === 'v0') {
// For legacy Shadow DOM
shadowRoot = parent.createShadowRoot();
} else if (template.getAttribute('data-slotting') === 'manual') {
} else if (template.getAttribute('data-slot-assignment') === 'manual') {
shadowRoot =
parent.attachShadow({mode: template.getAttribute('data-mode'),
slotting: 'manual'});
slotAssignment: 'manual'});
} else {
shadowRoot =
parent.attachShadow({mode: template.getAttribute('data-mode')});
......
......@@ -8030,6 +8030,7 @@ interface ShadowRoot : DocumentFragment
getter mode
getter pictureInPictureElement
getter pointerLockElement
getter slotAssignment
getter styleSheets
method constructor
method elementFromPoint
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment