Diff
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (220452 => 220453)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2017-08-09 10:48:51 UTC (rev 220452)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2017-08-09 13:04:39 UTC (rev 220453)
@@ -1,3 +1,17 @@
+2017-08-09 Chris Dumez <cdu...@apple.com>
+
+ Reinstate active flag for iterators
+ https://bugs.webkit.org/show_bug.cgi?id=175312
+
+ Reviewed by Sam Weinig.
+
+ Resync WPT tests from upstream to gain test coverage.
+
+ * web-platform-tests/dom/traversal/NodeIterator-expected.txt:
+ * web-platform-tests/dom/traversal/NodeIterator.html:
+ * web-platform-tests/dom/traversal/TreeWalker-expected.txt:
+ * web-platform-tests/dom/traversal/TreeWalker.html:
+
2017-08-07 Brady Eidson <beid...@apple.com>
Implement most of ServiceWorkerContainer::addRegistration.
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/dom/traversal/NodeIterator-expected.txt (220452 => 220453)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/dom/traversal/NodeIterator-expected.txt 2017-08-09 10:48:51 UTC (rev 220452)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/dom/traversal/NodeIterator-expected.txt 2017-08-09 13:04:39 UTC (rev 220453)
@@ -4,6 +4,7 @@
PASS createNodeIterator() with null as arguments
PASS createNodeIterator() with undefined as arguments
PASS Propagate exception from filter function
+PASS Recursive filters need to throw
PASS document.createNodeIterator(paras[0], 0, null)
PASS document.createNodeIterator(paras[0], 0, (function(node) { return true }))
PASS document.createNodeIterator(paras[0], 0, (function(node) { return false }))
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/dom/traversal/NodeIterator.html (220452 => 220453)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/dom/traversal/NodeIterator.html 2017-08-09 10:48:51 UTC (rev 220452)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/dom/traversal/NodeIterator.html 2017-08-09 13:04:39 UTC (rev 220453)
@@ -52,6 +52,23 @@
assert_throws({name: "failed"}, function() { iter.nextNode() });
}, "Propagate exception from filter function");
+test(function() {
+ var depth = 0;
+ var iter = document.createNodeIterator(document, NodeFilter.SHOW_ALL,
+ function() {
+ if (iter.referenceNode != document && depth == 0) {
+ depth++;
+ iter.nextNode();
+ }
+ return NodeFilter.FILTER_ACCEPT;
+ });
+ iter.nextNode();
+ iter.nextNode();
+ assert_throws("InvalidStateError", function() { iter.nextNode() });
+ depth--;
+ assert_throws("InvalidStateError", function() { iter.previousNode() });
+}, "Recursive filters need to throw");
+
function testIterator(root, whatToShow, filter) {
var iter = document.createNodeIterator(root, whatToShow, filter);
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/dom/traversal/TreeWalker-expected.txt (220452 => 220453)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/dom/traversal/TreeWalker-expected.txt 2017-08-09 10:48:51 UTC (rev 220452)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/dom/traversal/TreeWalker-expected.txt 2017-08-09 13:04:39 UTC (rev 220453)
@@ -1,4 +1,5 @@
+PASS Recursive filters need to throw
PASS document.createTreeWalker(paras[0], 0, null)
PASS document.createTreeWalker(paras[0], 0, (function(node) { return true }))
PASS document.createTreeWalker(paras[0], 0, (function(node) { return false }))
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/dom/traversal/TreeWalker.html (220452 => 220453)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/dom/traversal/TreeWalker.html 2017-08-09 10:48:51 UTC (rev 220452)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/dom/traversal/TreeWalker.html 2017-08-09 13:04:39 UTC (rev 220453)
@@ -11,6 +11,32 @@
// TODO .previousNode, .nextNode
+test(function() {
+ var depth = 0;
+ var walker = document.createTreeWalker(document, NodeFilter.SHOW_ALL,
+ function() {
+ if (depth == 0) {
+ depth++;
+ walker.firstChild();
+ }
+ return NodeFilter.FILTER_ACCEPT;
+ });
+ walker.currentNode = document.body;
+ assert_throws("InvalidStateError", function() { walker.parentNode() });
+ depth--;
+ assert_throws("InvalidStateError", function() { walker.firstChild() });
+ depth--;
+ assert_throws("InvalidStateError", function() { walker.lastChild() });
+ depth--;
+ assert_throws("InvalidStateError", function() { walker.previousSibling() });
+ depth--;
+ assert_throws("InvalidStateError", function() { walker.nextSibling() });
+ depth--;
+ assert_throws("InvalidStateError", function() { walker.previousNode() });
+ depth--;
+ assert_throws("InvalidStateError", function() { walker.nextNode() });
+}, "Recursive filters need to throw");
+
function filterNode(node, whatToShow, filter) {
// "If active flag is set throw an "InvalidStateError"."
// Ignore active flag for these tests, we aren't calling recursively
Modified: trunk/LayoutTests/traversal/moz-bug559526-expected.txt (220452 => 220453)
--- trunk/LayoutTests/traversal/moz-bug559526-expected.txt 2017-08-09 10:48:51 UTC (rev 220452)
+++ trunk/LayoutTests/traversal/moz-bug559526-expected.txt 2017-08-09 13:04:39 UTC (rev 220453)
@@ -1,7 +1,7 @@
-CONSOLE MESSAGE: line 49: Should have thrown an exception: null
-CONSOLE MESSAGE: line 49: Should have thrown an exception: null
-CONSOLE MESSAGE: line 49: Should have thrown an exception: null
-CONSOLE MESSAGE: line 49: Should have thrown an exception: null
+CONSOLE MESSAGE: line 49: Should have thrown an exception: InvalidStateError: Recursive filters are not allowed
+CONSOLE MESSAGE: line 49: Should have thrown an exception: InvalidStateError: Recursive filters are not allowed
+CONSOLE MESSAGE: line 49: Should have thrown an exception: InvalidStateError: Recursive filters are not allowed
+CONSOLE MESSAGE: line 49: Should have thrown an exception: InvalidStateError: Recursive filters are not allowed
CONSOLE MESSAGE: line 94: Should have tests 4 filter calls: 4
Mozilla Bug 559526
Modified: trunk/Source/WebCore/ChangeLog (220452 => 220453)
--- trunk/Source/WebCore/ChangeLog 2017-08-09 10:48:51 UTC (rev 220452)
+++ trunk/Source/WebCore/ChangeLog 2017-08-09 13:04:39 UTC (rev 220453)
@@ -1,3 +1,35 @@
+2017-08-09 Chris Dumez <cdu...@apple.com>
+
+ Reinstate active flag for iterators
+ https://bugs.webkit.org/show_bug.cgi?id=175312
+
+ Reviewed by Sam Weinig.
+
+ NodeIterator / TreeWalker should no longer allow recursive filters
+ after the following change to the DOM specification:
+ - https://github.com/whatwg/dom/pull/359
+
+ This patch aligns our behavior with the latest specification.
+
+ No new tests, updated existing tests.
+
+ * dom/NodeIterator.cpp:
+ (WebCore::NodeIterator::nextNode):
+ (WebCore::NodeIterator::previousNode):
+ Note that we now also call m_candidateNode.clear() before returning an
+ exception. This was a pre-existing bug that we failed to do so in the
+ exception case but it became more obvious after this change now that
+ we throw. This was causing traversal/moz-bug559526.html to fail
+ otherwise (the filter was called one too many times). The test case
+ is passing in Firefox (The filter is called 4 times and they throw
+ each time).
+
+ * dom/Traversal.cpp:
+ (WebCore::NodeIteratorBase::NodeIteratorBase):
+ (WebCore::NodeIteratorBase::acceptNode):
+ * dom/Traversal.h:
+ * dom/TreeWalker.cpp:
+
2017-08-09 Antti Koivisto <an...@apple.com>
RenderQuote should not mutate render tree
Modified: trunk/Source/WebCore/dom/NodeIterator.cpp (220452 => 220453)
--- trunk/Source/WebCore/dom/NodeIterator.cpp 2017-08-09 10:48:51 UTC (rev 220452)
+++ trunk/Source/WebCore/dom/NodeIterator.cpp 2017-08-09 13:04:39 UTC (rev 220453)
@@ -97,13 +97,13 @@
// of the rejected node. Hence, FILTER_REJECT is the same as FILTER_SKIP.
RefPtr<Node> provisionalResult = m_candidateNode.node;
- auto callbackResult = acceptNode(*provisionalResult);
- if (callbackResult.type() == CallbackResultType::ExceptionThrown)
- return Exception { ExistingExceptionError };
+ auto filterResult = acceptNode(*provisionalResult);
+ if (filterResult.hasException()) {
+ m_candidateNode.clear();
+ return filterResult.releaseException();
+ }
- ASSERT(callbackResult.type() == CallbackResultType::Success);
-
- bool nodeWasAccepted = callbackResult.releaseReturnValue() == NodeFilter::FILTER_ACCEPT;
+ bool nodeWasAccepted = filterResult.returnValue() == NodeFilter::FILTER_ACCEPT;
if (nodeWasAccepted) {
m_referenceNode = m_candidateNode;
result = WTFMove(provisionalResult);
@@ -126,13 +126,13 @@
// of the rejected node. Hence, FILTER_REJECT is the same as FILTER_SKIP.
RefPtr<Node> provisionalResult = m_candidateNode.node;
- auto callbackResult = acceptNode(*provisionalResult);
- if (callbackResult.type() == CallbackResultType::ExceptionThrown)
- return Exception { ExistingExceptionError };
+ auto filterResult = acceptNode(*provisionalResult);
+ if (filterResult.hasException()) {
+ m_candidateNode.clear();
+ return filterResult.releaseException();
+ }
- ASSERT(callbackResult.type() == CallbackResultType::Success);
-
- bool nodeWasAccepted = callbackResult.releaseReturnValue() == NodeFilter::FILTER_ACCEPT;
+ bool nodeWasAccepted = filterResult.returnValue() == NodeFilter::FILTER_ACCEPT;
if (nodeWasAccepted) {
m_referenceNode = m_candidateNode;
result = WTFMove(provisionalResult);
Modified: trunk/Source/WebCore/dom/Traversal.cpp (220452 => 220453)
--- trunk/Source/WebCore/dom/Traversal.cpp 2017-08-09 10:48:51 UTC (rev 220452)
+++ trunk/Source/WebCore/dom/Traversal.cpp 2017-08-09 13:04:39 UTC (rev 220453)
@@ -25,28 +25,39 @@
#include "config.h"
#include "Traversal.h"
+#include "CallbackResult.h"
#include "Node.h"
#include "NodeFilter.h"
+#include <wtf/SetForScope.h>
namespace WebCore {
NodeIteratorBase::NodeIteratorBase(Node& rootNode, unsigned whatToShow, RefPtr<NodeFilter>&& nodeFilter)
: m_root(rootNode)
+ , m_filter(WTFMove(nodeFilter))
, m_whatToShow(whatToShow)
- , m_filter(WTFMove(nodeFilter))
{
}
-CallbackResult<unsigned short> NodeIteratorBase::acceptNode(Node& node) const
+// https://dom.spec.whatwg.org/#concept-node-filter
+ExceptionOr<unsigned short> NodeIteratorBase::acceptNode(Node& node)
{
+ if (m_isActive)
+ return Exception { InvalidStateError, ASCIILiteral("Recursive filters are not allowed") };
+
// The bit twiddling here is done to map DOM node types, which are given as integers from
// 1 through 14, to whatToShow bit masks.
if (!(((1 << (node.nodeType() - 1)) & m_whatToShow)))
return NodeFilter::FILTER_SKIP;
+
if (!m_filter)
return NodeFilter::FILTER_ACCEPT;
- return m_filter->acceptNode(node);
+ SetForScope<bool> isActive(m_isActive, true);
+ auto callbackResult = m_filter->acceptNode(node);
+ if (callbackResult.type() == CallbackResultType::ExceptionThrown)
+ return Exception { ExistingExceptionError };
+ return callbackResult.releaseReturnValue();
}
} // namespace WebCore
Modified: trunk/Source/WebCore/dom/Traversal.h (220452 => 220453)
--- trunk/Source/WebCore/dom/Traversal.h 2017-08-09 10:48:51 UTC (rev 220452)
+++ trunk/Source/WebCore/dom/Traversal.h 2017-08-09 13:04:39 UTC (rev 220453)
@@ -24,7 +24,7 @@
#pragma once
-#include "CallbackResult.h"
+#include "ExceptionOr.h"
#include <wtf/RefPtr.h>
namespace WebCore {
@@ -42,12 +42,13 @@
protected:
NodeIteratorBase(Node&, unsigned whatToShow, RefPtr<NodeFilter>&&);
- CallbackResult<unsigned short> acceptNode(Node&) const;
+ ExceptionOr<unsigned short> acceptNode(Node&);
private:
Ref<Node> m_root;
+ RefPtr<NodeFilter> m_filter;
unsigned m_whatToShow;
- RefPtr<NodeFilter> m_filter;
+ bool m_isActive { false };
};
} // namespace WebCore
Modified: trunk/Source/WebCore/dom/TreeWalker.cpp (220452 => 220453)
--- trunk/Source/WebCore/dom/TreeWalker.cpp 2017-08-09 10:48:51 UTC (rev 220452)
+++ trunk/Source/WebCore/dom/TreeWalker.cpp 2017-08-09 13:04:39 UTC (rev 220453)
@@ -55,14 +55,11 @@
if (!node)
return nullptr;
- auto callbackResult = acceptNode(*node);
- if (callbackResult.type() == CallbackResultType::ExceptionThrown)
- return Exception { ExistingExceptionError };
+ auto filterResult = acceptNode(*node);
+ if (filterResult.hasException())
+ return filterResult.releaseException();
- ASSERT(callbackResult.type() == CallbackResultType::Success);
-
- auto acceptNodeResult = callbackResult.releaseReturnValue();
- if (acceptNodeResult == NodeFilter::FILTER_ACCEPT)
+ if (filterResult.returnValue() == NodeFilter::FILTER_ACCEPT)
return setCurrent(node.releaseNonNull());
}
return nullptr;
@@ -71,14 +68,11 @@
ExceptionOr<Node*> TreeWalker::firstChild()
{
for (RefPtr<Node> node = m_current->firstChild(); node; ) {
- auto callbackResult = acceptNode(*node);
- if (callbackResult.type() == CallbackResultType::ExceptionThrown)
- return Exception { ExistingExceptionError };
+ auto filterResult = acceptNode(*node);
+ if (filterResult.hasException())
+ return filterResult.releaseException();
- ASSERT(callbackResult.type() == CallbackResultType::Success);
-
- auto acceptNodeResult = callbackResult.releaseReturnValue();
- switch (acceptNodeResult) {
+ switch (filterResult.returnValue()) {
case NodeFilter::FILTER_ACCEPT:
m_current = node.releaseNonNull();
return m_current.ptr();
@@ -108,14 +102,11 @@
ExceptionOr<Node*> TreeWalker::lastChild()
{
for (RefPtr<Node> node = m_current->lastChild(); node; ) {
- auto callbackResult = acceptNode(*node);
- if (callbackResult.type() == CallbackResultType::ExceptionThrown)
- return Exception { ExistingExceptionError };
+ auto filterResult = acceptNode(*node);
+ if (filterResult.hasException())
+ return filterResult.releaseException();
- ASSERT(callbackResult.type() == CallbackResultType::Success);
-
- auto acceptNodeResult = callbackResult.releaseReturnValue();
- switch (acceptNodeResult) {
+ switch (filterResult.returnValue()) {
case NodeFilter::FILTER_ACCEPT:
m_current = node.releaseNonNull();
return m_current.ptr();
@@ -151,20 +142,17 @@
auto isNext = type == SiblingTraversalType::Next;
while (true) {
for (RefPtr<Node> sibling = isNext ? node->nextSibling() : node->previousSibling(); sibling; ) {
- auto callbackResult = acceptNode(*sibling);
- if (callbackResult.type() == CallbackResultType::ExceptionThrown)
- return Exception { ExistingExceptionError };
+ auto filterResult = acceptNode(*sibling);
+ if (filterResult.hasException())
+ return filterResult.releaseException();
- ASSERT(callbackResult.type() == CallbackResultType::Success);
-
- auto acceptNodeResult = callbackResult.releaseReturnValue();
- if (acceptNodeResult == NodeFilter::FILTER_ACCEPT) {
+ if (filterResult.returnValue() == NodeFilter::FILTER_ACCEPT) {
m_current = sibling.releaseNonNull();
return m_current.ptr();
}
node = sibling;
sibling = isNext ? sibling->firstChild() : sibling->lastChild();
- if (acceptNodeResult == NodeFilter::FILTER_REJECT || !sibling)
+ if (filterResult.returnValue() == NodeFilter::FILTER_REJECT || !sibling)
sibling = isNext ? node->nextSibling() : node->previousSibling();
}
node = node->parentNode();
@@ -171,14 +159,11 @@
if (!node || node == &root())
return nullptr;
- auto callbackResult = acceptNode(*node);
- if (callbackResult.type() == CallbackResultType::ExceptionThrown)
- return Exception { ExistingExceptionError };
+ auto filterResult = acceptNode(*node);
+ if (filterResult.hasException())
+ return filterResult.releaseException();
- ASSERT(callbackResult.type() == CallbackResultType::Success);
-
- auto acceptNodeResult = callbackResult.releaseReturnValue();
- if (acceptNodeResult == NodeFilter::FILTER_ACCEPT)
+ if (filterResult.returnValue() == NodeFilter::FILTER_ACCEPT)
return nullptr;
}
}
@@ -200,25 +185,21 @@
while (Node* previousSibling = node->previousSibling()) {
node = previousSibling;
- auto callbackResult = acceptNode(*node);
- if (callbackResult.type() == CallbackResultType::ExceptionThrown)
- return Exception { ExistingExceptionError };
+ auto filterResult = acceptNode(*node);
+ if (filterResult.hasException())
+ return filterResult.releaseException();
- ASSERT(callbackResult.type() == CallbackResultType::Success);
-
- auto acceptNodeResult = callbackResult.releaseReturnValue();
+ auto acceptNodeResult = filterResult.returnValue();
if (acceptNodeResult == NodeFilter::FILTER_REJECT)
continue;
while (Node* lastChild = node->lastChild()) {
node = lastChild;
- auto callbackResult = acceptNode(*node);
- if (callbackResult.type() == CallbackResultType::ExceptionThrown)
- return Exception { ExistingExceptionError };
+ auto filterResult = acceptNode(*node);
+ if (filterResult.hasException())
+ return filterResult.releaseException();
- ASSERT(callbackResult.type() == CallbackResultType::Success);
-
- acceptNodeResult = callbackResult.releaseReturnValue();
+ acceptNodeResult = filterResult.returnValue();
if (acceptNodeResult == NodeFilter::FILTER_REJECT)
break;
}
@@ -234,14 +215,11 @@
return nullptr;
node = parent;
- auto callbackResult = acceptNode(*node);
- if (callbackResult.type() == CallbackResultType::ExceptionThrown)
- return Exception { ExistingExceptionError };
+ auto filterResult = acceptNode(*node);
+ if (filterResult.hasException())
+ return filterResult.releaseException();
- ASSERT(callbackResult.type() == CallbackResultType::Success);
-
- auto acceptNodeResult = callbackResult.releaseReturnValue();
- if (acceptNodeResult == NodeFilter::FILTER_ACCEPT)
+ if (filterResult.returnValue() == NodeFilter::FILTER_ACCEPT)
return setCurrent(node.releaseNonNull());
}
return nullptr;
@@ -254,31 +232,25 @@
while (Node* firstChild = node->firstChild()) {
node = firstChild;
- auto callbackResult = acceptNode(*node);
- if (callbackResult.type() == CallbackResultType::ExceptionThrown)
- return Exception { ExistingExceptionError };
+ auto filterResult = acceptNode(*node);
+ if (filterResult.hasException())
+ return filterResult.releaseException();
- ASSERT(callbackResult.type() == CallbackResultType::Success);
-
- auto acceptNodeResult = callbackResult.releaseReturnValue();
- if (acceptNodeResult == NodeFilter::FILTER_ACCEPT)
+ if (filterResult.releaseReturnValue() == NodeFilter::FILTER_ACCEPT)
return setCurrent(node.releaseNonNull());
- if (acceptNodeResult == NodeFilter::FILTER_REJECT)
+ if (filterResult.returnValue() == NodeFilter::FILTER_REJECT)
break;
}
while (Node* nextSibling = NodeTraversal::nextSkippingChildren(*node, &root())) {
node = nextSibling;
- auto callbackResult = acceptNode(*node);
- if (callbackResult.type() == CallbackResultType::ExceptionThrown)
- return Exception { ExistingExceptionError };
+ auto filterResult = acceptNode(*node);
+ if (filterResult.hasException())
+ return filterResult.releaseException();
- ASSERT(callbackResult.type() == CallbackResultType::Success);
-
- auto acceptNodeResult = callbackResult.releaseReturnValue();
- if (acceptNodeResult == NodeFilter::FILTER_ACCEPT)
+ if (filterResult.returnValue() == NodeFilter::FILTER_ACCEPT)
return setCurrent(node.releaseNonNull());
- if (acceptNodeResult == NodeFilter::FILTER_SKIP)
+ if (filterResult.returnValue() == NodeFilter::FILTER_SKIP)
goto Children;
}
return nullptr;
