Diff
Modified: trunk/LayoutTests/ChangeLog (135227 => 135228)
--- trunk/LayoutTests/ChangeLog 2012-11-20 02:11:53 UTC (rev 135227)
+++ trunk/LayoutTests/ChangeLog 2012-11-20 02:16:33 UTC (rev 135228)
@@ -1,3 +1,20 @@
+2012-11-19 Adam Klein <ad...@chromium.org>
+
+ MutationObserver wrapper should not be collected while still observing
+ https://bugs.webkit.org/show_bug.cgi?id=102328
+
+ Reviewed by Adam Barth.
+
+ Tests showing that observers are kept alive, both in the simple case
+ and in the transient registered observer case (where the original
+ registration node is GCed but the transient observation node is still
+ alive).
+
+ * fast/mutation/observer-wrapper-dropoff-expected.txt: Added.
+ * fast/mutation/observer-wrapper-dropoff-transient-expected.txt: Added.
+ * fast/mutation/observer-wrapper-dropoff-transient.html: Added.
+ * fast/mutation/observer-wrapper-dropoff.html: Added.
+
2012-11-19 Tony Chang <t...@chromium.org>
Move more non-settings out of InternalSettings
Added: trunk/LayoutTests/fast/mutation/observer-wrapper-dropoff-expected.txt (0 => 135228)
--- trunk/LayoutTests/fast/mutation/observer-wrapper-dropoff-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/mutation/observer-wrapper-dropoff-expected.txt 2012-11-20 02:16:33 UTC (rev 135228)
@@ -0,0 +1,10 @@
+MutationObserver wrappers should survive GC for passing into the callback even if JS has lost references.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS observer.testProperty is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/mutation/observer-wrapper-dropoff-transient-expected.txt (0 => 135228)
--- trunk/LayoutTests/fast/mutation/observer-wrapper-dropoff-transient-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/mutation/observer-wrapper-dropoff-transient-expected.txt 2012-11-20 02:16:33 UTC (rev 135228)
@@ -0,0 +1,10 @@
+MutationObserver wrappers should survive GC for passing into the callback even if JS has lost references and the only remaining observations are transient.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS observer.testProperty is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/mutation/observer-wrapper-dropoff-transient.html (0 => 135228)
--- trunk/LayoutTests/fast/mutation/observer-wrapper-dropoff-transient.html (rev 0)
+++ trunk/LayoutTests/fast/mutation/observer-wrapper-dropoff-transient.html 2012-11-20 02:16:33 UTC (rev 135228)
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+
+<script src=""
+
+<script>
+description('MutationObserver wrappers should survive GC for passing into the callback even if JS has lost references and the only remaining observations are transient.');
+
+jsTestIsAsync = true;
+
+function addObserver(node, fn) {
+ var observer = new WebKitMutationObserver(fn);
+ observer.testProperty = true;
+ observer.observe(node, {attributes:true, subtree: true});
+}
+
+_onload_ = function() {
+ var root = document.createElement('div');
+ var child = root.appendChild(document.createElement('span'));
+ addObserver(root, function(records, observer) {
+ window.observer = observer;
+ shouldBe('observer.testProperty', 'true');
+ finishJSTest();
+ });
+
+ root.removeChild(child);
+ child.setAttribute('foo', 'bar');
+ root = null;
+
+ gc();
+};
+</script>
+
+<script src=""
Added: trunk/LayoutTests/fast/mutation/observer-wrapper-dropoff.html (0 => 135228)
--- trunk/LayoutTests/fast/mutation/observer-wrapper-dropoff.html (rev 0)
+++ trunk/LayoutTests/fast/mutation/observer-wrapper-dropoff.html 2012-11-20 02:16:33 UTC (rev 135228)
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+
+<script src=""
+
+<script>
+description('MutationObserver wrappers should survive GC for passing into the callback even if JS has lost references.');
+
+jsTestIsAsync = true;
+
+function addObserver(node, fn) {
+ var observer = new WebKitMutationObserver(fn);
+ observer.testProperty = true;
+ observer.observe(node, {attributes:true});
+}
+
+_onload_ = function() {
+ addObserver(document.body, function(records, observer) {
+ window.observer = observer;
+ shouldBe('observer.testProperty', 'true');
+ finishJSTest();
+ });
+
+ gc();
+
+ document.body.setAttribute('touch', 'the node');
+};
+</script>
+
+<script src=""
Modified: trunk/LayoutTests/platform/efl/TestExpectations (135227 => 135228)
--- trunk/LayoutTests/platform/efl/TestExpectations 2012-11-20 02:11:53 UTC (rev 135227)
+++ trunk/LayoutTests/platform/efl/TestExpectations 2012-11-20 02:16:33 UTC (rev 135228)
@@ -1713,3 +1713,6 @@
webkit.org/b/102367 fast/forms/zoomed-controls.html [ ImageOnlyFailure Missing ]
webkit.org/b/102493 fast/events/mouse-cursor.html [ Failure ]
+
+webkit.org/b/102744 fast/mutation/observer-wrapper-dropoff.html [ Skip ]
+webkit.org/b/102744 fast/mutation/observer-wrapper-dropoff-transient.html [ Skip ]
Modified: trunk/LayoutTests/platform/gtk/TestExpectations (135227 => 135228)
--- trunk/LayoutTests/platform/gtk/TestExpectations 2012-11-20 02:11:53 UTC (rev 135227)
+++ trunk/LayoutTests/platform/gtk/TestExpectations 2012-11-20 02:16:33 UTC (rev 135228)
@@ -1387,6 +1387,9 @@
webkit.org/b/102586 media/video-src-blob.html [ Failure ]
+webkit.org/b/102744 fast/mutation/observer-wrapper-dropoff.html [ Skip ]
+webkit.org/b/102744 fast/mutation/observer-wrapper-dropoff-transient.html [ Skip ]
+
#////////////////////////////////////////////////////////////////////////////////////////
# End of Tests failing
#////////////////////////////////////////////////////////////////////////////////////////
Modified: trunk/LayoutTests/platform/mac/TestExpectations (135227 => 135228)
--- trunk/LayoutTests/platform/mac/TestExpectations 2012-11-20 02:11:53 UTC (rev 135227)
+++ trunk/LayoutTests/platform/mac/TestExpectations 2012-11-20 02:16:33 UTC (rev 135228)
@@ -1193,3 +1193,6 @@
# Hangs safari apparently due to an issue with weird multi-frame CUR image files
webkit.org/b/101811 fast/events/mouse-cursor-multiframecur.html [ Skip ]
+
+webkit.org/b/102744 fast/mutation/observer-wrapper-dropoff.html [ Skip ]
+webkit.org/b/102744 fast/mutation/observer-wrapper-dropoff-transient.html [ Skip ]
Modified: trunk/LayoutTests/platform/qt/TestExpectations (135227 => 135228)
--- trunk/LayoutTests/platform/qt/TestExpectations 2012-11-20 02:11:53 UTC (rev 135227)
+++ trunk/LayoutTests/platform/qt/TestExpectations 2012-11-20 02:16:33 UTC (rev 135228)
@@ -2386,3 +2386,6 @@
# [Qt] fast/block/float/overhanging-tall-block.html asserts after r135025
webkit.org/b/102675 [ Debug ] fast/block/float/overhanging-tall-block.html
+
+webkit.org/b/102744 fast/mutation/observer-wrapper-dropoff.html [ Skip ]
+webkit.org/b/102744 fast/mutation/observer-wrapper-dropoff-transient.html [ Skip ]
Modified: trunk/LayoutTests/platform/win/TestExpectations (135227 => 135228)
--- trunk/LayoutTests/platform/win/TestExpectations 2012-11-20 02:11:53 UTC (rev 135227)
+++ trunk/LayoutTests/platform/win/TestExpectations 2012-11-20 02:16:33 UTC (rev 135228)
@@ -2430,3 +2430,6 @@
fast/css3-text/css3-text-align-last/getComputedStyle/getComputedStyle-text-align-last-inherited.html
fast/css3-text/css3-text-align-last/getComputedStyle/getComputedStyle-text-align-last.html
fast/css3-text/css3-text-decoration/getComputedStyle/getComputedStyle-text-decoration-line.html
+
+webkit.org/b/102744 fast/mutation/observer-wrapper-dropoff.html [ Skip ]
+webkit.org/b/102744 fast/mutation/observer-wrapper-dropoff-transient.html [ Skip ]
Modified: trunk/LayoutTests/platform/wincairo/TestExpectations (135227 => 135228)
--- trunk/LayoutTests/platform/wincairo/TestExpectations 2012-11-20 02:11:53 UTC (rev 135227)
+++ trunk/LayoutTests/platform/wincairo/TestExpectations 2012-11-20 02:16:33 UTC (rev 135228)
@@ -2961,3 +2961,6 @@
fast/css3-text/css3-text-align-last/getComputedStyle/getComputedStyle-text-align-last-inherited.html
fast/css3-text/css3-text-align-last/getComputedStyle/getComputedStyle-text-align-last.html
fast/css3-text/css3-text-decoration/getComputedStyle/getComputedStyle-text-decoration-line.html
+
+webkit.org/b/102744 fast/mutation/observer-wrapper-dropoff.html [ Skip ]
+webkit.org/b/102744 fast/mutation/observer-wrapper-dropoff-transient.html [ Skip ]
Modified: trunk/Source/WebCore/ChangeLog (135227 => 135228)
--- trunk/Source/WebCore/ChangeLog 2012-11-20 02:11:53 UTC (rev 135227)
+++ trunk/Source/WebCore/ChangeLog 2012-11-20 02:16:33 UTC (rev 135228)
@@ -1,3 +1,32 @@
+2012-11-19 Adam Klein <ad...@chromium.org>
+
+ MutationObserver wrapper should not be collected while still observing
+ https://bugs.webkit.org/show_bug.cgi?id=102328
+
+ Reviewed by Adam Barth.
+
+ Use the new opaqueRootForGC helper in V8GCController to put each
+ MutationObserver wrapper in the same object group as the nodes it's
+ observing.
+
+ Only includes V8 impl for now, JSC impl coming soon.
+
+ Tests: fast/mutation/observer-wrapper-dropoff-transient.html
+ fast/mutation/observer-wrapper-dropoff.html
+
+ * bindings/v8/V8GCController.cpp: Add custom code for MutationObserver
+ with a FIXME to move this out once we update the opaque roots API.
+ * dom/MutationObserver.cpp:
+ (WebCore::MutationObserver::getObservedNodes): Plumbing to expose the observed nodes
+ to the GC controller.
+ (WebCore):
+ * dom/MutationObserver.h:
+ * dom/MutationObserverRegistration.cpp:
+ (WebCore::MutationObserverRegistration::addRegistrationNodesToSet): More plumbing.
+ (WebCore):
+ * dom/MutationObserverRegistration.h:
+ (MutationObserverRegistration):
+
2012-11-19 Tony Chang <t...@chromium.org>
Move more non-settings out of InternalSettings
Modified: trunk/Source/WebCore/bindings/v8/V8GCController.cpp (135227 => 135228)
--- trunk/Source/WebCore/bindings/v8/V8GCController.cpp 2012-11-20 02:11:53 UTC (rev 135227)
+++ trunk/Source/WebCore/bindings/v8/V8GCController.cpp 2012-11-20 02:16:33 UTC (rev 135228)
@@ -38,6 +38,7 @@
#include "V8AbstractEventListener.h"
#include "V8Binding.h"
#include "V8MessagePort.h"
+#include "V8MutationObserver.h"
#include "V8Node.h"
#include "V8RecursionScope.h"
#include "WrapperTypeInfo.h"
@@ -176,6 +177,14 @@
MessagePort* port = static_cast<MessagePort*>(object);
if (port->isEntangled() || port->hasPendingActivity())
m_grouper.keepAlive(wrapper);
+#if ENABLE(MUTATION_OBSERVERS)
+ } else if (V8MutationObserver::info.equals(type)) {
+ // FIXME: Allow opaqueRootForGC to operate on multiple roots and move this logic into V8MutationObserverCustom.
+ MutationObserver* observer = static_cast<MutationObserver*>(object);
+ HashSet<Node*> observedNodes = observer->getObservedNodes();
+ for (HashSet<Node*>::iterator it = observedNodes.begin(); it != observedNodes.end(); ++it)
+ m_grouper.addToGroup(V8GCController::opaqueRootForGC(*it), wrapper);
+#endif // ENABLE(MUTATION_OBSERVERS)
} else {
ActiveDOMObject* activeDOMObject = type->toActiveDOMObject(wrapper);
if (activeDOMObject && activeDOMObject->hasPendingActivity())
Modified: trunk/Source/WebCore/dom/MutationObserver.cpp (135227 => 135228)
--- trunk/Source/WebCore/dom/MutationObserver.cpp 2012-11-20 02:11:53 UTC (rev 135227)
+++ trunk/Source/WebCore/dom/MutationObserver.cpp 2012-11-20 02:16:33 UTC (rev 135228)
@@ -167,6 +167,14 @@
activeMutationObservers().add(this);
}
+HashSet<Node*> MutationObserver::getObservedNodes() const
+{
+ HashSet<Node*> observedNodes;
+ for (HashSet<MutationObserverRegistration*>::const_iterator iter = m_registrations.begin(); iter != m_registrations.end(); ++iter)
+ (*iter)->addRegistrationNodesToSet(observedNodes);
+ return observedNodes;
+}
+
void MutationObserver::deliver()
{
// Calling clearTransientRegistrations() can modify m_registrations, so it's necessary
Modified: trunk/Source/WebCore/dom/MutationObserver.h (135227 => 135228)
--- trunk/Source/WebCore/dom/MutationObserver.h 2012-11-20 02:11:53 UTC (rev 135227)
+++ trunk/Source/WebCore/dom/MutationObserver.h 2012-11-20 02:16:33 UTC (rev 135228)
@@ -87,6 +87,8 @@
void enqueueMutationRecord(PassRefPtr<MutationRecord>);
void setHasTransientRegistration();
+ HashSet<Node*> getObservedNodes() const;
+
private:
struct ObserverLessThan;
Modified: trunk/Source/WebCore/dom/MutationObserverRegistration.cpp (135227 => 135228)
--- trunk/Source/WebCore/dom/MutationObserverRegistration.cpp 2012-11-20 02:11:53 UTC (rev 135227)
+++ trunk/Source/WebCore/dom/MutationObserverRegistration.cpp 2012-11-20 02:16:33 UTC (rev 135228)
@@ -124,6 +124,15 @@
return m_attributeFilter.contains(attributeName->localName());
}
+void MutationObserverRegistration::addRegistrationNodesToSet(HashSet<Node*>& nodes) const
+{
+ nodes.add(m_registrationNode);
+ if (!m_transientRegistrationNodes)
+ return;
+ for (NodeHashSet::const_iterator iter = m_transientRegistrationNodes->begin(); iter != m_transientRegistrationNodes->end(); ++iter)
+ nodes.add(iter->get());
+}
+
} // namespace WebCore
#endif // ENABLE(MUTATION_OBSERVERS)
Modified: trunk/Source/WebCore/dom/MutationObserverRegistration.h (135227 => 135228)
--- trunk/Source/WebCore/dom/MutationObserverRegistration.h 2012-11-20 02:11:53 UTC (rev 135227)
+++ trunk/Source/WebCore/dom/MutationObserverRegistration.h 2012-11-20 02:16:33 UTC (rev 135228)
@@ -60,6 +60,8 @@
MutationRecordDeliveryOptions deliveryOptions() const { return m_options & (MutationObserver::AttributeOldValue | MutationObserver::CharacterDataOldValue); }
MutationObserverOptions mutationTypes() const { return m_options & MutationObserver::AllMutationTypes; }
+ void addRegistrationNodesToSet(HashSet<Node*>&) const;
+
private:
MutationObserverRegistration(PassRefPtr<MutationObserver>, Node*, MutationObserverOptions, const HashSet<AtomicString>& attributeFilter);
