Attachment 210010 Details for Bug 99244 – Patch

[patch] Patch

bug-99244-20130829215955.patch (text/plain), 20.60 KB, created by Chris Dumez on 2013-08-29 11:59:59 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2013-08-29 11:59:59 PDT

Size: 20.60 KB

patch

obsolete

>Subversion Revision: 154823
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index c0652fcc4cf5cbd93cfd72d692625b9a0e6dea5e..7ec94153d0ab3cf58b7bb807815d4d1144fd3203 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,36 @@
>+2013-08-29  Christophe Dumez  <ch.dumez@sisa.samsung.com>
>+
>+        According to DOM4, all DocType nodes should have a document
>+        https://bugs.webkit.org/show_bug.cgi?id=99244
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Doctypes now always have a node document and can be moved across document boundaries as per
>+        the latest DOM4 specification:
>+        http://dom.spec.whatwg.org/#dom-domimplementation-createdocumenttype
>+        http://dom.spec.whatwg.org/#dom-node-ownerdocument
>+
>+        This means that DOMImplementation.createDocumentType() now sets the ownerDocument of the
>+        new DocumentType Node to the associated document of the current "context" object. In
>+        DOM4, all nodes have a document at all times. DocumentType nodes can now be moved across
>+        document boundaries so that the node can be added to a Document after being created.
>+
>+        This means we will no longer need to special case DocumentType nodes in the code and
>+        Node::document() can no longer return NULL, which means that we'll be able to remove
>+        NULL checks in call sites.
>+
>+        Firefox stable and since recently Blink already follow DOM4 here while IE10 does not (yet).
>+
>+        Test: fast/dom/createDocumentType-ownerDocument.html
>+
>+        * dom/ContainerNode.cpp:
>+        (WebCore::checkAcceptChild):
>+        * dom/DOMImplementation.cpp:
>+        (WebCore::DOMImplementation::createDocumentType):
>+        (WebCore::DOMImplementation::createDocument):
>+        * dom/Node.h:
>+        (WebCore::Node::document):
>+
> 2013-08-29  Pratik Solanki  <pratik.solanki@gmail.com>
> 
>         SharedBuffer m_segments and m_dataArray must be exclusive
>diff --git a/Source/WebCore/dom/ContainerNode.cpp b/Source/WebCore/dom/ContainerNode.cpp
>index c79297adb42cd80118184e972ac44be7f56acc6f..1bac909382f06fdd2a97a5947d0bd266303e4d9f 100644
>--- a/Source/WebCore/dom/ContainerNode.cpp
>+++ b/Source/WebCore/dom/ContainerNode.cpp
>@@ -213,8 +213,6 @@ static inline ExceptionCode checkAcceptChild(ContainerNode* newParent, Node* new
> 
>     if (newParent->isReadOnlyNode())
>         return NO_MODIFICATION_ALLOWED_ERR;
>-    if (newChild->inDocument() && newChild->isDocumentTypeNode())
>-        return HIERARCHY_REQUEST_ERR;
>     if (containsConsideringHostElements(newChild, newParent))
>         return HIERARCHY_REQUEST_ERR;
> 
>diff --git a/Source/WebCore/dom/DOMImplementation.cpp b/Source/WebCore/dom/DOMImplementation.cpp
>index e81ca8dfa061ffc16c5c42a0933fcb8b1127d6c0..cd76ffbe0868d98b5acbf6a55dd779bfc893e8ac 100644
>--- a/Source/WebCore/dom/DOMImplementation.cpp
>+++ b/Source/WebCore/dom/DOMImplementation.cpp
>@@ -219,7 +219,7 @@ PassRefPtr<DocumentType> DOMImplementation::createDocumentType(const String& qua
>     if (!Document::parseQualifiedName(qualifiedName, prefix, localName, ec))
>         return 0;
> 
>-    return DocumentType::create(0, qualifiedName, publicId, systemId);
>+    return DocumentType::create(m_document, qualifiedName, publicId, systemId);
> }
> 
> DOMImplementation* DOMImplementation::getInterface(const String& /*feature*/)
>@@ -251,16 +251,6 @@ PassRefPtr<Document> DOMImplementation::createDocument(const String& namespaceUR
>             return 0;
>     }
> 
>-    // WRONG_DOCUMENT_ERR: Raised if doctype has already been used with a different document or was
>-    // created from a different implementation.
>-    // Hixie's interpretation of the DOM Core spec suggests we should prefer
>-    // other exceptions to WRONG_DOCUMENT_ERR (based on order mentioned in spec),
>-    // but this matches the new DOM Core spec (http://www.w3.org/TR/domcore/).
>-    if (doctype && doctype->document()) {
>-        ec = WRONG_DOCUMENT_ERR;
>-        return 0;
>-    }
>-
>     if (doctype)
>         doc->appendChild(doctype);
>     if (documentElement)
>diff --git a/Source/WebCore/dom/Node.h b/Source/WebCore/dom/Node.h
>index d8b6ec72bf052c0e47adceef93da00574277a1e6..7c702661a6f4ef6c44c8292e5c57b0b9936304f5 100644
>--- a/Source/WebCore/dom/Node.h
>+++ b/Source/WebCore/dom/Node.h
>@@ -395,18 +395,16 @@ public:
> 
>     unsigned nodeIndex() const;
> 
>-    // Returns the DOM ownerDocument attribute. This method never returns NULL, except in the case 
>-    // of (1) a Document node or (2) a DocumentType node that is not used with any Document yet. 
>+    // Returns the DOM ownerDocument attribute. This method never returns 0, except in the case
>+    // of a Document node.
>     Document* ownerDocument() const;
> 
>-    // Returns the document associated with this node. This method never returns NULL, except in the case 
>-    // of a DocumentType node that is not used with any Document yet. A Document node returns itself.
>+    // Returns the document associated with this node. This method never returns 0.
>+    // A Document node returns itself.
>     Document* document() const
>     {
>         ASSERT(this);
>-        // FIXME: below ASSERT is useful, but prevents the use of document() in the constructor or destructor
>-        // due to the virtual function call to nodeType().
>-        ASSERT(documentInternal() || (nodeType() == DOCUMENT_TYPE_NODE && !inDocument()));
>+        ASSERT(documentInternal());
>         return documentInternal();
>     }
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 428077375653a4c630b8ff2b1e26f083d87e21b2..ef6fc24d2407fda3bd1928f69cc77f009810dc17 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,26 @@
>+2013-08-29  Christophe Dumez  <ch.dumez@sisa.samsung.com>
>+
>+        According to DOM4, all DocType nodes should have a document
>+        https://bugs.webkit.org/show_bug.cgi?id=99244
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add layout test to check that DocumentType Nodes have a document after being
>+        created. Also update a few existing test cases to reflect this change.
>+
>+        * fast/dom/DOMImplementation/createDocument-with-used-doctype-expected.txt:
>+        * fast/dom/DOMImplementation/createDocument-with-used-doctype.html:
>+        * fast/dom/DOMImplementation/resources/createDocument-with-used-doctype-frame.html:
>+        * fast/dom/XMLSerializer-doctype2-expected.txt:
>+        * fast/dom/XMLSerializer-doctype2.html:
>+        * fast/dom/createDocumentType-ownerDocument-expected.txt: Added.
>+        * fast/dom/createDocumentType-ownerDocument.html: Added.
>+        * fast/dom/move-nodes-across-documents.html:
>+        * fast/dom/node-iterator-with-doctype-root-expected.txt:
>+        * fast/dom/node-iterator-with-doctype-root.html:
>+        * fast/events/dispatch-event-no-document-expected.txt:
>+        * fast/events/dispatch-event-no-document.html:
>+
> 2013-08-29  Joseph Pecoraro  <pecoraro@apple.com>
> 
>         Web Inspector: Consolidate inspector-protocol Debugger tests
>diff --git a/LayoutTests/fast/dom/DOMImplementation/createDocument-with-used-doctype-expected.txt b/LayoutTests/fast/dom/DOMImplementation/createDocument-with-used-doctype-expected.txt
>index 69cfc5a98db74fbe32ac9b678a6134af9736dd59..5caf6879e73fd6bbe5cadfd15d1432615e5fa044 100644
>--- a/LayoutTests/fast/dom/DOMImplementation/createDocument-with-used-doctype-expected.txt
>+++ b/LayoutTests/fast/dom/DOMImplementation/createDocument-with-used-doctype-expected.txt
>@@ -1,2 +1,17 @@
>-PASS
>+document.implementation.createDocument with current document's DOCTYPE.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+PASS doc = document.implementation.createDocument(null, null, document.doctype) did not throw exception.
>+PASS doc.doctype is doctype
>+PASS doc.firstChild is doctype
>+PASS document.doctype is null
>+
>+
>+
>+
>+
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
> 
>diff --git a/LayoutTests/fast/dom/DOMImplementation/createDocument-with-used-doctype.html b/LayoutTests/fast/dom/DOMImplementation/createDocument-with-used-doctype.html
>index 42c238839850b0f52c7a7ed9a4fcb93f0f735d54..34dbb7d9ca42590cb1698a5f3bc0d5349a23c683 100644
>--- a/LayoutTests/fast/dom/DOMImplementation/createDocument-with-used-doctype.html
>+++ b/LayoutTests/fast/dom/DOMImplementation/createDocument-with-used-doctype.html
>@@ -1,18 +1,7 @@
> <body>
>+<script src="../../js/resources/js-test-pre.js"></script>
> <script>
>-if (window.testRunner) {
>-    testRunner.dumpAsText();
>-    testRunner.waitUntilDone();
>-}
>-
>-function gc()
>-{
>-    if (window.GCController)
>-        return GCController.collect();
>-
>-    for (var i = 0; i < 10000; i++)
>-        var s = new String("");
>-}
>+window.jsTestIsAsync = true;
> 
> // Reload multiple times, to make crashing more likely.
> var iterationsLeft = 50;
>@@ -22,12 +11,11 @@ function test()
>         frames[0].history.go(0);
>     } else {
>         gc();
>-        document.getElementById("result").innerText = frames[0].document.body.textContent;
>-        if (window.testRunner)
>-            testRunner.notifyDone();
>+        debug(frames[0].document.body.outerHTML);
>+        finishJSTest();
>     }
> }
> </script>
>-<div id="result">FAIL</div>
> <iframe src="resources/createDocument-with-used-doctype-frame.html" onload="test()"></iframe>
>+<script src="../../js/resources/js-test-post.js"></script>
> </body>
>diff --git a/LayoutTests/fast/dom/DOMImplementation/resources/createDocument-with-used-doctype-frame.html b/LayoutTests/fast/dom/DOMImplementation/resources/createDocument-with-used-doctype-frame.html
>index e0e7e6795d7a891fb243760f4207c63517f5c85d..ec6687b805b68ea0fc64e6e636aac00fd8803850 100644
>--- a/LayoutTests/fast/dom/DOMImplementation/resources/createDocument-with-used-doctype-frame.html
>+++ b/LayoutTests/fast/dom/DOMImplementation/resources/createDocument-with-used-doctype-frame.html
>@@ -1,17 +1,18 @@
> <!doctype html>
>-<title>document.implementation.createDocument with current document's DOCTYPE</title>
>+<html>
>+<head>
>+<script src="../../../js/resources/js-test-pre.js"></script>
>+</head>
> <body>
>-FAIL (Script did not run);
> <script>
>-document.body.textContent = "FAIL";
>-try {
>-    document.implementation.createDocument(null, null, document.doctype);
>-    document.body.textContent = "FAIL (no exception)";
>-}
>-catch(e) {
>-    if (e.code === DOMException.WRONG_DOCUMENT_ERR || e.code === DOMException.NOT_SUPPORTED_ERR)
>-        document.body.textContent = "PASS";
>-    else
>-        document.body.textContent = "FAIL (wrong exception: " + e.code + ")";
>-}
>+description("document.implementation.createDocument with current document's DOCTYPE.");
>+
>+var doctype = document.doctype;
>+var doc;
>+shouldNotThrow("doc = document.implementation.createDocument(null, null, document.doctype)");
>+shouldBe('doc.doctype', 'doctype');
>+shouldBe('doc.firstChild', 'doctype');
>+shouldBe('document.doctype', 'null');
> </script>
>+</body>
>+</html>
>diff --git a/LayoutTests/fast/dom/XMLSerializer-doctype2-expected.txt b/LayoutTests/fast/dom/XMLSerializer-doctype2-expected.txt
>index abf8e200a50f5da2644a9ce68733c9f664bdae1a..3104b5db83c40056fd85b2367c4a8444e48bf9b3 100644
>--- a/LayoutTests/fast/dom/XMLSerializer-doctype2-expected.txt
>+++ b/LayoutTests/fast/dom/XMLSerializer-doctype2-expected.txt
>@@ -1,2 +1,8 @@
>-This tests XMLSerializer.serializeToString() on a DocumentType node that does not have a document associated with it. It should throw an INVALID_ACCESS_ERR DOMException.
>-PASS: an Error: InvalidAccessError: DOM Exception 15 was thrown as expected.
>+This tests XMLSerializer.serializeToString() on a newly created DocumentType node does not throw since the node has an associated document.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS text = serializer.serializeToString(docType) did not throw exception.
>+PASS text is "<!DOCTYPE aDocTypeName PUBLIC \"aPublicID\" \"aSystemID\">"
>+
>diff --git a/LayoutTests/fast/dom/XMLSerializer-doctype2.html b/LayoutTests/fast/dom/XMLSerializer-doctype2.html
>index 5daf8cb8fb58cf743ce958e067240106b2e25613..6afc809eb8788a63f7d0997b4bff2abf931b8790 100644
>--- a/LayoutTests/fast/dom/XMLSerializer-doctype2.html
>+++ b/LayoutTests/fast/dom/XMLSerializer-doctype2.html
>@@ -1,37 +1,19 @@
> <html>
> <head>
>-    <script>
>-        function debug(str) {
>-            li = document.createElement('li');
>-            li.appendChild(document.createTextNode(str));
>-            document.getElementById('console').appendChild(li);
>-        }
>-
>-        function runTests() {
>-            if (window.testRunner)
>-                testRunner.dumpAsText();
>-
>-            var docType = window.document.implementation.createDocumentType("aDocTypeName", "aPublicID", "aSystemID");
>+<script src="../js/resources/js-test-pre.js"></script>
>+</head>
>+<body>
>+<script>
>+description("This tests XMLSerializer.serializeToString() on a newly created DocumentType node does not throw since the node has an associated document.");
> 
>-            var serializer = new XMLSerializer();
>+var docType = window.document.implementation.createDocumentType("aDocTypeName", "aPublicID", "aSystemID");
> 
>-            try {
>-                var text = serializer.serializeToString(docType);
>-                debug("FAIL: XMLSerializer.serializeToString() should throw an exception if it tries to serialize a documentless DocumentType node.");
>-            } catch (e) {
>-                if (e == "Error: InvalidAccessError: DOM Exception 15")
>-                    debug("PASS: an " + e + " was thrown as expected.")
>-                else
>-                    debug("FAIL: XMLSerializer.serializeToString() should throw an INVALID_ACCESS_ERR DOMExeption if it tries to serialize a documentless DocumentType node.");
>-            }
>-        }
>-    </script>
>-</head>
>-<body onload="runTests()">
>-This tests XMLSerializer.serializeToString() on a DocumentType node that does not have a document associated
>-with it.  It should throw an INVALID_ACCESS_ERR DOMException.
>+var serializer = new XMLSerializer();
> 
>-<ul id="console">
>-</ul>
>+var text;
>+shouldNotThrow("text = serializer.serializeToString(docType)");
>+shouldBeEqualToString("text", "<!DOCTYPE aDocTypeName PUBLIC \"aPublicID\" \"aSystemID\">");
>+</script>
>+<script src="../js/resources/js-test-pre.js"></script>
> </body>
> </html>
>diff --git a/LayoutTests/fast/dom/createDocumentType-ownerDocument-expected.txt b/LayoutTests/fast/dom/createDocumentType-ownerDocument-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..e629077f17d04f25514e9742c90f1f0cb10201c2
>--- /dev/null
>+++ b/LayoutTests/fast/dom/createDocumentType-ownerDocument-expected.txt
>@@ -0,0 +1,13 @@
>+Tests that DOMImplementation.createDocumentType() properly sets the node's document to the associated document of the context object.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS docType.ownerDocument is document
>+PASS newDocument = document.implementation.createDocument('', null, docType) did not throw exception.
>+PASS newDocument.doctype is docType
>+PASS newDocument.doctype.ownerDocument is newDocument
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/fast/dom/createDocumentType-ownerDocument.html b/LayoutTests/fast/dom/createDocumentType-ownerDocument.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..ace9cce206b872bbe085b5a44e15a132e1a06dde
>--- /dev/null
>+++ b/LayoutTests/fast/dom/createDocumentType-ownerDocument.html
>@@ -0,0 +1,20 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<link rel="help" href="http://www.w3.org/TR/2012/WD-dom-20121206/#dom-domimplementation-createdocumenttype">
>+<script src="../js/resources/js-test-pre.js"></script>
>+</head>
>+<body>
>+<script>
>+description("Tests that DOMImplementation.createDocumentType() properly sets the node's document to the associated document of the context object.");
>+var docType = document.implementation.createDocumentType("html", null, null);
>+shouldBe("docType.ownerDocument", "document");
>+var newDocument;
>+shouldNotThrow("newDocument = document.implementation.createDocument('', null, docType)");
>+shouldBe("newDocument.doctype", "docType");
>+shouldBe("newDocument.doctype.ownerDocument", "newDocument");
>+
>+</script>
>+<script src="../js/resources/js-test-post.js"></script>
>+</body>
>+</html>
>diff --git a/LayoutTests/fast/dom/move-nodes-across-documents.html b/LayoutTests/fast/dom/move-nodes-across-documents.html
>index 58eb3da30b5b9d6e2e0d77bc26831cdb3d51af2f..d3501a17db86c4ec860fb5f122c7d5cdae6f5a35 100644
>--- a/LayoutTests/fast/dom/move-nodes-across-documents.html
>+++ b/LayoutTests/fast/dom/move-nodes-across-documents.html
>@@ -168,7 +168,7 @@ function run()
>     });
>     runTest(function() {
>         iframeDoc.implementation.createDocument('', 'html', document.doctype); 
>-    }, 'WrongDocumentError');
>+    });
>     runTest(function() {
>         rangeInIframe().compareBoundaryPoints(Range.END_TO_END, rangeInCurrentDocument());
>     }, 'WrongDocumentError');
>@@ -179,7 +179,7 @@ function run()
>     runTest(function() {
>         iframeDoc.appendChild(document.doctype);
>         console.log(document.doctype);
>-    }, 'HierarchyRequestError');
>+    }, 'NotFoundError');
> 
>     // When setting a boundary of the range in a different
>     // document, the call should succeed and the range should be collapsed.
>diff --git a/LayoutTests/fast/dom/node-iterator-with-doctype-root-expected.txt b/LayoutTests/fast/dom/node-iterator-with-doctype-root-expected.txt
>index 56da317aae847aebed58ff2505a06db0ead8a01c..be190f184bdec8ae8acab737739f6305760913f4 100644
>--- a/LayoutTests/fast/dom/node-iterator-with-doctype-root-expected.txt
>+++ b/LayoutTests/fast/dom/node-iterator-with-doctype-root-expected.txt
>@@ -1,5 +1,5 @@
> NodeIterator rooted at a DocumentType node not yet associated with a document:
>-PASS iter.referenceNode.ownerDocument is null
>+PASS iter.referenceNode.ownerDocument is document
> PASS iter.nextNode() is dt
> PASS iter.nextNode() is null
> PASS iter.previousNode() is dt
>diff --git a/LayoutTests/fast/dom/node-iterator-with-doctype-root.html b/LayoutTests/fast/dom/node-iterator-with-doctype-root.html
>index 08b36eb7a3507cdada5956843cfb130917b2cb95..acb8b1ed169b5dbf78e3258ae028ee898ddc719b 100644
>--- a/LayoutTests/fast/dom/node-iterator-with-doctype-root.html
>+++ b/LayoutTests/fast/dom/node-iterator-with-doctype-root.html
>@@ -7,7 +7,7 @@
> var dt = document.implementation.createDocumentType("foo", "", "");
> var iter = document.createNodeIterator(dt, NodeFilter.SHOW_ALL, null, true);
> debug("NodeIterator rooted at a DocumentType node not yet associated with a document:");
>-shouldBe('iter.referenceNode.ownerDocument', 'null');
>+shouldBe('iter.referenceNode.ownerDocument', 'document');
> shouldBe('iter.nextNode()', 'dt');
> shouldBe('iter.nextNode()', 'null');
> shouldBe('iter.previousNode()', 'dt');
>diff --git a/LayoutTests/fast/events/dispatch-event-no-document-expected.txt b/LayoutTests/fast/events/dispatch-event-no-document-expected.txt
>index ac6bd7a01168d7a3a60cc349b8709ec026681efe..f9baa35e8cb5cb551223bf3467a897abd3133f3f 100644
>--- a/LayoutTests/fast/events/dispatch-event-no-document-expected.txt
>+++ b/LayoutTests/fast/events/dispatch-event-no-document-expected.txt
>@@ -1,5 +1,6 @@
>-The test verifies that EventTarget with an event listener but without ScriptExecutionContext (not inserted into Document) does not crash during an attempt to dispatch an event. It should just not call the handler. This is what FF 3.5 is also doing.
>+The test verifies that EventTarget with an event listener not inserted into a Document does not crash during an attempt to dispatch an event.
> 
>-Test passes if there is no crash, and event is not dispatched.
>+Test passes if there is no crash, and event is dispatched.
>+
>+PASS: generic handled.
> 
>-PASS
>diff --git a/LayoutTests/fast/events/dispatch-event-no-document.html b/LayoutTests/fast/events/dispatch-event-no-document.html
>index 7f99ad5b92dcc14ca46a4a53ce699aafefe0f182..ccee7f8a3b37323b269ee117c1a68a76044b0939 100644
>--- a/LayoutTests/fast/events/dispatch-event-no-document.html
>+++ b/LayoutTests/fast/events/dispatch-event-no-document.html
>@@ -1,6 +1,6 @@
> <script>
> function handleEvent(message) {
>-    document.getElementById("log").innerHTML = "FAIL: " + message + " handled.<br>";
>+    document.getElementById("log").innerHTML = "PASS: " + message + " handled.<br>";
> }
> 
> function test() {
>@@ -21,6 +21,6 @@ function test() {
> }
> </script>
> <body onload="test()">
>-<p>The test verifies that EventTarget with an event listener but without ScriptExecutionContext (not inserted into Document) does not crash during an attempt to dispatch an event. It should just not call the handler. This is what FF 3.5 is also doing.</p>
>-<p>Test passes if there is no crash, and event is not dispatched.</p>
>-<div id="log">PASS</div>
>+<p>The test verifies that EventTarget with an event listener not inserted into a Document does not crash during an attempt to dispatch an event.</p>
>+<p>Test passes if there is no crash, and event is dispatched.</p>
>+<div id="log">FAIL</div>

Actions: View | Formatted Diff | Diff

Attachments on bug 99244: 168546 | 210010