Title: [224424] branches/safari-604.4.7.10-branch

Diff

Modified: branches/safari-604.4.7.10-branch/LayoutTests/ChangeLog (224423 => 224424)


--- branches/safari-604.4.7.10-branch/LayoutTests/ChangeLog	2017-11-03 20:19:00 UTC (rev 224423)
+++ branches/safari-604.4.7.10-branch/LayoutTests/ChangeLog	2017-11-03 20:19:05 UTC (rev 224424)
@@ -1,5 +1,22 @@
 2017-11-03  Jason Marcell  <jmarc...@apple.com>
 
+        Cherry-pick r224405. rdar://problem/35339755
+
+    2017-11-03  Ryosuke Niwa  <rn...@webkit.org>
+
+            Crash inside ChildListMutationAccumulator::enqueueMutationRecord()
+            https://bugs.webkit.org/show_bug.cgi?id=179234
+            <rdar://problem/35287748>
+
+            Reviewed by Darin Adler.
+
+            Added a regression test.
+
+            * fast/dom/MutationObserver/disconnect-observer-while-mutation-records-are-enqueued-crash-expected.txt: Added.
+            * fast/dom/MutationObserver/disconnect-observer-while-mutation-records-are-enqueued-crash.html: Added.
+
+2017-11-03  Jason Marcell  <jmarc...@apple.com>
+
         Cherry-pick r224398. rdar://problem/35329712
 
     2017-11-03  Daniel Bates  <daba...@apple.com>

Added: branches/safari-604.4.7.10-branch/LayoutTests/fast/dom/MutationObserver/disconnect-observer-while-mutation-records-are-enqueued-crash-expected.txt (0 => 224424)


--- branches/safari-604.4.7.10-branch/LayoutTests/fast/dom/MutationObserver/disconnect-observer-while-mutation-records-are-enqueued-crash-expected.txt	                        (rev 0)
+++ branches/safari-604.4.7.10-branch/LayoutTests/fast/dom/MutationObserver/disconnect-observer-while-mutation-records-are-enqueued-crash-expected.txt	2017-11-03 20:19:05 UTC (rev 224424)
@@ -0,0 +1,10 @@
+This tests disconnecting a MutationObserver while a mutation record is enqueued. WebKit should not crash.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS WebKit did not crash
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: branches/safari-604.4.7.10-branch/LayoutTests/fast/dom/MutationObserver/disconnect-observer-while-mutation-records-are-enqueued-crash.html (0 => 224424)


--- branches/safari-604.4.7.10-branch/LayoutTests/fast/dom/MutationObserver/disconnect-observer-while-mutation-records-are-enqueued-crash.html	                        (rev 0)
+++ branches/safari-604.4.7.10-branch/LayoutTests/fast/dom/MutationObserver/disconnect-observer-while-mutation-records-are-enqueued-crash.html	2017-11-03 20:19:05 UTC (rev 224424)
@@ -0,0 +1,45 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<script>
+
+description('This tests disconnecting a MutationObserver while a mutation record is enqueued. WebKit should not crash.');
+
+if (!window.GCController)
+    testFailed('This test requires GCController');
+else {
+    jsTestIsAsync = true;
+
+    var observer = new MutationObserver(function() {});
+
+    var div = document.createElement('div');
+    document.body.appendChild(div);
+
+    observer.observe(div, {childList: true});
+
+    var script = document.createElement('script');
+    script.textContent = 'disconnectObserver()';
+
+    var child = document.createElement('div');
+    child.appendChild(script);
+    div.appendChild(child);
+
+    function disconnectObserver() {
+        observer.disconnect();
+        observer = null;
+        GCController.collect();
+        observer = new MutationObserver(function() {});
+    }
+
+    window._onload_ = () => {
+        testPassed('WebKit did not crash');
+        finishJSTest();
+    }
+}
+
+var successfullyParsed = true;
+
+</script>
+</body>
+</html>

Modified: branches/safari-604.4.7.10-branch/Source/WebCore/ChangeLog (224423 => 224424)


--- branches/safari-604.4.7.10-branch/Source/WebCore/ChangeLog	2017-11-03 20:19:00 UTC (rev 224423)
+++ branches/safari-604.4.7.10-branch/Source/WebCore/ChangeLog	2017-11-03 20:19:05 UTC (rev 224424)
@@ -1,5 +1,44 @@
 2017-11-03  Jason Marcell  <jmarc...@apple.com>
 
+        Cherry-pick r224405. rdar://problem/35339755
+
+    2017-11-03  Ryosuke Niwa  <rn...@webkit.org>
+
+            Crash inside ChildListMutationAccumulator::enqueueMutationRecord()
+            https://bugs.webkit.org/show_bug.cgi?id=179234
+            <rdar://problem/35287748>
+
+            Reviewed by Darin Adler.
+
+            Fixed the crash by keeping MutationObserver referenced by MutationObserverInterestGroup alive.
+
+            Also added hasCallback() virtual function on MutationObserver to check whether the callback is alive
+            to work around the bug that JS function referenced by MutationObserver isn't kept alive.
+            We'll address this bug separately in https://webkit.org/b/179224.
+
+            Test: fast/dom/MutationObserver/disconnect-observer-while-mutation-records-are-enqueued-crash.html
+
+            * bindings/scripts/CodeGeneratorJS.pm:
+            (GenerateCallbackHeaderContent): Added an override for the newly added virtual hasCallback().
+            * dom/MutationCallback.h:
+            * dom/MutationObserver.cpp:
+            (WebCore::MutationObserver::deliver): Added the aforementioned workaround.
+            * dom/MutationObserverInterestGroup.cpp:
+            (WebCore::MutationObserverInterestGroup::MutationObserverInterestGroup): Fixed the crash by using Ref.
+            (WebCore::MutationObserverInterestGroup::enqueueMutationRecord): Ditto.
+            * dom/MutationObserverInterestGroup.h:
+            * dom/NativeNodeFilter.cpp:
+            (WebCore::NativeNodeFilter::hasCallback const): Always return true here. This function is never called
+            but we still need to implement it since NodeFilter has a pure virtual hasCallback() now.
+            * dom/NativeNodeFilter.h:
+            * dom/Node.cpp:
+            (WebCore::collectMatchingObserversForMutation): Use Ref to fix the crash.
+            (WebCore::Node::registeredMutationObservers): Ditto.
+            * dom/Node.h:
+            * dom/NodeFilter.h:
+
+2017-11-03  Jason Marcell  <jmarc...@apple.com>
+
         Cherry-pick r224398. rdar://problem/35329712
 
     2017-11-03  Daniel Bates  <daba...@apple.com>

Modified: branches/safari-604.4.7.10-branch/Source/WebCore/dom/MutationObserverInterestGroup.cpp (224423 => 224424)


--- branches/safari-604.4.7.10-branch/Source/WebCore/dom/MutationObserverInterestGroup.cpp	2017-11-03 20:19:00 UTC (rev 224423)
+++ branches/safari-604.4.7.10-branch/Source/WebCore/dom/MutationObserverInterestGroup.cpp	2017-11-03 20:19:05 UTC (rev 224424)
@@ -37,7 +37,7 @@
 
 namespace WebCore {
 
-inline MutationObserverInterestGroup::MutationObserverInterestGroup(HashMap<MutationObserver*, MutationRecordDeliveryOptions>&& observers, MutationRecordDeliveryOptions oldValueFlag)
+inline MutationObserverInterestGroup::MutationObserverInterestGroup(HashMap<Ref<MutationObserver>, MutationRecordDeliveryOptions>&& observers, MutationRecordDeliveryOptions oldValueFlag)
     : m_observers(WTFMove(observers))
     , m_oldValueFlag(oldValueFlag)
 {
@@ -67,9 +67,9 @@
 {
     RefPtr<MutationRecord> mutationWithNullOldValue;
     for (auto& observerOptionsPair : m_observers) {
-        MutationObserver* observer = observerOptionsPair.key;
+        auto& observer = observerOptionsPair.key.get();
         if (hasOldValue(observerOptionsPair.value)) {
-            observer->enqueueMutationRecord(mutation.copyRef());
+            observer.enqueueMutationRecord(mutation.copyRef());
             continue;
         }
         if (!mutationWithNullOldValue) {
@@ -78,7 +78,7 @@
             else
                 mutationWithNullOldValue = MutationRecord::createWithNullOldValue(mutation).ptr();
         }
-        observer->enqueueMutationRecord(*mutationWithNullOldValue);
+        observer.enqueueMutationRecord(*mutationWithNullOldValue);
     }
 }
 

Modified: branches/safari-604.4.7.10-branch/Source/WebCore/dom/MutationObserverInterestGroup.h (224423 => 224424)


--- branches/safari-604.4.7.10-branch/Source/WebCore/dom/MutationObserverInterestGroup.h	2017-11-03 20:19:00 UTC (rev 224423)
+++ branches/safari-604.4.7.10-branch/Source/WebCore/dom/MutationObserverInterestGroup.h	2017-11-03 20:19:05 UTC (rev 224424)
@@ -42,7 +42,7 @@
 class MutationObserverInterestGroup {
     WTF_MAKE_FAST_ALLOCATED;
 public:
-    MutationObserverInterestGroup(HashMap<MutationObserver*, MutationRecordDeliveryOptions>&&, MutationRecordDeliveryOptions);
+    MutationObserverInterestGroup(HashMap<Ref<MutationObserver>, MutationRecordDeliveryOptions>&&, MutationRecordDeliveryOptions);
 
     static std::unique_ptr<MutationObserverInterestGroup> createForChildListMutation(Node& target)
     {
@@ -77,7 +77,7 @@
 
     bool hasOldValue(MutationRecordDeliveryOptions options) const { return options & m_oldValueFlag; }
 
-    HashMap<MutationObserver*, MutationRecordDeliveryOptions> m_observers;
+    HashMap<Ref<MutationObserver>, MutationRecordDeliveryOptions> m_observers;
     MutationRecordDeliveryOptions m_oldValueFlag;
 };
 

Modified: branches/safari-604.4.7.10-branch/Source/WebCore/dom/Node.cpp (224423 => 224424)


--- branches/safari-604.4.7.10-branch/Source/WebCore/dom/Node.cpp	2017-11-03 20:19:00 UTC (rev 224423)
+++ branches/safari-604.4.7.10-branch/Source/WebCore/dom/Node.cpp	2017-11-03 20:19:05 UTC (rev 224424)
@@ -2263,7 +2263,7 @@
     return &data->transientRegistry;
 }
 
-template<typename Registry> static inline void collectMatchingObserversForMutation(HashMap<MutationObserver*, MutationRecordDeliveryOptions>& observers, Registry* registry, Node& target, MutationObserver::MutationType type, const QualifiedName* attributeName)
+template<typename Registry> static inline void collectMatchingObserversForMutation(HashMap<Ref<MutationObserver>, MutationRecordDeliveryOptions>& observers, Registry* registry, Node& target, MutationObserver::MutationType type, const QualifiedName* attributeName)
 {
     if (!registry)
         return;
@@ -2271,7 +2271,7 @@
     for (auto& registration : *registry) {
         if (registration->shouldReceiveMutationFrom(target, type, attributeName)) {
             auto deliveryOptions = registration->deliveryOptions();
-            auto result = observers.add(&registration->observer(), deliveryOptions);
+            auto result = observers.add(registration->observer(), deliveryOptions);
             if (!result.isNewEntry)
                 result.iterator->value |= deliveryOptions;
         }
@@ -2278,9 +2278,9 @@
     }
 }
 
-HashMap<MutationObserver*, MutationRecordDeliveryOptions> Node::registeredMutationObservers(MutationObserver::MutationType type, const QualifiedName* attributeName)
+HashMap<Ref<MutationObserver>, MutationRecordDeliveryOptions> Node::registeredMutationObservers(MutationObserver::MutationType type, const QualifiedName* attributeName)
 {
-    HashMap<MutationObserver*, MutationRecordDeliveryOptions> result;
+    HashMap<Ref<MutationObserver>, MutationRecordDeliveryOptions> result;
     ASSERT((type == MutationObserver::Attributes && attributeName) || !attributeName);
     collectMatchingObserversForMutation(result, mutationObserverRegistry(), *this, type, attributeName);
     collectMatchingObserversForMutation(result, transientMutationObserverRegistry(), *this, type, attributeName);

Modified: branches/safari-604.4.7.10-branch/Source/WebCore/dom/Node.h (224423 => 224424)


--- branches/safari-604.4.7.10-branch/Source/WebCore/dom/Node.h	2017-11-03 20:19:00 UTC (rev 224423)
+++ branches/safari-604.4.7.10-branch/Source/WebCore/dom/Node.h	2017-11-03 20:19:05 UTC (rev 224424)
@@ -534,7 +534,7 @@
     EventTargetData* eventTargetDataConcurrently() final;
     EventTargetData& ensureEventTargetData() final;
 
-    HashMap<MutationObserver*, MutationRecordDeliveryOptions> registeredMutationObservers(MutationObserver::MutationType, const QualifiedName* attributeName);
+    HashMap<Ref<MutationObserver>, MutationRecordDeliveryOptions> registeredMutationObservers(MutationObserver::MutationType, const QualifiedName* attributeName);
     void registerMutationObserver(MutationObserver&, MutationObserverOptions, const HashSet<AtomicString>& attributeFilter);
     void unregisterMutationObserver(MutationObserverRegistration&);
     void registerTransientMutationObserver(MutationObserverRegistration&);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to