Fix crash due to unexpected Node deletion during MutationObserver registration book...

https://bugs.webkit.org/show_bug.cgi?id=119124

Reviewed by Sam Weinig.

Merge https://chromium.googlesource.com/chromium/blink/+/b6afb927695b3acf2c75c25f05e99682660993e2

No new tests since I could not reproduce the crash with the test attached in the Blink change.

The bug was caused by Node::unregisterMutationObserver removing the MutationObserverRegistration
that holds the last ref to the node. Avoid that by explicitly allocating a local RefPtr to the node
in MutationObserverRegistration::unregister. Also rename it to unregisterAndDelete to clarify
the semantics and make it a static member function to be even safer.

* dom/MutationObserver.cpp:
(WebCore::MutationObserver::disconnect):
* dom/MutationObserverRegistration.cpp:
(WebCore::MutationObserverRegistration::unregisterAndDelete):
* dom/MutationObserverRegistration.h:

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

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 91d458dd3d609a0c3de407ed4a34ab72d5ba7e1e..7d2c2121f45e2103d93e37442597f6e44fb39a5a 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,25 @@
+2013-07-26  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Fix crash due to unexpected Node deletion during MutationObserver registration book-keeping
+        https://bugs.webkit.org/show_bug.cgi?id=119124
+
+        Reviewed by Sam Weinig.
+
+        Merge https://chromium.googlesource.com/chromium/blink/+/b6afb927695b3acf2c75c25f05e99682660993e2
+
+        No new tests since I could not reproduce the crash with the test attached in the Blink change.
+
+        The bug was caused by Node::unregisterMutationObserver removing the MutationObserverRegistration
+        that holds the last ref to the node. Avoid that by explicitly allocating a local RefPtr to the node
+        in MutationObserverRegistration::unregister. Also rename it to unregisterAndDelete to clarify
+        the semantics and make it a static member function to be even safer.
+
+        * dom/MutationObserver.cpp:
+        (WebCore::MutationObserver::disconnect):
+        * dom/MutationObserverRegistration.cpp:
+        (WebCore::MutationObserverRegistration::unregisterAndDelete):
+        * dom/MutationObserverRegistration.h:
+
 2013-07-29  Alex Christensen  <achristensen@apple.com>
 
         Moved GLTransportSurface.h and .cpp out of efl-specific directory.

diff --git a/Source/WebCore/dom/MutationObserver.cpp b/Source/WebCore/dom/MutationObserver.cpp
index da800137a33bff76a892686497abe6d698f60d59..d4a6e6e28ab988a440468fdc8eba133f9ca55d32 100644 (file)
--- a/Source/WebCore/dom/MutationObserver.cpp
+++ b/Source/WebCore/dom/MutationObserver.cpp
@@ -129,7 +129,7 @@ void MutationObserver::disconnect()
     m_records.clear();
     HashSet<MutationObserverRegistration*> registrations(m_registrations);
     for (HashSet<MutationObserverRegistration*>::iterator iter = registrations.begin(); iter != registrations.end(); ++iter)
-        (*iter)->unregister();
+        MutationObserverRegistration::unregisterAndDelete(*iter);
 }
 
 void MutationObserver::observationStarted(MutationObserverRegistration* registration)

diff --git a/Source/WebCore/dom/MutationObserverRegistration.cpp b/Source/WebCore/dom/MutationObserverRegistration.cpp
index 4b18f1f6a7a2054b10ea53da86b01bb6233bc993..4f81ad6786e23df131920dd4b1e7bc6806c8ae18 100644 (file)
--- a/Source/WebCore/dom/MutationObserverRegistration.cpp
+++ b/Source/WebCore/dom/MutationObserverRegistration.cpp
@@ -98,10 +98,11 @@ void MutationObserverRegistration::clearTransientRegistrations()
     m_registrationNodeKeepAlive = 0; // Balanced in observeSubtreeNodeWillDetach.
 }
 
-void MutationObserverRegistration::unregister()
+void MutationObserverRegistration::unregisterAndDelete(MutationObserverRegistration* registry)
 {
-    m_registrationNode->unregisterMutationObserver(this);
-    // The above line will cause this object to be deleted, so don't do any more in this function.
+    RefPtr<Node> registrationNode(registry->m_registrationNode);
+    registrationNode->unregisterMutationObserver(registry);
+    // The above line will cause registry to be deleted, so don't do any more in this function.
 }
 
 bool MutationObserverRegistration::shouldReceiveMutationFrom(Node* node, MutationObserver::MutationType type, const QualifiedName* attributeName) const

diff --git a/Source/WebCore/dom/MutationObserverRegistration.h b/Source/WebCore/dom/MutationObserverRegistration.h
index 486400b850634a2c41a8a448214a3716ee92145f..5a14a5959e4e6429902254e63b3584f75fd943c7 100644 (file)
--- a/Source/WebCore/dom/MutationObserverRegistration.h
+++ b/Source/WebCore/dom/MutationObserverRegistration.h
@@ -49,7 +49,7 @@ public:
     void observedSubtreeNodeWillDetach(Node*);
     void clearTransientRegistrations();
     bool hasTransientRegistrations() const { return m_transientRegistrationNodes && !m_transientRegistrationNodes->isEmpty(); }
-    void unregister();
+    static void unregisterAndDelete(MutationObserverRegistration*);
 
     bool shouldReceiveMutationFrom(Node*, MutationObserver::MutationType, const QualifiedName* attributeName) const;
     bool isSubtree() const { return m_options & MutationObserver::Subtree; }