From 1dcb35cc5697c04409b011a7ad6342f4d3fd816c Mon Sep 17 00:00:00 2001 From: "rniwa@webkit.org" Date: Tue, 30 Jul 2013 00:12:02 +0000 Subject: [PATCH] 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: git-svn-id: https://svn.webkit.org/repository/webkit/trunk@153447 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- Source/WebCore/ChangeLog | 22 +++++++++++++++++++ Source/WebCore/dom/MutationObserver.cpp | 2 +- .../dom/MutationObserverRegistration.cpp | 7 +++--- .../dom/MutationObserverRegistration.h | 2 +- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index 91d458dd3d60..7d2c2121f45e 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,25 @@ +2013-07-26 Ryosuke Niwa + + 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 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 da800137a33b..d4a6e6e28ab9 100644 --- a/Source/WebCore/dom/MutationObserver.cpp +++ b/Source/WebCore/dom/MutationObserver.cpp @@ -129,7 +129,7 @@ void MutationObserver::disconnect() m_records.clear(); HashSet registrations(m_registrations); for (HashSet::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 4b18f1f6a7a2..4f81ad6786e2 100644 --- 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 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 486400b85063..5a14a5959e4e 100644 --- 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; } -- 2.36.0