- Revision
- 137731
- Author
- shin...@chromium.org
- Date
- 2012-12-14 01:23:22 -0800 (Fri, 14 Dec 2012)
Log Message
[Shadow DOM] ShadowRoot.getElementById() should work outside document.
https://bugs.webkit.org/show_bug.cgi?id=87815
Reviewed by Hajime Morita.
Source/WebCore:
ShadowRoot.getElementById() didn't work if ShadowRoot is outside document. We need to update id when an element
is in ShadowTree event if it is not in document.
For performance reason, we introduce IsInShadowTree flag, which enables us to check isInTreeScope() fast.
This is maintained in Element::insertedInto and removedFrom. Here, we're anxious about performance regression,
however our benchmark result shows this doesn't regress the performance.
I've measured Dromaeo/dom-modify.html and Parser/html5-full-render.html 2 times.
Dromaeo/dom-modify.html
35.21, 35.27 [runs/s] ---> 35.76, 35.56 [runs/s]
Parser/html5-full-render.html
4328.51, 4254.94 [ms] ---> 4277.14, 4222.43 [ms]
Test: fast/dom/shadow/getelementbyid-in-orphan.html
* dom/Element.cpp:
(WebCore::Element::insertedInto):
* dom/Element.h:
(WebCore::Element::updateId):
* dom/Node.cpp:
(WebCore::Node::insertedInto): If the parent node is in shadow tree, this node should be also in the same shadow tree.
Since this node is inserted, parentOrHostNode() will not be null.
(WebCore::Node::removedFrom): When node is removed from ShadowTree, its treeScope() should not be ShadowRoot.
* dom/Node.h:
(Node):
(WebCore::Node::isInShadowTree):
(WebCore::Node::isInTreeScope):
LayoutTests:
* fast/dom/shadow/getelementbyid-in-orphan-expected.txt: Added.
* fast/dom/shadow/getelementbyid-in-orphan.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (137730 => 137731)
--- trunk/LayoutTests/ChangeLog 2012-12-14 09:08:30 UTC (rev 137730)
+++ trunk/LayoutTests/ChangeLog 2012-12-14 09:23:22 UTC (rev 137731)
@@ -1,3 +1,13 @@
+2012-12-14 Shinya Kawanaka <shin...@chromium.org>
+
+ [Shadow DOM] ShadowRoot.getElementById() should work outside document.
+ https://bugs.webkit.org/show_bug.cgi?id=87815
+
+ Reviewed by Hajime Morita.
+
+ * fast/dom/shadow/getelementbyid-in-orphan-expected.txt: Added.
+ * fast/dom/shadow/getelementbyid-in-orphan.html: Added.
+
2012-12-14 Eugene Klyuchnikov <eus...@chromium.org>
http/tests/inspector/resource-main-cookies.php is broken on Mac after r137585
Added: trunk/LayoutTests/fast/dom/shadow/getelementbyid-in-orphan-expected.txt (0 => 137731)
--- trunk/LayoutTests/fast/dom/shadow/getelementbyid-in-orphan-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/shadow/getelementbyid-in-orphan-expected.txt 2012-12-14 09:23:22 UTC (rev 137731)
@@ -0,0 +1,35 @@
+Tests to ensure ShadowRoot.getElementById works even if a ShadowRoot is orphan.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS shadowRoot.getElementById("child1") is shadowChild1
+PASS shadowRoot.getElementById("child2") is shadowChild2
+PASS shadowRoot.getElementById("child3") is null
+PASS shadowRoot.getElementById("child4") is null
+PASS document.getElementById("child1") is null
+PASS document.getElementById("child2") is null
+PASS document.getElementById("child3") is null
+PASS document.getElementById("child4") is null
+
+Insert elements having the same id
+PASS shadowRoot.getElementById("child2") is shadowChild2
+PASS shadowRoot.getElementById("child2") is shadowChild2_3
+
+Make the host in document
+PASS shadowRoot.getElementById("child1") is shadowChild1
+PASS shadowRoot.getElementById("child2") is shadowChild2_3
+PASS shadowRoot.getElementById("child3") is null
+PASS shadowRoot.getElementById("child4") is null
+PASS document.getElementById("child1") is null
+PASS document.getElementById("child2") is null
+PASS document.getElementById("child3") is child3
+PASS document.getElementById("child4") is child4
+
+Add a child and make the host not in document
+PASS shadowRoot.getElementById("child5") is shadowChild5
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/dom/shadow/getelementbyid-in-orphan.html (0 => 137731)
--- trunk/LayoutTests/fast/dom/shadow/getelementbyid-in-orphan.html (rev 0)
+++ trunk/LayoutTests/fast/dom/shadow/getelementbyid-in-orphan.html 2012-12-14 09:23:22 UTC (rev 137731)
@@ -0,0 +1,84 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<script src=""
+
+<div id="wrapper"></div>
+<pre id="console"></pre>
+
+<script>
+function createDivWithId(id)
+{
+ var div = document.createElement('div');
+ div.id = id;
+
+ return div;
+}
+
+description("Tests to ensure ShadowRoot.getElementById works even if a ShadowRoot is orphan.");
+
+var host = document.createElement('div');
+var shadowRoot = host.webkitCreateShadowRoot();
+
+var shadowChild1 = createDivWithId('child1');
+var shadowChild2 = createDivWithId('child2');
+var hostChild1 = createDivWithId('child3');
+var hostChild2 = createDivWithId('child4');
+
+host.appendChild(hostChild1);
+host.appendChild(hostChild2);
+shadowRoot.appendChild(shadowChild1);
+shadowRoot.appendChild(shadowChild2);
+
+shouldBe('shadowRoot.getElementById("child1")', 'shadowChild1');
+shouldBe('shadowRoot.getElementById("child2")', 'shadowChild2');
+shouldBe('shadowRoot.getElementById("child3")', 'null');
+shouldBe('shadowRoot.getElementById("child4")', 'null');
+
+shouldBe('document.getElementById("child1")', 'null');
+shouldBe('document.getElementById("child2")', 'null');
+shouldBe('document.getElementById("child3")', 'null');
+shouldBe('document.getElementById("child4")', 'null');
+
+debug('');
+debug('Insert elements having the same id');
+
+var shadowChild2_2 = createDivWithId('child2');
+shadowRoot.appendChild(shadowChild2_2);
+
+shouldBe('shadowRoot.getElementById("child2")', 'shadowChild2');
+
+var shadowChild2_3 = createDivWithId('child2');
+shadowRoot.insertBefore(shadowChild2_3, shadowRoot.firstChild);
+
+shouldBe('shadowRoot.getElementById("child2")', 'shadowChild2_3');
+
+debug('');
+debug('Make the host in document');
+wrapper.appendChild(host);
+
+shouldBe('shadowRoot.getElementById("child1")', 'shadowChild1');
+shouldBe('shadowRoot.getElementById("child2")', 'shadowChild2_3');
+shouldBe('shadowRoot.getElementById("child3")', 'null');
+shouldBe('shadowRoot.getElementById("child4")', 'null');
+
+shouldBe('document.getElementById("child1")', 'null');
+shouldBe('document.getElementById("child2")', 'null');
+shouldBe('document.getElementById("child3")', 'child3');
+shouldBe('document.getElementById("child4")', 'child4');
+
+debug('');
+debug('Add a child and make the host not in document');
+var shadowChild5 = createDivWithId('child5');
+shadowRoot.appendChild(shadowChild5);
+wrapper.removeChild(host);
+
+shouldBe('shadowRoot.getElementById("child5")', 'shadowChild5');
+
+debug('');
+wrapper.innerHTML = "";
+</script>
+<script src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (137730 => 137731)
--- trunk/Source/WebCore/ChangeLog 2012-12-14 09:08:30 UTC (rev 137730)
+++ trunk/Source/WebCore/ChangeLog 2012-12-14 09:23:22 UTC (rev 137731)
@@ -1,3 +1,39 @@
+2012-12-14 Shinya Kawanaka <shin...@chromium.org>
+
+ [Shadow DOM] ShadowRoot.getElementById() should work outside document.
+ https://bugs.webkit.org/show_bug.cgi?id=87815
+
+ Reviewed by Hajime Morita.
+
+ ShadowRoot.getElementById() didn't work if ShadowRoot is outside document. We need to update id when an element
+ is in ShadowTree event if it is not in document.
+
+ For performance reason, we introduce IsInShadowTree flag, which enables us to check isInTreeScope() fast.
+ This is maintained in Element::insertedInto and removedFrom. Here, we're anxious about performance regression,
+ however our benchmark result shows this doesn't regress the performance.
+
+ I've measured Dromaeo/dom-modify.html and Parser/html5-full-render.html 2 times.
+
+ Dromaeo/dom-modify.html
+ 35.21, 35.27 [runs/s] ---> 35.76, 35.56 [runs/s]
+ Parser/html5-full-render.html
+ 4328.51, 4254.94 [ms] ---> 4277.14, 4222.43 [ms]
+
+ Test: fast/dom/shadow/getelementbyid-in-orphan.html
+
+ * dom/Element.cpp:
+ (WebCore::Element::insertedInto):
+ * dom/Element.h:
+ (WebCore::Element::updateId):
+ * dom/Node.cpp:
+ (WebCore::Node::insertedInto): If the parent node is in shadow tree, this node should be also in the same shadow tree.
+ Since this node is inserted, parentOrHostNode() will not be null.
+ (WebCore::Node::removedFrom): When node is removed from ShadowTree, its treeScope() should not be ShadowRoot.
+ * dom/Node.h:
+ (Node):
+ (WebCore::Node::isInShadowTree):
+ (WebCore::Node::isInTreeScope):
+
2012-12-14 Antoine Quint <grao...@apple.com>
LayerTreeAgent should only be enabled upon restore if it was previously in the enabled state
Modified: trunk/Source/WebCore/dom/Element.cpp (137730 => 137731)
--- trunk/Source/WebCore/dom/Element.cpp 2012-12-14 09:08:30 UTC (rev 137730)
+++ trunk/Source/WebCore/dom/Element.cpp 2012-12-14 09:23:22 UTC (rev 137731)
@@ -1098,7 +1098,7 @@
setContainsFullScreenElementOnAncestorsCrossingFrameBoundaries(true);
#endif
- if (!insertionPoint->inDocument())
+ if (!insertionPoint->isInTreeScope())
return InsertionDone;
const AtomicString& idValue = getIdAttribute();
Modified: trunk/Source/WebCore/dom/Element.h (137730 => 137731)
--- trunk/Source/WebCore/dom/Element.h 2012-12-14 09:08:30 UTC (rev 137730)
+++ trunk/Source/WebCore/dom/Element.h 2012-12-14 09:23:22 UTC (rev 137731)
@@ -679,7 +679,7 @@
inline void Element::updateId(const AtomicString& oldId, const AtomicString& newId)
{
- if (!inDocument())
+ if (!isInTreeScope())
return;
if (oldId == newId)
@@ -690,7 +690,7 @@
inline void Element::updateId(TreeScope* scope, const AtomicString& oldId, const AtomicString& newId)
{
- ASSERT(inDocument());
+ ASSERT(isInTreeScope());
ASSERT(oldId != newId);
if (!oldId.isEmpty())
Modified: trunk/Source/WebCore/dom/Node.cpp (137730 => 137731)
--- trunk/Source/WebCore/dom/Node.cpp 2012-12-14 09:08:30 UTC (rev 137730)
+++ trunk/Source/WebCore/dom/Node.cpp 2012-12-14 09:23:22 UTC (rev 137731)
@@ -1270,11 +1270,6 @@
return parent && !parent->isShadowRoot() ? parent : 0;
}
-bool Node::isInShadowTree() const
-{
- return treeScope() != document();
-}
-
Element* Node::parentOrHostElement() const
{
ContainerNode* parent = parentOrHostNode();
@@ -2100,6 +2095,8 @@
ASSERT(insertionPoint->inDocument() || isContainerNode());
if (insertionPoint->inDocument())
setFlag(InDocumentFlag);
+ if (parentOrHostNode()->isInShadowTree())
+ setFlag(IsInShadowTreeFlag);
return InsertionDone;
}
@@ -2108,6 +2105,8 @@
ASSERT(insertionPoint->inDocument() || isContainerNode());
if (insertionPoint->inDocument())
clearFlag(InDocumentFlag);
+ if (isInShadowTree() && !treeScope()->rootNode()->isShadowRoot())
+ clearFlag(IsInShadowTreeFlag);
}
void Node::didMoveToNewDocument(Document* oldDocument)
Modified: trunk/Source/WebCore/dom/Node.h (137730 => 137731)
--- trunk/Source/WebCore/dom/Node.h 2012-12-14 09:08:30 UTC (rev 137730)
+++ trunk/Source/WebCore/dom/Node.h 2012-12-14 09:23:22 UTC (rev 137731)
@@ -275,7 +275,7 @@
// Returns 0, a child of ShadowRoot, or a legacy shadow root.
Node* nonBoundaryShadowTreeRootNode();
- bool isInShadowTree() const;
+
// Node's parent, shadow tree host.
ContainerNode* parentOrHostNode() const;
Element* parentOrHostElement() const;
@@ -476,6 +476,8 @@
ASSERT(m_document || !getFlag(InDocumentFlag));
return getFlag(InDocumentFlag);
}
+ bool isInShadowTree() const { return getFlag(IsInShadowTreeFlag); }
+ bool isInTreeScope() const { return getFlag(static_cast<NodeFlags>(InDocumentFlag | IsInShadowTreeFlag)); }
bool isReadOnlyNode() const { return nodeType() == ENTITY_REFERENCE_NODE; }
bool isDocumentTypeNode() const { return nodeType() == DOCUMENT_TYPE_NODE; }
@@ -712,11 +714,12 @@
HasEventTargetDataFlag = 1 << 23,
V8CollectableDuringMinorGCFlag = 1 << 24,
NeedsShadowTreeWalkerFlag = 1 << 25,
+ IsInShadowTreeFlag = 1 << 26,
DefaultNodeFlags = IsParsingChildrenFinishedFlag
};
- // 6 bits remaining
+ // 5 bits remaining
bool getFlag(NodeFlags mask) const { return m_nodeFlags & mask; }
void setFlag(bool f, NodeFlags mask) const { m_nodeFlags = (m_nodeFlags & ~mask) | (-(int32_t)f & mask); }
@@ -730,7 +733,7 @@
CreateContainer = DefaultNodeFlags | IsContainerFlag,
CreateElement = CreateContainer | IsElementFlag,
CreatePseudoElement = CreateElement | InDocumentFlag | NeedsShadowTreeWalkerFlag,
- CreateShadowRoot = CreateContainer | IsDocumentFragmentFlag | NeedsShadowTreeWalkerFlag,
+ CreateShadowRoot = CreateContainer | IsDocumentFragmentFlag | NeedsShadowTreeWalkerFlag | IsInShadowTreeFlag,
CreateDocumentFragment = CreateContainer | IsDocumentFragmentFlag,
CreateStyledElement = CreateElement | IsStyledElementFlag,
CreateHTMLElement = CreateStyledElement | IsHTMLFlag,
