Attachment 174350 Details for Bug 102328 – Patch

[patch] Patch

bug-102328-20121115012401.patch (text/plain), 9.85 KB, created by Elliott Sprehn on 2012-11-14 22:26:04 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Elliott Sprehn

Created: 2012-11-14 22:26:04 PST

Size: 9.85 KB

patch

obsolete

>Subversion Revision: 134738
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 694d6f64f0639d063e1f8b8f34231ea2d149be3e..f31b0f6b66f12fd112423db33877c68c3e4e68d4 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,30 @@
>+2012-11-14  Elliott Sprehn  <esprehn@chromium.org>
>+
>+        MutationObserver wrapper should not be collected while still observing
>+        https://bugs.webkit.org/show_bug.cgi?id=102328
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Make MutationObserver an ActiveDOMObject so that the wrapper is not
>+        collected while it is still observing the DOM. This is needed because
>+        the wrapper is passed into the callback and expandos on the wrapper
>+        should be preserved.
>+
>+        Test: fast/mutation/observer-wrapper-dropoff.html
>+
>+        * bindings/js/JSMutationObserverCustom.cpp:
>+        (WebCore::JSMutationObserverConstructor::constructJSMutationObserver):
>+        * bindings/v8/custom/V8MutationObserverCustom.cpp:
>+        (WebCore::V8MutationObserver::constructorCallback):
>+        * dom/MutationObserver.cpp:
>+        (WebCore::MutationObserver::create):
>+        (WebCore::MutationObserver::MutationObserver):
>+        (WebCore::MutationObserver::observationStarted):
>+        (WebCore::MutationObserver::observationEnded):
>+        * dom/MutationObserver.h:
>+        (WebCore):
>+        * dom/MutationObserver.idl:
>+
> 2012-11-14  Kentaro Hara  <haraken@chromium.org>
> 
>         Unreviewed. Rebaselined run-bindings-tests.
>diff --git a/Source/WebCore/bindings/js/JSMutationObserverCustom.cpp b/Source/WebCore/bindings/js/JSMutationObserverCustom.cpp
>index ab5560d80cc97c4301c7f50ae1408417fe0cb0b2..d6784515de082d06f47a88ce9fb07f6237495e66 100644
>--- a/Source/WebCore/bindings/js/JSMutationObserverCustom.cpp
>+++ b/Source/WebCore/bindings/js/JSMutationObserverCustom.cpp
>@@ -54,8 +54,11 @@ EncodedJSValue JSC_HOST_CALL JSMutationObserverConstructor::constructJSMutationO
>     }
> 
>     JSMutationObserverConstructor* jsConstructor = jsCast<JSMutationObserverConstructor*>(exec->callee());
>-    RefPtr<MutationCallback> callback = JSMutationCallback::create(object, jsConstructor->globalObject());
>-    return JSValue::encode(asObject(toJS(exec, jsConstructor->globalObject(), MutationObserver::create(callback.release()))));
>+    RefPtr<JSMutationCallback> callback = JSMutationCallback::create(object, jsConstructor->globalObject());
>+    RefPtr<MutationObserver> observer = MutationObserver::create(callback, jsConstructor->globalObject()->scriptExecutionContext());
>+    JSObject* wrapper = asObject(toJS(exec, jsConstructor->globalObject(), observer.release()));
>+
>+    return JSValue::encode(wrapper);
> }
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/bindings/v8/custom/V8MutationObserverCustom.cpp b/Source/WebCore/bindings/v8/custom/V8MutationObserverCustom.cpp
>index f80ce598a972acb7d5fc2f349968f4b9ea8f2bf6..5e7aeda10e65d4eb891c02ef5700c1a29a1fabb8 100644
>--- a/Source/WebCore/bindings/v8/custom/V8MutationObserverCustom.cpp
>+++ b/Source/WebCore/bindings/v8/custom/V8MutationObserverCustom.cpp
>@@ -63,7 +63,7 @@ v8::Handle<v8::Value> V8MutationObserver::constructorCallback(const v8::Argument
>     ScriptExecutionContext* context = getScriptExecutionContext();
> 
>     RefPtr<MutationCallback> callback = V8MutationCallback::create(arg, context);
>-    RefPtr<MutationObserver> observer = MutationObserver::create(callback.release());
>+    RefPtr<MutationObserver> observer = MutationObserver::create(callback.release(), context);
> 
>     v8::Handle<v8::Object> wrapper = args.Holder();
>     V8DOMWrapper::createDOMWrapper(observer.release(), &info, wrapper);
>diff --git a/Source/WebCore/dom/MutationObserver.cpp b/Source/WebCore/dom/MutationObserver.cpp
>index 0c210fa46bada69263c4ca8b01d925bcf89a34d9..b8c0b924806e62862ed66e702045d01e7dd672c4 100644
>--- a/Source/WebCore/dom/MutationObserver.cpp
>+++ b/Source/WebCore/dom/MutationObserver.cpp
>@@ -57,14 +57,17 @@ struct MutationObserver::ObserverLessThan {
>     }
> };
> 
>-PassRefPtr<MutationObserver> MutationObserver::create(PassRefPtr<MutationCallback> callback)
>+PassRefPtr<MutationObserver> MutationObserver::create(PassRefPtr<MutationCallback> callback, ScriptExecutionContext* context)
> {
>     ASSERT(isMainThread());
>-    return adoptRef(new MutationObserver(callback));
>+    RefPtr<MutationObserver> observer = adoptRef(new MutationObserver(callback, context));
>+    observer->suspendIfNeeded();
>+    return observer.release();
> }
> 
>-MutationObserver::MutationObserver(PassRefPtr<MutationCallback> callback)
>-    : m_callback(callback)
>+MutationObserver::MutationObserver(PassRefPtr<MutationCallback> callback, ScriptExecutionContext* context)
>+    : ActiveDOMObject(context, this)
>+    , m_callback(callback)
>     , m_priority(s_observerPriority++)
> {
> }
>@@ -138,12 +141,14 @@ void MutationObserver::observationStarted(MutationObserverRegistration* registra
> {
>     ASSERT(!m_registrations.contains(registration));
>     m_registrations.add(registration);
>+    setPendingActivity(this);
> }
> 
> void MutationObserver::observationEnded(MutationObserverRegistration* registration)
> {
>     ASSERT(m_registrations.contains(registration));
>     m_registrations.remove(registration);
>+    unsetPendingActivity(this);
> }
> 
> typedef HashSet<RefPtr<MutationObserver> > MutationObserverSet;
>diff --git a/Source/WebCore/dom/MutationObserver.h b/Source/WebCore/dom/MutationObserver.h
>index fadb378d1efdebd1cbce3247ed9f9fb103773401..05ab7ef3c915f98ad1c0bb34823c3cac39b853f4 100644
>--- a/Source/WebCore/dom/MutationObserver.h
>+++ b/Source/WebCore/dom/MutationObserver.h
>@@ -33,6 +33,7 @@
> 
> #if ENABLE(MUTATION_OBSERVERS)
> 
>+#include "ActiveDOMObject.h"
> #include <wtf/HashMap.h>
> #include <wtf/HashSet.h>
> #include <wtf/PassRefPtr.h>
>@@ -48,13 +49,14 @@ class MutationCallback;
> class MutationObserverRegistration;
> class MutationRecord;
> class Node;
>+class ScriptExecutionContext;
> 
> typedef int ExceptionCode;
> 
> typedef unsigned char MutationObserverOptions;
> typedef unsigned char MutationRecordDeliveryOptions;
> 
>-class MutationObserver : public RefCounted<MutationObserver> {
>+class MutationObserver : public RefCounted<MutationObserver>, public ActiveDOMObject {
> public:
>     enum MutationType {
>         ChildList = 1 << 0,
>@@ -74,10 +76,10 @@ public:
>         CharacterDataOldValue = 1 << 6,
>     };
> 
>-    static PassRefPtr<MutationObserver> create(PassRefPtr<MutationCallback>);
>+    static PassRefPtr<MutationObserver> create(PassRefPtr<MutationCallback>, ScriptExecutionContext*);
>     static void deliverAllMutations();
> 
>-    ~MutationObserver();
>+    virtual ~MutationObserver();
> 
>     void observe(Node*, const Dictionary&, ExceptionCode&);
>     Vector<RefPtr<MutationRecord> > takeRecords();
>@@ -90,7 +92,7 @@ public:
> private:
>     struct ObserverLessThan;
> 
>-    explicit MutationObserver(PassRefPtr<MutationCallback>);
>+    MutationObserver(PassRefPtr<MutationCallback>, ScriptExecutionContext*);
>     void deliver();
> 
>     static bool validateOptions(MutationObserverOptions);
>diff --git a/Source/WebCore/dom/MutationObserver.idl b/Source/WebCore/dom/MutationObserver.idl
>index 1174f29eafc0d167d9de67075e9d0fc6c62b72a6..286e0631be7b03c02bb2bbeb08fc08b854411444 100644
>--- a/Source/WebCore/dom/MutationObserver.idl
>+++ b/Source/WebCore/dom/MutationObserver.idl
>@@ -29,6 +29,7 @@
>  */
> 
> [
>+    ActiveDOMObject,
>     Conditional=MUTATION_OBSERVERS,
>     CustomConstructor,
>     ConstructorParameters=1
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index b95c55916c9c720ab680175f7bcc01e249a34d06..076e47bcc45b49bc908cb824c698d32c34780ede 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,17 @@
>+2012-11-14  Elliott Sprehn  <esprehn@chromium.org>
>+
>+        MutationObserver wrapper should not be collected while still observing
>+        https://bugs.webkit.org/show_bug.cgi?id=102328
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Test that a MutationObserver wrapper is not collected while the observer
>+        is still observing since the wrapper must be passed into the callback
>+        later.
>+
>+        * fast/mutation/observer-wrapper-dropoff-expected.txt: Added.
>+        * fast/mutation/observer-wrapper-dropoff.html: Added.
>+
> 2012-11-14  Dirk Schulze  <krit@webkit.org>
> 
>         Support animation of basic shape 'polygon'
>diff --git a/LayoutTests/fast/mutation/observer-wrapper-dropoff-expected.txt b/LayoutTests/fast/mutation/observer-wrapper-dropoff-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..c8c0f2e9dcc64165dd82a7d124c5f329ec3be93c
>--- /dev/null
>+++ b/LayoutTests/fast/mutation/observer-wrapper-dropoff-expected.txt
>@@ -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
>+
>diff --git a/LayoutTests/fast/mutation/observer-wrapper-dropoff.html b/LayoutTests/fast/mutation/observer-wrapper-dropoff.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..c5dbcbbf095edf4450f456ce13a6363673ce14b8
>--- /dev/null
>+++ b/LayoutTests/fast/mutation/observer-wrapper-dropoff.html
>@@ -0,0 +1,29 @@
>+<!DOCTYPE html>
>+
>+<script src="../js/resources/js-test-pre.js"></script>
>+
>+<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="../js/resources/js-test-post.js"></script>

Actions: View | Formatted Diff | Diff

Attachments on bug 102328: 174338 | 174350 | 174483 | 175083 | 175088