[Qt] Replace use of internal Weak smart pointer with JSWeakObjectMap
authorhausmann@webkit.org <hausmann@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 17 Aug 2012 09:42:31 +0000 (09:42 +0000)
committerhausmann@webkit.org <hausmann@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 17 Aug 2012 09:42:31 +0000 (09:42 +0000)
https://bugs.webkit.org/show_bug.cgi?id=93872

Reviewed by Kenneth Rohde Christiansen.

Source/JavaScriptCore:

* Target.pri: Add missing JSWeakObjectMap file to build.

Source/WebCore:

The intention of this patch series is to replace use of internal JSC
API with use of the stable and public C API.

The JSC::Weak template is internal API and the only part of the C API
that exposes similar functionality is the JSWeakObjectMap. It is
special in the sense that its life-time is tied to the life-time of the
JS global object, which in turn is subject to garbage collection. In
order to maximize re-use of the same map across different JSContextRef
instances, we use one JSWeakObjectMap per context group and store the
map in a separate context.

* bridge/qt/qt_instance.cpp:
(JSC::Bindings::unusedWeakObjectMapCallback):
(Bindings):
(JSC::Bindings::WeakMapImpl::WeakMapImpl):
(JSC::Bindings::WeakMapImpl::~WeakMapImpl):
(JSC::Bindings::WeakMap::~WeakMap):
(JSC::Bindings::WeakMap::set):
(JSC::Bindings::WeakMap::get):
(JSC::Bindings::WeakMap::remove):
* bridge/qt/qt_instance.h:
(WeakMapImpl):
(Bindings):
(WeakMap):
(QtInstance):
* bridge/qt/qt_runtime.cpp:
(JSC::Bindings::QtRuntimeMethod::~QtRuntimeMethod):
(JSC::Bindings::QtRuntimeMethod::jsObjectRef):
* bridge/qt/qt_runtime.h:
(QtRuntimeMethod):

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/Target.pri
Source/WebCore/ChangeLog
Source/WebCore/bridge/qt/qt_instance.cpp
Source/WebCore/bridge/qt/qt_instance.h
Source/WebCore/bridge/qt/qt_runtime.cpp
Source/WebCore/bridge/qt/qt_runtime.h

index 5c95723..1becaf8 100644 (file)
@@ -1,3 +1,12 @@
+2012-08-16  Simon Hausmann  <simon.hausmann@nokia.com>
+
+        [Qt] Replace use of internal Weak smart pointer with JSWeakObjectMap
+        https://bugs.webkit.org/show_bug.cgi?id=93872
+
+        Reviewed by Kenneth Rohde Christiansen.
+
+        * Target.pri: Add missing JSWeakObjectMap file to build.
+
 2012-08-16  Filip Pizlo  <fpizlo@apple.com>
 
         Structure check hoisting should be less expensive
index 5340032..e862236 100644 (file)
@@ -40,6 +40,7 @@ SOURCES += \
     API/JSObjectRef.cpp \
     API/JSStringRef.cpp \
     API/JSValueRef.cpp \
+    API/JSWeakObjectMapRefPrivate.cpp \
     API/OpaqueJSString.cpp \
     assembler/ARMAssembler.cpp \
     assembler/ARMv7Assembler.cpp \
index 6bff9a1..acaff06 100644 (file)
@@ -1,3 +1,42 @@
+2012-08-16  Simon Hausmann  <simon.hausmann@nokia.com>
+
+        [Qt] Replace use of internal Weak smart pointer with JSWeakObjectMap
+        https://bugs.webkit.org/show_bug.cgi?id=93872
+
+        Reviewed by Kenneth Rohde Christiansen.
+
+        The intention of this patch series is to replace use of internal JSC
+        API with use of the stable and public C API.
+
+        The JSC::Weak template is internal API and the only part of the C API
+        that exposes similar functionality is the JSWeakObjectMap. It is
+        special in the sense that its life-time is tied to the life-time of the
+        JS global object, which in turn is subject to garbage collection. In
+        order to maximize re-use of the same map across different JSContextRef
+        instances, we use one JSWeakObjectMap per context group and store the
+        map in a separate context.
+
+        * bridge/qt/qt_instance.cpp:
+        (JSC::Bindings::unusedWeakObjectMapCallback):
+        (Bindings):
+        (JSC::Bindings::WeakMapImpl::WeakMapImpl):
+        (JSC::Bindings::WeakMapImpl::~WeakMapImpl):
+        (JSC::Bindings::WeakMap::~WeakMap):
+        (JSC::Bindings::WeakMap::set):
+        (JSC::Bindings::WeakMap::get):
+        (JSC::Bindings::WeakMap::remove):
+        * bridge/qt/qt_instance.h:
+        (WeakMapImpl):
+        (Bindings):
+        (WeakMap):
+        (QtInstance):
+        * bridge/qt/qt_runtime.cpp:
+        (JSC::Bindings::QtRuntimeMethod::~QtRuntimeMethod):
+        (JSC::Bindings::QtRuntimeMethod::jsObjectRef):
+        * bridge/qt/qt_runtime.h:
+        (QtRuntimeMethod):
+
+
 2012-08-16  Pavel Feldman  <pfeldman@chromium.org>
 
         Web Inspector: build Elements, Resources, Timeline, Audits and Console panels lazily.
index 513d711..d231c64 100644 (file)
 namespace JSC {
 namespace Bindings {
 
+static void unusedWeakObjectMapCallback(JSWeakObjectMapRef, void*)
+{
+}
+
+WeakMapImpl::WeakMapImpl(JSContextGroupRef group)
+{
+    m_context = JSGlobalContextCreateInGroup(group, 0);
+    // Deleted by GC when m_context's globalObject gets collected.
+    m_map = JSWeakObjectMapCreate(m_context, 0, unusedWeakObjectMapCallback);
+}
+
+WeakMapImpl::~WeakMapImpl()
+{
+    JSGlobalContextRelease(m_context);
+    m_context = 0;
+    m_map = 0;
+}
+
+typedef HashMap<JSContextGroupRef, RefPtr<WeakMapImpl> > WeakMapSet;
+static WeakMapSet weakMaps;
+
+WeakMap::~WeakMap()
+{
+    // If this is the last WeakMap instance left, then we should remove
+    // the cached WeakMapImpl from the global weakMaps, too.
+    if (m_impl && m_impl->refCount() == 2) {
+        weakMaps.remove(JSContextGetGroup(m_impl->m_context));
+        ASSERT(m_impl->hasOneRef());
+    }
+}
+
+void WeakMap::set(JSContextRef context, void *key, JSObjectRef object)
+{
+    if (!m_impl) {
+        JSContextGroupRef group = JSContextGetGroup(context);
+        WeakMapSet::AddResult entry = weakMaps.add(group, 0);
+        if (entry.isNewEntry)
+            entry.iterator->second = new WeakMapImpl(group);
+        m_impl = entry.iterator->second;
+    }
+    JSWeakObjectMapSet(m_impl->m_context, m_impl->m_map, key, object);
+}
+
+JSObjectRef WeakMap::get(void* key)
+{
+    if (!m_impl)
+        return 0;
+    return JSWeakObjectMapGet(m_impl->m_context, m_impl->m_map, key);
+}
+
+void WeakMap::remove(void *key)
+{
+    if (!m_impl)
+        return;
+    JSWeakObjectMapRemove(m_impl->m_context, m_impl->m_map, key);
+}
+
 // Cache QtInstances
 typedef QMultiHash<void*, QtInstance*> QObjectInstanceMap;
 static QObjectInstanceMap cachedInstances;
index cdd734a..e5d2a93 100644 (file)
@@ -21,6 +21,7 @@
 #define qt_instance_h
 
 #include "BridgeJSC.h"
+#include "JSWeakObjectMapRefPrivate.h"
 #include <QPointer>
 #include "Weak.h"
 #include "runtime_root.h"
@@ -35,6 +36,27 @@ class QtClass;
 class QtField;
 class QtRuntimeMethod;
 
+class WeakMapImpl : public RefCounted<WeakMapImpl> {
+public:
+    WeakMapImpl(JSContextGroupRef);
+    ~WeakMapImpl();
+
+    JSGlobalContextRef m_context;
+    JSWeakObjectMapRef m_map;
+};
+
+class WeakMap {
+public:
+    ~WeakMap();
+
+    void set(JSContextRef, void* key, JSObjectRef);
+    JSObjectRef get(void* key);
+    void remove(void* key);
+
+private:
+    RefPtr<WeakMapImpl> m_impl;
+};
+
 class QtInstance : public Instance {
 public:
     enum ValueOwnership {
@@ -81,11 +103,13 @@ private:
 
     friend class QtClass;
     friend class QtField;
+    friend class QtRuntimeMethod;
     QtInstance(QObject*, PassRefPtr<RootObject>, ValueOwnership); // Factory produced only..
     mutable QtClass* m_class;
     QPointer<QObject> m_object;
     QObject* m_hashkey;
     mutable QHash<QByteArray, QtRuntimeMethod*> m_methods;
+    WeakMap m_cachedMethods;
     mutable QHash<QString, QtField*> m_fields;
     ValueOwnership m_ownership;
 };
index 2cd2f58..ad507b4 100644 (file)
@@ -1301,8 +1301,9 @@ QtRuntimeMethod::QtRuntimeMethod(JSContextRef ctx, QObject* object, const QByteA
 
 QtRuntimeMethod::~QtRuntimeMethod()
 {
-    if (m_jsObject)
-        JSObjectSetPrivate(toRef(m_jsObject.get()), 0);
+    if (JSObjectRef cachedWrapper = m_instance->m_cachedMethods.get(this))
+        JSObjectSetPrivate(cachedWrapper, 0);
+    m_instance->m_cachedMethods.remove(this);
 }
 
 JSValueRef QtRuntimeMethod::call(JSContextRef context, JSObjectRef function, JSObjectRef /*thisObject*/, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception)
@@ -1350,8 +1351,8 @@ JSValueRef QtRuntimeMethod::disconnect(JSContextRef context, JSObjectRef functio
 
 JSObjectRef QtRuntimeMethod::jsObjectRef(JSContextRef context, JSValueRef* exception)
 {
-    if (m_jsObject)
-        return toRef(m_jsObject.get());
+    if (JSObjectRef cachedWrapper = m_instance->m_cachedMethods.get(this))
+        return cachedWrapper;
 
     static const JSClassDefinition classDefForConnect = {
         0, 0, "connect", 0, 0, 0,
@@ -1387,7 +1388,8 @@ JSObjectRef QtRuntimeMethod::jsObjectRef(JSContextRef context, JSValueRef* excep
     JSObjectSetProperty(context, object, lengthStr, JSValueMakeNumber(context, 0), attributes, exception);
     JSObjectSetProperty(context, object, nameStr, JSValueMakeString(context, actualNameStr.get()), attributes, exception);
 
-    m_jsObject = PassWeak<JSObject>(toJS(object));
+    m_instance->m_cachedMethods.set(context, this, object);
+
     return object;
 }
 
index 98756d1..a91e4f5 100644 (file)
@@ -122,7 +122,6 @@ private:
     QByteArray m_identifier;
     int m_index;
     int m_flags;
-    Weak<JSObject> m_jsObject;
     QtInstance* m_instance;
 };