Attachment 113922 Details for Bug 71499 – Slightly cleaner

[patch] Slightly cleaner

bug-71499-20111107134620.patch (text/plain), 14.22 KB, created by Adam Klein on 2011-11-07 13:46:21 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Adam Klein

Created: 2011-11-07 13:46:21 PST

Size: 14.22 KB

patch

obsolete

>Subversion Revision: 99341
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 040446aaebb52cc6095c1bfc0872b9617409a012..4f76541861ef5f1ecdbacb305f691d139941c7b1 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,40 @@
>+2011-11-07  Adam Klein  <adamk@chromium.org>
>+
>+        Only walk up the tree in search of MutationObservers if one has been added
>+        https://bugs.webkit.org/show_bug.cgi?id=71499
>+
>+        Reviewed by Ojan Vafai.
>+
>+        Analogous to m_listenerTypes, added an m_subtreeMutationObserverTypes field
>+        to Document that keeps track of which observer types have been added.
>+        This allows us to avoid doing any extra work if MutationObservers are
>+        not attached to a document.
>+
>+        This could be improved upon to keep a count of each type, as removing
>+        an observer currently has no effect on m_subtreeMutationObserverTypes.
>+        But that would require a more complex implementation: one counter per
>+        mutation type. And it would be easier to get wrong: if the counter
>+        gets out of sync with the actual state of the DOM, we could start
>+        dropping mutation notifications.
>+
>+        Test: fast/mutation/cross-document.html
>+
>+        * dom/Document.h:
>+        (WebCore::Document::hasSubtreeMutationObserverOfType):
>+        (WebCore::Document::hasSubtreeMutationObserver):
>+        (WebCore::Document::addSubtreeMutationObserverTypes):
>+        * dom/MutationObserverRegistration.h:
>+        (WebCore::MutationObserverRegistration::isSubtree):
>+        (WebCore::MutationObserverRegistration::deliveryOptions):
>+        (WebCore::MutationObserverRegistration::mutationTypes):
>+        * dom/Node.cpp:
>+        (WebCore::Node::didMoveToNewOwnerDocument): Update mutationObserverTypes when a Node is moved to a new document.
>+        (WebCore::Node::getRegisteredMutationObserversOfType): Exit early if it's known that no observers of |type| are registered.
>+        (WebCore::Node::notifyMutationObserversNodeWillDetach): Exit early if it's known no subtree observers of any type are registered.
>+        * dom/WebKitMutationObserver.cpp:
>+        (WebCore::WebKitMutationObserver::observe): Update mutationObserverTypes when an observation occurs.
>+        * dom/WebKitMutationObserver.h: Add WebKitMutationObserver::AllMutationTypes to enum
>+
> 2011-11-04  Stephen Chenney  <schenney@chromium.org>
> 
>         Crash in ScrollAnimator.cpp
>diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp
>index ca989f1905c62e4ec6c32bdb2192f9db4b8a7d86..307bd4a2a78a899b4667f730cf157d07e18d0115 100644
>--- a/Source/WebCore/dom/Document.cpp
>+++ b/Source/WebCore/dom/Document.cpp
>@@ -359,6 +359,9 @@ Document::Document(Frame* frame, const KURL& url, bool isXHTML, bool isHTML)
>     , m_compatibilityMode(NoQuirksMode)
>     , m_compatibilityModeLocked(false)
>     , m_domTreeVersion(++s_globalTreeVersion)
>+#if ENABLE(MUTATION_OBSERVERS)
>+    , m_subtreeMutationObserverTypes(0)
>+#endif
>     , m_styleSheets(StyleSheetList::create(this))
>     , m_readyState(Complete)
>     , m_styleRecalcTimer(this, &Document::styleRecalcTimerFired)
>diff --git a/Source/WebCore/dom/Document.h b/Source/WebCore/dom/Document.h
>index 4a4a9339c77e4f84274d9db11d9c46c55e895949..48dfec9096b67a79d1edc3bdda43e155c6b7822d 100644
>--- a/Source/WebCore/dom/Document.h
>+++ b/Source/WebCore/dom/Document.h
>@@ -46,6 +46,7 @@
> #include "Timer.h"
> #include "TreeScope.h"
> #include "ViewportArguments.h"
>+#include "WebKitMutationObserver.h"
> #include <wtf/Deque.h>
> #include <wtf/FixedArray.h>
> #include <wtf/OwnPtr.h>
>@@ -776,6 +777,15 @@ public:
>     void addListenerType(ListenerType listenerType) { m_listenerTypes = m_listenerTypes | listenerType; }
>     void addListenerTypeIfNeeded(const AtomicString& eventType);
> 
>+#if ENABLE(MUTATION_OBSERVERS)
>+    bool hasSubtreeMutationObserverOfType(WebKitMutationObserver::MutationType type) const
>+    {
>+        return m_subtreeMutationObserverTypes & type;
>+    }
>+    bool hasSubtreeMutationObserver() const { return m_subtreeMutationObserverTypes; }
>+    void addSubtreeMutationObserverTypes(MutationObserverOptions types) { m_subtreeMutationObserverTypes |= types; }
>+#endif
>+
>     CSSStyleDeclaration* getOverrideStyle(Element*, const String& pseudoElt);
> 
>     int nodeAbsIndex(Node*);
>@@ -1238,6 +1248,10 @@ private:
> 
>     unsigned short m_listenerTypes;
> 
>+#if ENABLE(MUTATION_OBSERVERS)
>+    MutationObserverOptions m_subtreeMutationObserverTypes;
>+#endif
>+
>     RefPtr<StyleSheetList> m_styleSheets; // All of the stylesheets that are currently in effect for our media type and stylesheet set.
>     
>     typedef ListHashSet<Node*, 32> StyleSheetCandidateListHashSet;
>diff --git a/Source/WebCore/dom/MutationObserverRegistration.h b/Source/WebCore/dom/MutationObserverRegistration.h
>index 2b822211be3d0a36ed540329653e4660c32481b2..3c7e6b57b6d62bc6d8bcd7645c8550447554ae5d 100644
>--- a/Source/WebCore/dom/MutationObserverRegistration.h
>+++ b/Source/WebCore/dom/MutationObserverRegistration.h
>@@ -53,9 +53,11 @@ public:
>     void unregister();
> 
>     bool shouldReceiveMutationFrom(Node*, WebKitMutationObserver::MutationType);
>+    bool isSubtree() const { return m_options & WebKitMutationObserver::Subtree; }
> 
>     WebKitMutationObserver* observer() { return m_observer.get(); }
>-    MutationRecordDeliveryOptions deliveryOptions() { return m_options & (WebKitMutationObserver::AttributeOldValue | WebKitMutationObserver::CharacterDataOldValue); }
>+    MutationRecordDeliveryOptions deliveryOptions() const { return m_options & (WebKitMutationObserver::AttributeOldValue | WebKitMutationObserver::CharacterDataOldValue); }
>+    MutationObserverOptions mutationTypes() const { return m_options & WebKitMutationObserver::AllMutationTypes; }
> 
> private:
>     MutationObserverRegistration(PassRefPtr<WebKitMutationObserver>, Node*);
>diff --git a/Source/WebCore/dom/Node.cpp b/Source/WebCore/dom/Node.cpp
>index b76b6d65b459486907bb4d76cf8bfd498eb1dae2..58b7cf49265cea90880a7616008bda640ab0b195 100644
>--- a/Source/WebCore/dom/Node.cpp
>+++ b/Source/WebCore/dom/Node.cpp
>@@ -2544,6 +2544,24 @@ void Node::didMoveToNewOwnerDocument()
> {
>     ASSERT(!didMoveToNewOwnerDocumentWasCalled);
>     setDidMoveToNewOwnerDocumentWasCalled(true);
>+
>+    // FIXME: Event listener types for this node should be set on the new owner document here.
>+
>+#if ENABLE(MUTATION_OBSERVERS)
>+    if (Vector<OwnPtr<MutationObserverRegistration> >* registry = mutationObserverRegistry()) {
>+        for (size_t i = 0; i < registry->size(); ++i) {
>+            if (registry->at(i)->isSubtree())
>+                document()->addSubtreeMutationObserverTypes(registry->at(i)->mutationTypes());
>+        }
>+    }
>+
>+    if (HashSet<MutationObserverRegistration*>* transientRegistry = transientMutationObserverRegistry()) {
>+        for (HashSet<MutationObserverRegistration*>::iterator iter = transientRegistry->begin(); iter != transientRegistry->end(); ++iter) {
>+            if ((*iter)->isSubtree())
>+                document()->addSubtreeMutationObserverTypes((*iter)->mutationTypes());
>+        }
>+    }
>+#endif
> }
> 
> #if ENABLE(SVG)
>@@ -2734,7 +2752,12 @@ void Node::collectMatchingObserversForMutation(HashMap<WebKitMutationObserver*,
> 
> void Node::getRegisteredMutationObserversOfType(HashMap<WebKitMutationObserver*, MutationRecordDeliveryOptions>& observers, WebKitMutationObserver::MutationType type)
> {
>-    for (Node* node = this; node; node = node->parentNode())
>+    collectMatchingObserversForMutation(observers, this, type);
>+
>+    if (!document()->hasSubtreeMutationObserverOfType(type))
>+        return;
>+
>+    for (Node* node = parentNode(); node; node = node->parentNode())
>         collectMatchingObserversForMutation(observers, node, type);
> }
> 
>@@ -2785,6 +2808,9 @@ void Node::unregisterTransientMutationObserver(MutationObserverRegistration* reg
> 
> void Node::notifyMutationObserversNodeWillDetach()
> {
>+    if (!document()->hasSubtreeMutationObserver())
>+        return;
>+
>     for (Node* node = parentNode(); node; node = node->parentNode()) {
>         if (Vector<OwnPtr<MutationObserverRegistration> >* registry = node->mutationObserverRegistry()) {
>             const size_t size = registry->size();
>diff --git a/Source/WebCore/dom/WebKitMutationObserver.cpp b/Source/WebCore/dom/WebKitMutationObserver.cpp
>index 0c7cbfc240d19e19cc1965def78a265750b6e760..a01d245c25905b19bc3f6aa37f8ed70c4057d8f5 100644
>--- a/Source/WebCore/dom/WebKitMutationObserver.cpp
>+++ b/Source/WebCore/dom/WebKitMutationObserver.cpp
>@@ -34,6 +34,7 @@
> 
> #include "WebKitMutationObserver.h"
> 
>+#include "Document.h"
> #include "MutationCallback.h"
> #include "MutationObserverRegistration.h"
> #include "MutationRecord.h"
>@@ -61,6 +62,9 @@ void WebKitMutationObserver::observe(Node* node, MutationObserverOptions options
> {
>     MutationObserverRegistration* registration = node->registerMutationObserver(this);
>     registration->resetObservation(options);
>+
>+    if (registration->isSubtree())
>+        node->document()->addSubtreeMutationObserverTypes(registration->mutationTypes());
> }
> 
> void WebKitMutationObserver::disconnect()
>diff --git a/Source/WebCore/dom/WebKitMutationObserver.h b/Source/WebCore/dom/WebKitMutationObserver.h
>index fe6e6e27e09a61473ffd82e4d7aeadee77ca3db9..49899547518bbcddb0ca7fab8eb48004932e7c78 100644
>--- a/Source/WebCore/dom/WebKitMutationObserver.h
>+++ b/Source/WebCore/dom/WebKitMutationObserver.h
>@@ -55,7 +55,9 @@ public:
>     enum MutationType {
>         ChildList = 1 << 0,
>         Attributes = 1 << 1,
>-        CharacterData = 1 << 2
>+        CharacterData = 1 << 2,
>+
>+        AllMutationTypes = ChildList | Attributes | CharacterData
>     };
> 
>     enum ObservationFlags  {
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index db0941f3c3247708f840c39089e963d2db9c555a..02867d3c64d0ec734753d67318e9c016c846b53d 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,16 @@
>+2011-11-07  Adam Klein  <adamk@chromium.org>
>+
>+        Only walk up the tree in search of MutationObservers if one has been added
>+        https://bugs.webkit.org/show_bug.cgi?id=71499
>+
>+        Reviewed by Ojan Vafai.
>+
>+        Created a basic test showing that MutationObservers
>+        are preserved when a Node switches documents.
>+
>+        * fast/mutation/cross-document-expected.txt: Added.
>+        * fast/mutation/cross-document.html: Added.
>+
> 2011-11-04  Stephen Chenney  <schenney@chromium.org>
> 
>         Crash in ScrollAnimator.cpp
>diff --git a/LayoutTests/fast/mutation/cross-document-expected.txt b/LayoutTests/fast/mutation/cross-document-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..91ed4390471885dad5d86614c652c0ed4f16b717
>--- /dev/null
>+++ b/LayoutTests/fast/mutation/cross-document-expected.txt
>@@ -0,0 +1,23 @@
>+Test that MutationObservers move correctly across documents
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+Testing basic aspects of cross-document observation.
>+PASS mutations.length is 1
>+PASS mutations[0].type is "attributes"
>+PASS mutations[0].target is div
>+PASS mutations[0].attributeName is "id"
>+PASS mutations[0].attributeNamespace is null
>+
>+Testing that subtree observation works after node is moved.
>+PASS mutations.length is 1
>+PASS mutations[0].type is "attributes"
>+PASS mutations[0].target is subDiv
>+PASS mutations[0].attributeName is "id"
>+PASS mutations[0].attributeNamespace is null
>+
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/fast/mutation/cross-document.html b/LayoutTests/fast/mutation/cross-document.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..f4ee7f0cfcd874bedbf88a9fafa7aba9f1804fa1
>--- /dev/null
>+++ b/LayoutTests/fast/mutation/cross-document.html
>@@ -0,0 +1,100 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<script src="../js/resources/js-test-pre.js"></script>
>+</head>
>+<body>
>+<script>
>+
>+window.jsTestIsAsync = true;
>+var mutations;
>+var div, subDiv;
>+
>+function testBasic() {
>+    var observer;
>+
>+    function start() {
>+        debug('Testing basic aspects of cross-document observation.');
>+
>+        mutations = null;
>+        div = document.createElement('div');
>+        observer = new WebKitMutationObserver(function(mutations) {
>+            window.mutations = mutations;
>+        });
>+
>+        observer.observe(div, {attributes: true});
>+        var newDoc = document.implementation.createDocument('', '', null);
>+        newDoc.appendChild(div);
>+        div.id = 'foo';
>+        setTimeout(finish, 0);
>+    }
>+
>+    function finish() {
>+        shouldBe('mutations.length', '1');
>+        shouldBe('mutations[0].type', '"attributes"');
>+        shouldBe('mutations[0].target', 'div');
>+        shouldBe('mutations[0].attributeName', '"id"');
>+        shouldBe('mutations[0].attributeNamespace', 'null');
>+        observer.disconnect();
>+        debug('');
>+        runNextTest();
>+    }
>+
>+    start();
>+}
>+
>+function testSubtree() {
>+    var observer;
>+
>+    function start() {
>+        debug('Testing that subtree observation works after node is moved.');
>+
>+        mutations = null;
>+        div = document.createElement('div');
>+        subDiv = div.appendChild(document.createElement('div'));
>+        observer = new WebKitMutationObserver(function(mutations) {
>+            window.mutations = mutations;
>+        });
>+
>+        observer.observe(div, {attributes: true, subtree: true});
>+        var newDoc = document.implementation.createDocument('', '', null);
>+        newDoc.appendChild(div);
>+        subDiv.id = 'foo';
>+        setTimeout(finish, 0);
>+    }
>+
>+    function finish() {
>+        shouldBe('mutations.length', '1');
>+        shouldBe('mutations[0].type', '"attributes"');
>+        shouldBe('mutations[0].target', 'subDiv');
>+        shouldBe('mutations[0].attributeName', '"id"');
>+        shouldBe('mutations[0].attributeNamespace', 'null');
>+        observer.disconnect();
>+        debug('');
>+        runNextTest();
>+    }
>+
>+    start();
>+}
>+
>+var tests = [testBasic, testSubtree];
>+var testIndex = 0;
>+
>+function runNextTest() {
>+    if (testIndex < tests.length)
>+        tests[testIndex++]();
>+    else
>+        finishJSTest();
>+}
>+
>+description('Test that MutationObservers move correctly across documents');
>+
>+if (!window.WebKitMutationObserver)
>+    testFailed('This test requires ENABLE(MUTATION_OBSERVERS)');
>+else
>+    runNextTest();
>+
>+</script>
>+<script src="../js/resources/js-test-post.js"></script>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 71499: 113546 | 113560 | 113584 | 113592 | 113605 | 113919 | 113922