Re: Prohibit createShadowRoot({mode: 'open'}) on existing shadow root. (issue 1223473004 by kochi@chromium.org)

[ko...@chromium.org]

Reviewers: hayato,

Message:
PTAL

Description:
Prohibit createShadowRoot({mode: 'open'}) on existing shadow root.

This implements Shadow DOM spec 8.2.2-2.
https://w3c.github.io/webcomponents/spec/shadow/#widl-Element-createShadowRoot-ShadowRoot-ShadowRootInit-shadowRootInitDict
> createShadowRoot
...
> 2. If the context object already hosts the shadow tree, throws
InvalidStateError exception.

Calling createShadowRoot() without init dictionary to create multple shadow
roots are
still allowed for compatibility.

BUG=459136
TEST=fast/dom/shadow/create-shadow-root-with-parameter.html

Please review this at https://codereview.chromium.org/1223473004/

Base URL: svn://svn.chromium.org/blink/trunk

Affected files (+16, -4 lines):
M LayoutTests/fast/dom/shadow/create-shadow-root-with-parameter.html
M
LayoutTests/fast/dom/shadow/create-shadow-root-with-parameter-expected.txt
M Source/core/dom/Element.cpp


Index:
LayoutTests/fast/dom/shadow/create-shadow-root-with-parameter-expected.txt
diff --git
a/LayoutTests/fast/dom/shadow/create-shadow-root-with-parameter-expected.txt
b/LayoutTests/fast/dom/shadow/create-shadow-root-with-parameter-expected.txt
index
051111769a68c495c8f5f7ef695c12fd0b8c5feb..64613a08ffbe1fca146112102e36a9536ac4a9f3
100644
---
a/LayoutTests/fast/dom/shadow/create-shadow-root-with-parameter-expected.txt
+++
b/LayoutTests/fast/dom/shadow/create-shadow-root-with-parameter-expected.txt
@@ -8,6 +8,8 @@ Create shadow root whose mode is neither open nor closed.
PASS host4.createShadowRoot({mode: 'illegal'}) threw exception TypeError:
Failed to execute 'createShadowRoot' on 'Element': The provided
value 'illegal' is not a valid enum value of type ShadowRootMode..
Create open shadow root with shadow-dom.js utility.
PASS [object ShadowRoot] is non-null.
+Create shadow root on already shadowed host will raise InvalidStateError
exception.
+PASS host1.createShadowRoot({mode: 'open'}) threw exception
InvalidStateError: Failed to execute 'createShadowRoot' on 'Element':
Shadow root cannot be created on a host which already hosts a shadow tree..
PASS successfullyParsed is true

TEST COMPLETE
Index: LayoutTests/fast/dom/shadow/create-shadow-root-with-parameter.html
diff --git
a/LayoutTests/fast/dom/shadow/create-shadow-root-with-parameter.html
b/LayoutTests/fast/dom/shadow/create-shadow-root-with-parameter.html
index
c1d50fcdd29bf04018522aa42e30e7761200e849..3568176b453976586780199cbf5e6ed0eb04c248
100644
--- a/LayoutTests/fast/dom/shadow/create-shadow-root-with-parameter.html
+++ b/LayoutTests/fast/dom/shadow/create-shadow-root-with-parameter.html
@@ -37,5 +37,8 @@ document.body.appendChild(
var host5 = document.querySelector('#host5');
var root5 = host5.shadowRoot;
shouldBeNonNull(root5);
+
+debug('Create shadow root on already shadowed host will raise
InvalidStateError exception.');
+shouldThrow("host1.createShadowRoot({mode: 'open'})");
</script>
</html>
Index: Source/core/dom/Element.cpp
diff --git a/Source/core/dom/Element.cpp b/Source/core/dom/Element.cpp
index
85e3cac75909a1c28968bc646f0ac8497a31723a..f1636c5f76c05f3f2850626f0ddef7971be0aa33
100644
--- a/Source/core/dom/Element.cpp
+++ b/Source/core/dom/Element.cpp
@@ -1828,10 +1828,17 @@ PassRefPtrWillBeRawPtr<ShadowRoot>
Element::createShadowRoot(const ScriptState*
UseCounter::count(document(),
UseCounter::ElementCreateShadowRootWithParameter);

OriginsUsingFeatures::count(scriptState, document(),
OriginsUsingFeatures::Feature::ElementCreateShadowRoot);
- // TODO(kochi): Add support for closed shadow root. crbug.com/459136
- if (shadowRootInitDict.hasMode() && shadowRootInitDict.mode()
== "closed") {
- exceptionState.throwDOMException(NotSupportedError, "Closed shadow
root is not implemented yet.");
- return nullptr;
+
+ if (shadowRootInitDict.hasMode()) {
+ if (shadowRoot()) {
+ exceptionState.throwDOMException(InvalidStateError, "Shadow
root cannot be created on a host which already hosts a shadow tree.");
+ return nullptr;
+ }
+ // TODO(kochi): Add support for closed shadow root.
crbug.com/459136
+ if (shadowRootInitDict.mode() == "closed") {
+ exceptionState.throwDOMException(NotSupportedError, "Closed
shadow root is not implemented yet.");
+ return nullptr;
+ }
}

RefPtrWillBeRawPtr<ShadowRoot> shadowRoot =
createShadowRoot(exceptionState);

[hay...@chromium.org]

lgtm



https://codereview.chromium.org/1223473004/

[commit-bot@chromium.org via codereview.chromium.org]

CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/patch-status/1223473004/1


https://codereview.chromium.org/1223473004/

[commit-bot@chromium.org via codereview.chromium.org]

Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=198388

https://codereview.chromium.org/1223473004/