Reviewed by Darin Adler.
authorap@webkit.org <ap@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 1 Jun 2009 08:27:25 +0000 (08:27 +0000)
committerap@webkit.org <ap@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 1 Jun 2009 08:27:25 +0000 (08:27 +0000)
        https://bugs.webkit.org/show_bug.cgi?id=12471
        XPathResult not invalidated for documents retrieved via XMLHttpRequest

        Test: fast/xpath/detached-subtree-invalidate-iterator.html and existing tests in dom/svg/level3/xpath.

        Use DOM tree version instead of DOMSubtreeModified events to invalidate, which is more
        reliable and much faster.

        * xml/XPathExpression.cpp:
        (WebCore::XPathExpression::evaluate):
        * xml/XPathResult.cpp:
        (WebCore::XPathResult::XPathResult):
        (WebCore::XPathResult::~XPathResult):
        (WebCore::XPathResult::invalidIteratorState):
        (WebCore::XPathResult::iterateNext):
        * xml/XPathResult.h:
        (WebCore::XPathResult::create):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@44317 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/dom/svg/level3/xpath/XPathResult_invalidIteratorState_ANY_TYPE-expected.txt
LayoutTests/dom/svg/level3/xpath/XPathResult_invalidIteratorState_ORDERED_NODE_ITERATOR_TYPE-expected.txt
LayoutTests/dom/svg/level3/xpath/XPathResult_invalidIteratorState_UNORDERED_NODE_ITERATOR_TYPE-expected.txt
LayoutTests/dom/svg/level3/xpath/XPathResult_iterateNext_INVALID_STATE_ERR-expected.txt
LayoutTests/fast/xpath/detached-subtree-invalidate-iterator-expected.txt [new file with mode: 0644]
LayoutTests/fast/xpath/detached-subtree-invalidate-iterator.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/xml/XPathExpression.cpp
WebCore/xml/XPathResult.cpp
WebCore/xml/XPathResult.h

index fd7d353..a840c7d 100644 (file)
@@ -1,3 +1,19 @@
+2009-06-01  Alexey Proskuryakov  <ap@webkit.org>
+
+        Reviewed by Darin Adler.
+
+        https://bugs.webkit.org/show_bug.cgi?id=12471
+        XPathResult not invalidated for documents retrieved via XMLHttpRequest
+
+        * dom/svg/level3/xpath/XPathResult_invalidIteratorState_ANY_TYPE-expected.txt:
+        * dom/svg/level3/xpath/XPathResult_invalidIteratorState_ORDERED_NODE_ITERATOR_TYPE-expected.txt:
+        * dom/svg/level3/xpath/XPathResult_invalidIteratorState_UNORDERED_NODE_ITERATOR_TYPE-expected.txt:
+        * dom/svg/level3/xpath/XPathResult_iterateNext_INVALID_STATE_ERR-expected.txt:
+        These tests now pass.
+
+        * fast/xpath/detached-subtree-invalidate-iterator-expected.txt: Added.
+        * fast/xpath/detached-subtree-invalidate-iterator.html: Added.
+
 2009-06-01  Brett Wilson  <brettw@chromium.org>
 
         Reviewed by Darin Adler.  Landed by Adam Barth.
diff --git a/LayoutTests/fast/xpath/detached-subtree-invalidate-iterator-expected.txt b/LayoutTests/fast/xpath/detached-subtree-invalidate-iterator-expected.txt
new file mode 100644 (file)
index 0000000..4b4af52
--- /dev/null
@@ -0,0 +1,11 @@
+Test that iterators are invalidated even if the original context is detached.
+
+PASS result.invalidIteratorState is false
+PASS result.iterateNext().tagName is 'span'
+Modifying the document...
+PASS result.invalidIteratorState is true
+PASS result.iterateNext() threw exception Error: INVALID_STATE_ERR: DOM Exception 11.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/xpath/detached-subtree-invalidate-iterator.html b/LayoutTests/fast/xpath/detached-subtree-invalidate-iterator.html
new file mode 100644 (file)
index 0000000..33673eb
--- /dev/null
@@ -0,0 +1,31 @@
+<html>
+<head>
+<link rel="stylesheet" href="../js/resources/js-test-style.css">
+<script src="../js/resources/js-test-pre.js"></script>
+<script src="xpath-test-pre.js"></script>
+</head>
+<body>
+<p>Test that iterators are invalidated even if the original context is detached.</p>
+<div id="console"></div>
+<script>
+var doc = document.implementation.createDocument(null, "doc", null);
+var root = doc.createElement("div");
+root.appendChild(doc.createElement("span"));
+root.appendChild(doc.createElement("p"));
+
+var result = doc.evaluate(".//*", root, null, XPathResult.ORDERED_NODE_ITERATOR_TYPE, null);
+shouldBe("result.invalidIteratorState", "false");
+shouldBe("result.iterateNext().tagName", "'span'");
+
+debug("Modifying the document...");
+doc.documentElement.setAttribute("foo", "bar");
+
+shouldBe("result.invalidIteratorState", "true");
+shouldThrow("result.iterateNext()");
+
+var successfullyParsed = true;
+
+</script>
+<script src="../js/resources/js-test-post.js"></script>
+</body>
+</html>
index ffd6901..b79e296 100644 (file)
@@ -1,3 +1,25 @@
+2009-06-01  Alexey Proskuryakov  <ap@webkit.org>
+
+        Reviewed by Darin Adler.
+
+        https://bugs.webkit.org/show_bug.cgi?id=12471
+        XPathResult not invalidated for documents retrieved via XMLHttpRequest
+
+        Test: fast/xpath/detached-subtree-invalidate-iterator.html and existing tests in dom/svg/level3/xpath.
+
+        Use DOM tree version instead of DOMSubtreeModified events to invalidate, which is more
+        reliable and much faster.
+
+        * xml/XPathExpression.cpp:
+        (WebCore::XPathExpression::evaluate):
+        * xml/XPathResult.cpp:
+        (WebCore::XPathResult::XPathResult):
+        (WebCore::XPathResult::~XPathResult):
+        (WebCore::XPathResult::invalidIteratorState):
+        (WebCore::XPathResult::iterateNext):
+        * xml/XPathResult.h:
+        (WebCore::XPathResult::create):
+
 2009-06-01  Brett Wilson  <brettw@chromium.org>
 
         Reviewed by Darin Adler.  Landed by Adam Barth.
index ecec79e..a5b7f9b 100644 (file)
@@ -1,6 +1,6 @@
 /*
- * Copyright 2005 Frerich Raabe <raabe@kde.org>
- * Copyright (C) 2006 Apple Computer, Inc.
+ * Copyright (C) 2005 Frerich Raabe <raabe@kde.org>
+ * Copyright (C) 2006, 2009 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -67,13 +67,11 @@ PassRefPtr<XPathResult> XPathExpression::evaluate(Node* contextNode, unsigned sh
         return 0;
     }
 
-    Node* eventTarget = contextNode->ownerDocument() ? contextNode->ownerDocument() : contextNode;
-
     EvaluationContext& evaluationContext = Expression::evaluationContext();
     evaluationContext.node = contextNode;
     evaluationContext.size = 1;
     evaluationContext.position = 1;
-    RefPtr<XPathResult> result = XPathResult::create(eventTarget, m_topExpression->evaluate());
+    RefPtr<XPathResult> result = XPathResult::create(contextNode->document(), m_topExpression->evaluate());
     evaluationContext.node = 0; // Do not hold a reference to the context node, as this may prevent the whole document from being destroyed in time.
 
     if (type != XPathResult::ANY_TYPE) {
index a1c8a08..b608280 100644 (file)
@@ -1,6 +1,6 @@
 /*
- * Copyright 2005 Frerich Raabe <raabe@kde.org>
- * Copyright (C) 2006 Apple Computer, Inc.
+ * Copyright (C) 2005 Frerich Raabe <raabe@kde.org>
+ * Copyright (C) 2006, 2009 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -29,8 +29,7 @@
 
 #if ENABLE(XPATH)
 
-#include "EventListener.h"
-#include "EventNames.h"
+#include "Document.h"
 #include "Node.h"
 #include "ExceptionCode.h"
 #include "XPathEvaluator.h"
@@ -40,22 +39,9 @@ namespace WebCore {
 
 using namespace XPath;
 
-class InvalidatingEventListener : public EventListener {
-public:
-    static PassRefPtr<InvalidatingEventListener> create(XPathResult* result) { return adoptRef(new InvalidatingEventListener(result)); }
-    virtual void handleEvent(Event*, bool) { m_result->invalidateIteratorState(); }
-
-private:
-    InvalidatingEventListener(XPathResult* result) : m_result(result) { }
-    XPathResult* m_result;
-};
-
-XPathResult::XPathResult(Node* eventTarget, const Value& value)
+XPathResult::XPathResult(Document* document, const Value& value)
     : m_value(value)
-    , m_eventTarget(eventTarget)
 {
-    m_eventListener = InvalidatingEventListener::create(this);
-    m_eventTarget->addEventListener(eventNames().DOMSubtreeModifiedEvent, m_eventListener, false);
     switch (m_value.type()) {
         case Value::BooleanValue:
             m_resultType = BOOLEAN_TYPE;
@@ -70,7 +56,8 @@ XPathResult::XPathResult(Node* eventTarget, const Value& value)
             m_resultType = UNORDERED_NODE_ITERATOR_TYPE;
             m_nodeSetPosition = 0;
             m_nodeSet = m_value.toNodeSet();
-            m_invalidIteratorState = false;
+            m_document = document;
+            m_domTreeVersion = document->domTreeVersion();
             return;
     }
     ASSERT_NOT_REACHED();
@@ -78,8 +65,6 @@ XPathResult::XPathResult(Node* eventTarget, const Value& value)
 
 XPathResult::~XPathResult()
 {
-    if (m_eventTarget)
-        m_eventTarget->removeEventListener(eventNames().DOMSubtreeModifiedEvent, m_eventListener.get(), false);
 }
 
 void XPathResult::convertTo(unsigned short type, ExceptionCode& ec)
@@ -174,24 +159,13 @@ Node* XPathResult::singleNodeValue(ExceptionCode& ec) const
         return nodes.anyNode();
 }
 
-void XPathResult::invalidateIteratorState()
-{ 
-    m_invalidIteratorState = true;
-    
-    ASSERT(m_eventTarget);
-    ASSERT(m_eventListener);
-    
-    m_eventTarget->removeEventListener(eventNames().DOMSubtreeModifiedEvent, m_eventListener.get(), false);
-    
-    m_eventTarget = 0;
-}
-
 bool XPathResult::invalidIteratorState() const
 {
     if (resultType() != UNORDERED_NODE_ITERATOR_TYPE && resultType() != ORDERED_NODE_ITERATOR_TYPE)
         return false;
-    
-    return m_invalidIteratorState;
+
+    ASSERT(m_document);
+    return m_document->domTreeVersion() != m_domTreeVersion;
 }
 
 unsigned long XPathResult::snapshotLength(ExceptionCode& ec) const
@@ -211,7 +185,7 @@ Node* XPathResult::iterateNext(ExceptionCode& ec)
         return 0;
     }
     
-    if (m_invalidIteratorState) {
+    if (invalidIteratorState()) {
         ec = INVALID_STATE_ERR;
         return 0;
     }
index 03accc6..3b91d66 100644 (file)
@@ -1,6 +1,6 @@
 /*
- * Copyright 2005 Frerich Raabe <raabe@kde.org>
- * Copyright (C) 2006 Apple Computer, Inc.
+ * Copyright (C) 2005 Frerich Raabe <raabe@kde.org>
+ * Copyright (C) 2006, 2009 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
 
 #if ENABLE(XPATH)
 
-#include <wtf/RefCounted.h>
 #include "XPathValue.h"
+#include <wtf/RefCounted.h>
 
 namespace WebCore {
 
     typedef int ExceptionCode;
 
-    class EventListener;
-    class Node;
+    class Document;
     class Node;
     class String;
 
@@ -56,7 +55,7 @@ namespace WebCore {
             FIRST_ORDERED_NODE_TYPE = 9
         };
         
-        static PassRefPtr<XPathResult> create(Node* eventTarget, const XPath::Value& value) { return adoptRef(new XPathResult(eventTarget, value)); }
+        static PassRefPtr<XPathResult> create(Document* document, const XPath::Value& value) { return adoptRef(new XPathResult(document, value)); }
         ~XPathResult();
         
         void convertTo(unsigned short type, ExceptionCode&);
@@ -73,21 +72,18 @@ namespace WebCore {
         Node* iterateNext(ExceptionCode&);
         Node* snapshotItem(unsigned long index, ExceptionCode&);
 
-        void invalidateIteratorState();
-
     private:
-        XPathResult(Node*, const XPath::Value&);
+        XPathResult(Document*, const XPath::Value&);
         
         XPath::Value m_value;
         unsigned m_nodeSetPosition;
         XPath::NodeSet m_nodeSet; // FIXME: why duplicate the node set stored in m_value?
         unsigned short m_resultType;
-        bool m_invalidIteratorState;
-        RefPtr<Node> m_eventTarget;
-        RefPtr<EventListener> m_eventListener;
+        RefPtr<Document> m_document;
+        unsigned m_domTreeVersion;
     };
 
-}
+} // namespace WebCore
 
 #endif // ENABLE(XPATH)