Change JSEventListener::m_jsFunction to be a weak ref.
authormark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 15 Nov 2012 01:01:41 +0000 (01:01 +0000)
committermark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 15 Nov 2012 01:01:41 +0000 (01:01 +0000)
https://bugs.webkit.org/show_bug.cgi?id=101989.

Reviewed by Geoffrey Garen.

Source/JavaScriptCore:

Added infrastructure for scanning weak ref slots.

* heap/SlotVisitor.cpp: Added #include "SlotVisitorInlines.h".
* heap/SlotVisitor.h:
(SlotVisitor): Added SlotVisitor::appendUnbarrieredWeak().
* heap/SlotVisitorInlines.h: Added #include "Weak.h".
(JSC::SlotVisitor::appendUnbarrieredWeak): Added.
* heap/Weak.h:
(JSC::operator==): Added operator==() for Weak.
* runtime/JSCell.h: Removed #include "SlotVisitorInlines.h".
* runtime/JSObject.h: Added #include "SlotVisitorInlines.h".

Source/WebCore:

No new tests.

* ForwardingHeaders/heap/SlotVisitor.h: Added.
* bindings/js/JSDOMBinding.h: Added #include <heap/SlotVisitor.h>
* bindings/js/JSEventListener.cpp:
(WebCore::JSEventListener::JSEventListener):
(WebCore::JSEventListener::visitJSFunction):
(WebCore::JSEventListener::operator==):
 - Removed the m_wrapper checks in operator==() because they are not
   needed. There is no longer any threat of m_jsFunction pointing to
   recycled memory. The use of weak refs will ensure that m_jsFunction
   is either still holding on to its old memory exclusively, or is 0'ed
   out when the GC collects it.
* bindings/js/JSEventListener.h:
(JSEventListener):
(WebCore::JSEventListener::jsFunction):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@134697 268f45cc-cd09-0410-ab3c-d52691b4dbfc

12 files changed:
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/heap/SlotVisitor.cpp
Source/JavaScriptCore/heap/SlotVisitor.h
Source/JavaScriptCore/heap/SlotVisitorInlines.h
Source/JavaScriptCore/heap/Weak.h
Source/JavaScriptCore/runtime/JSCell.h
Source/JavaScriptCore/runtime/JSObject.h
Source/WebCore/ChangeLog
Source/WebCore/ForwardingHeaders/heap/SlotVisitor.h [new file with mode: 0644]
Source/WebCore/bindings/js/JSDOMBinding.h
Source/WebCore/bindings/js/JSEventListener.cpp
Source/WebCore/bindings/js/JSEventListener.h

index 38a9893..33b3957 100644 (file)
@@ -1,3 +1,22 @@
+2012-11-14  Mark Lam  <mark.lam@apple.com>
+
+        Change JSEventListener::m_jsFunction to be a weak ref.
+        https://bugs.webkit.org/show_bug.cgi?id=101989.
+
+        Reviewed by Geoffrey Garen.
+
+        Added infrastructure for scanning weak ref slots.
+
+        * heap/SlotVisitor.cpp: Added #include "SlotVisitorInlines.h".
+        * heap/SlotVisitor.h:
+        (SlotVisitor): Added SlotVisitor::appendUnbarrieredWeak().
+        * heap/SlotVisitorInlines.h: Added #include "Weak.h".
+        (JSC::SlotVisitor::appendUnbarrieredWeak): Added.
+        * heap/Weak.h:
+        (JSC::operator==): Added operator==() for Weak.
+        * runtime/JSCell.h: Removed #include "SlotVisitorInlines.h".
+        * runtime/JSObject.h: Added #include "SlotVisitorInlines.h".
+
 2012-11-14  Filip Pizlo  <fpizlo@apple.com>
 
         Read-only properties created with putDirect() should tell the structure that there are read-only properties
index 35af820..cff3615 100644 (file)
@@ -1,5 +1,6 @@
 #include "config.h"
 #include "SlotVisitor.h"
+#include "SlotVisitorInlines.h"
 
 #include "ConservativeRoots.h"
 #include "CopiedSpace.h"
index 34b1bc8..53c7de6 100644 (file)
@@ -36,6 +36,7 @@ namespace JSC {
 class ConservativeRoots;
 class GCThreadSharedData;
 class Heap;
+template<typename T> class Weak;
 template<typename T> class WriteBarrierBase;
 template<typename T> class JITWriteBarrier;
 
@@ -56,6 +57,8 @@ public:
     template<typename T>
     void appendUnbarrieredPointer(T**);
     void appendUnbarrieredValue(JSValue*);
+    template<typename T>
+    void appendUnbarrieredWeak(Weak<T>*);
     
     void addOpaqueRoot(void*);
     bool containsOpaqueRoot(void*);
index b0f30b6..ea8126f 100644 (file)
@@ -29,6 +29,7 @@
 #include "CopiedSpaceInlines.h"
 #include "Options.h"
 #include "SlotVisitor.h"
+#include "Weak.h"
 
 namespace JSC {
 
@@ -66,6 +67,14 @@ ALWAYS_INLINE void SlotVisitor::append(JSCell** slot)
     internalAppend(*slot);
 }
 
+template<typename T>
+ALWAYS_INLINE void SlotVisitor::appendUnbarrieredWeak(Weak<T>* weak)
+{
+    ASSERT(weak);
+    if (weak->get())
+        internalAppend(weak->get());
+}
+
 ALWAYS_INLINE void SlotVisitor::internalAppend(JSValue value)
 {
     if (!value || !value.isCell())
index 3c3d1d0..efb2a9a 100644 (file)
@@ -151,6 +151,11 @@ template<typename T> inline WeakImpl* Weak<T>::hashTableDeletedValue()
     return reinterpret_cast<WeakImpl*>(-1);
 }
 
+template <typename T> inline bool operator==(const Weak<T>& lhs, const Weak<T>& rhs)
+{
+    return lhs.get() == rhs.get();
+}
+
 // This function helps avoid modifying a weak table while holding an iterator into it. (Object allocation
 // can run a finalizer that modifies the table. We avoid that by requiring a pre-constructed object as our value.)
 template<typename Map, typename Key, typename Value> inline void weakAdd(Map& map, const Key& key, Value value)
index df31b85..b28fedd 100644 (file)
@@ -30,7 +30,6 @@
 #include "JSLock.h"
 #include "JSValueInlines.h"
 #include "SlotVisitor.h"
-#include "SlotVisitorInlines.h"
 #include "TypedArrayDescriptor.h"
 #include "WriteBarrier.h"
 #include <wtf/Noncopyable.h>
index da8c6cc..4f7f470 100644 (file)
@@ -39,6 +39,7 @@
 #include "Structure.h"
 #include "JSGlobalData.h"
 #include "JSString.h"
+#include "SlotVisitorInlines.h"
 #include "SparseArrayValueMap.h"
 #include <wtf/StdLibExtras.h>
 
index dc11f0a..40b452c 100644 (file)
@@ -1,3 +1,27 @@
+2012-11-14  Mark Lam  <mark.lam@apple.com>
+
+        Change JSEventListener::m_jsFunction to be a weak ref.
+        https://bugs.webkit.org/show_bug.cgi?id=101989.
+
+        Reviewed by Geoffrey Garen.
+
+        No new tests.
+
+        * ForwardingHeaders/heap/SlotVisitor.h: Added.
+        * bindings/js/JSDOMBinding.h: Added #include <heap/SlotVisitor.h>
+        * bindings/js/JSEventListener.cpp:
+        (WebCore::JSEventListener::JSEventListener):
+        (WebCore::JSEventListener::visitJSFunction):
+        (WebCore::JSEventListener::operator==):
+         - Removed the m_wrapper checks in operator==() because they are not
+           needed. There is no longer any threat of m_jsFunction pointing to
+           recycled memory. The use of weak refs will ensure that m_jsFunction
+           is either still holding on to its old memory exclusively, or is 0'ed
+           out when the GC collects it.
+        * bindings/js/JSEventListener.h:
+        (JSEventListener):
+        (WebCore::JSEventListener::jsFunction):
+
 2012-11-14  Dan Carney  <dcarney@google.com>
 
         [V8] Rename dispatchWrap
diff --git a/Source/WebCore/ForwardingHeaders/heap/SlotVisitor.h b/Source/WebCore/ForwardingHeaders/heap/SlotVisitor.h
new file mode 100644 (file)
index 0000000..df8fb4d
--- /dev/null
@@ -0,0 +1,30 @@
+/*
+ * Copyright (C) 2011 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE INC. OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. 
+ */
+
+#ifndef WebCore_FWD_SlotVisitor_h
+#define WebCore_FWD_SlotVisitor_h
+#include <JavaScriptCore/SlotVisitor.h>
+#include <JavaScriptCore/SlotVisitorInlines.h>
+#endif
index 3c2f115..eaa772e 100644 (file)
@@ -35,6 +35,7 @@
 #include "ScriptWrappable.h"
 #include "StylePropertySet.h"
 #include "StyledElement.h"
+#include <heap/SlotVisitor.h>
 #include <heap/Weak.h>
 #include <runtime/Error.h>
 #include <runtime/FunctionPrototype.h>
index 8bfa9e1..92f2a36 100644 (file)
@@ -41,9 +41,10 @@ JSEventListener::JSEventListener(JSObject* function, JSObject* wrapper, bool isA
     , m_isAttribute(isAttribute)
     , m_isolatedWorld(isolatedWorld)
 {
-    if (wrapper)
-        m_jsFunction.setMayBeNull(*m_isolatedWorld->globalData(), wrapper, function);
-    else
+    if (wrapper) {
+        JSC::Heap::writeBarrier(wrapper, function);
+        m_jsFunction = JSC::PassWeak<JSC::JSObject>(function);
+    } else
         ASSERT(!function);
 #if ENABLE(INSPECTOR)
     ThreadLocalInspectorCounters::current().incrementCounter(ThreadLocalInspectorCounters::JSEventListenerCounter);
@@ -68,8 +69,7 @@ void JSEventListener::visitJSFunction(SlotVisitor& visitor)
     if (!m_wrapper)
         return;
 
-    if (m_jsFunction)
-        visitor.append(&m_jsFunction);
+    visitor.appendUnbarrieredWeak(&m_jsFunction);
 }
 
 void JSEventListener::handleEvent(ScriptExecutionContext* scriptExecutionContext, Event* event)
@@ -163,14 +163,8 @@ bool JSEventListener::virtualisAttribute() const
 
 bool JSEventListener::operator==(const EventListener& listener)
 {
-    if (const JSEventListener* jsEventListener = JSEventListener::cast(&listener)) {
-        // If m_wrapper is 0, then m_jsFunction is zombied, and should never be
-        // accessed. m_jsFunction should effectively be 0 in that case.
-        JSC::JSObject* jsFunction = m_wrapper ? m_jsFunction.get() : 0;
-        JSC::JSObject* otherJSFunction = jsEventListener->m_wrapper ?
-            jsEventListener->m_jsFunction.get() : 0;
-        return jsFunction == otherJSFunction && m_isAttribute == jsEventListener->m_isAttribute;
-    }
+    if (const JSEventListener* jsEventListener = JSEventListener::cast(&listener))
+        return m_jsFunction == jsEventListener->m_jsFunction && m_isAttribute == jsEventListener->m_isAttribute;
     return false;
 }
 
index 8cc55f5..ea1052d 100644 (file)
@@ -66,7 +66,7 @@ namespace WebCore {
         virtual void handleEvent(ScriptExecutionContext*, Event*);
 
     private:
-        mutable JSC::WriteBarrier<JSC::JSObject> m_jsFunction;
+        mutable JSC::Weak<JSC::JSObject> m_jsFunction;
         mutable JSC::Weak<JSC::JSObject> m_wrapper;
 
         bool m_isAttribute;
@@ -82,7 +82,8 @@ namespace WebCore {
 
         if (!m_jsFunction) {
             JSC::JSObject* function = initializeJSFunction(scriptExecutionContext);
-            m_jsFunction.setMayBeNull(*scriptExecutionContext->globalData(), m_wrapper.get(), function);
+            JSC::Heap::writeBarrier(m_wrapper.get(), function);
+            m_jsFunction = JSC::PassWeak<JSC::JSObject>(function);
         }
 
         // Verify that we have a valid wrapper protecting our function from