Diff
Modified: trunk/LayoutTests/ChangeLog (224404 => 224405)
--- trunk/LayoutTests/ChangeLog 2017-11-03 16:59:30 UTC (rev 224404)
+++ trunk/LayoutTests/ChangeLog 2017-11-03 17:10:42 UTC (rev 224405)
@@ -1,3 +1,16 @@
+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-02 Andy Estes <aes...@apple.com>
[Payment Request] show() should only be called with user activation
Added: trunk/LayoutTests/fast/dom/MutationObserver/disconnect-observer-while-mutation-records-are-enqueued-crash-expected.txt (0 => 224405)
--- trunk/LayoutTests/fast/dom/MutationObserver/disconnect-observer-while-mutation-records-are-enqueued-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/MutationObserver/disconnect-observer-while-mutation-records-are-enqueued-crash-expected.txt 2017-11-03 17:10:42 UTC (rev 224405)
@@ -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: trunk/LayoutTests/fast/dom/MutationObserver/disconnect-observer-while-mutation-records-are-enqueued-crash.html (0 => 224405)
--- trunk/LayoutTests/fast/dom/MutationObserver/disconnect-observer-while-mutation-records-are-enqueued-crash.html (rev 0)
+++ trunk/LayoutTests/fast/dom/MutationObserver/disconnect-observer-while-mutation-records-are-enqueued-crash.html 2017-11-03 17:10:42 UTC (rev 224405)
@@ -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: trunk/Source/WebCore/ChangeLog (224404 => 224405)
--- trunk/Source/WebCore/ChangeLog 2017-11-03 16:59:30 UTC (rev 224404)
+++ trunk/Source/WebCore/ChangeLog 2017-11-03 17:10:42 UTC (rev 224405)
@@ -1,3 +1,38 @@
+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 Zalan Bujtas <za...@apple.com>
RenderObject::*positioned() naming cleanup
Modified: trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm (224404 => 224405)
--- trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm 2017-11-03 16:59:30 UTC (rev 224404)
+++ trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm 2017-11-03 17:10:42 UTC (rev 224405)
@@ -5882,6 +5882,10 @@
push(@$contentRef, " ${className}(JSC::JSObject*, JSDOMGlobalObject*);\n\n");
+ if ($interfaceOrCallback->extendedAttributes->{IsWeakCallback}) {
+ push(@$contentRef, " bool hasCallback() const final { return m_data && m_data->callback(); }\n\n");
+ }
+
push(@$contentRef, " void visitJSFunction(JSC::SlotVisitor&) override;\n\n") if $interfaceOrCallback->extendedAttributes->{IsWeakCallback};
push(@$contentRef, " ${callbackDataType}* m_data;\n");
Modified: trunk/Source/WebCore/dom/MutationCallback.h (224404 => 224405)
--- trunk/Source/WebCore/dom/MutationCallback.h 2017-11-03 16:59:30 UTC (rev 224404)
+++ trunk/Source/WebCore/dom/MutationCallback.h 2017-11-03 17:10:42 UTC (rev 224405)
@@ -45,6 +45,8 @@
public:
using ActiveDOMCallback::ActiveDOMCallback;
+ virtual bool hasCallback() const = 0;
+
virtual CallbackResult<void> handleEvent(MutationObserver&, const Vector<Ref<MutationRecord>>&, MutationObserver&) = 0;
};
Modified: trunk/Source/WebCore/dom/MutationObserver.cpp (224404 => 224405)
--- trunk/Source/WebCore/dom/MutationObserver.cpp 2017-11-03 16:59:30 UTC (rev 224404)
+++ trunk/Source/WebCore/dom/MutationObserver.cpp 2017-11-03 17:10:42 UTC (rev 224405)
@@ -232,7 +232,9 @@
Vector<Ref<MutationRecord>> records;
records.swap(m_records);
- m_callback->handleEvent(*this, records, *this);
+ // FIXME: Keep mutation observer callback as long as its observed nodes are alive. See https://webkit.org/b/179224.
+ if (m_callback->hasCallback())
+ m_callback->handleEvent(*this, records, *this);
}
void MutationObserver::notifyMutationObservers()
Modified: trunk/Source/WebCore/dom/MutationObserverInterestGroup.cpp (224404 => 224405)
--- trunk/Source/WebCore/dom/MutationObserverInterestGroup.cpp 2017-11-03 16:59:30 UTC (rev 224404)
+++ trunk/Source/WebCore/dom/MutationObserverInterestGroup.cpp 2017-11-03 17:10:42 UTC (rev 224405)
@@ -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: trunk/Source/WebCore/dom/MutationObserverInterestGroup.h (224404 => 224405)
--- trunk/Source/WebCore/dom/MutationObserverInterestGroup.h 2017-11-03 16:59:30 UTC (rev 224404)
+++ trunk/Source/WebCore/dom/MutationObserverInterestGroup.h 2017-11-03 17:10:42 UTC (rev 224405)
@@ -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: trunk/Source/WebCore/dom/NativeNodeFilter.cpp (224404 => 224405)
--- trunk/Source/WebCore/dom/NativeNodeFilter.cpp 2017-11-03 16:59:30 UTC (rev 224404)
+++ trunk/Source/WebCore/dom/NativeNodeFilter.cpp 2017-11-03 17:10:42 UTC (rev 224405)
@@ -34,6 +34,11 @@
{
}
+bool NativeNodeFilter::hasCallback() const
+{
+ return true;
+}
+
CallbackResult<unsigned short> NativeNodeFilter::acceptNode(Node& node)
{
return m_condition->acceptNode(node);
Modified: trunk/Source/WebCore/dom/NativeNodeFilter.h (224404 => 224405)
--- trunk/Source/WebCore/dom/NativeNodeFilter.h 2017-11-03 16:59:30 UTC (rev 224404)
+++ trunk/Source/WebCore/dom/NativeNodeFilter.h 2017-11-03 17:10:42 UTC (rev 224405)
@@ -43,6 +43,8 @@
private:
WEBCORE_EXPORT explicit NativeNodeFilter(ScriptExecutionContext*, Ref<NodeFilterCondition>&&);
+ bool hasCallback() const final;
+
Ref<NodeFilterCondition> m_condition;
};
Modified: trunk/Source/WebCore/dom/Node.cpp (224404 => 224405)
--- trunk/Source/WebCore/dom/Node.cpp 2017-11-03 16:59:30 UTC (rev 224404)
+++ trunk/Source/WebCore/dom/Node.cpp 2017-11-03 17:10:42 UTC (rev 224405)
@@ -2202,7 +2202,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;
@@ -2210,7 +2210,7 @@
for (auto& registration : *registry) {
if (registration->shouldReceiveMutationFrom(target, type, attributeName)) {
auto deliveryOptions = registration->deliveryOptions();
- auto result = observers.add(®istration->observer(), deliveryOptions);
+ auto result = observers.add(registration->observer(), deliveryOptions);
if (!result.isNewEntry)
result.iterator->value |= deliveryOptions;
}
@@ -2217,9 +2217,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: trunk/Source/WebCore/dom/Node.h (224404 => 224405)
--- trunk/Source/WebCore/dom/Node.h 2017-11-03 16:59:30 UTC (rev 224404)
+++ trunk/Source/WebCore/dom/Node.h 2017-11-03 17:10:42 UTC (rev 224405)
@@ -533,7 +533,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&);
Modified: trunk/Source/WebCore/dom/NodeFilter.h (224404 => 224405)
--- trunk/Source/WebCore/dom/NodeFilter.h 2017-11-03 16:59:30 UTC (rev 224404)
+++ trunk/Source/WebCore/dom/NodeFilter.h 2017-11-03 17:10:42 UTC (rev 224405)
@@ -38,6 +38,8 @@
virtual CallbackResult<unsigned short> acceptNode(Node&) = 0;
+ virtual bool hasCallback() const = 0;
+
/*
* The following constants are returned by the acceptNode()
* method:
