Merge pull request cocos2d#14694 from ricardoquesada/v3_10_js_fixes

ccvector_to / ccmap_to converted to new API

Author: pandamicro

cocos/base/CCRef.cpp

@@ -65,7 +65,7 @@ Ref::Ref()
 
 Ref::~Ref()
 {
-#if CC_ENABLE_SCRIPT_BINDING
+#if CC_ENABLE_SCRIPT_BINDING && not CC_ENABLE_GC_FOR_NATIVE_OBJECTS
     // if the object is referenced by Lua engine, remove it
     if (_luaID)
     {
@@ -100,11 +100,7 @@ void Ref::retain()
         if (scriptMgr && scriptMgr->getScriptType() == kScriptTypeJavascript)
         {
             _referenceCountAtRootTime = _referenceCount-1;
-
-            CCLOG("retain + root: %p (%s) rc=%d rcrt=%d", this, typeid(*this).name(), _referenceCount, _referenceCountAtRootTime);
-
             scriptMgr->rootObject(this);
-            _rooted = true;
         }
     }
 #endif // CC_ENABLE_SCRIPT_BINDING
@@ -116,13 +112,12 @@ void Ref::release()
     --_referenceCount;
 
 #if CC_ENABLE_SCRIPT_BINDING && CC_ENABLE_GC_FOR_NATIVE_OBJECTS
-    if (_scriptOwned && _rooted && _referenceCount==_referenceCountAtRootTime)
+    if (_scriptOwned && _rooted && _referenceCount==/*_referenceCountAtRootTime*/ 1)
     {
         auto scriptMgr = ScriptEngineManager::getInstance()->getScriptEngine();
         if (scriptMgr && scriptMgr->getScriptType() == kScriptTypeJavascript)
         {
             scriptMgr->unrootObject(this);
-            _rooted = false;
         }
     }
 #endif // CC_ENABLE_SCRIPT_BINDING

cocos/scripting/js-bindings/manual/ScriptingCore.cpp

@@ -212,18 +212,30 @@ static std::string getMouseFuncName(EventMouse::MouseEventType eventType)
     return funcName;
 }
 
-void removeJSObject(JSContext* cx, void* nativeObj)
+static void removeJSObject(JSContext* cx, cocos2d::Ref* nativeObj)
 {
     js_proxy_t* nproxy;
     js_proxy_t* jsproxy;
 
     nproxy = jsb_get_native_proxy(nativeObj);
-    if (nproxy) {
-        JS::RootedObject jsobj(cx, nproxy->obj);
-        jsproxy = jsb_get_js_proxy(jsobj);
-        RemoveObjectRoot(cx, &jsproxy->obj);
+    if (nproxy)
+    {
+        jsproxy = jsb_get_js_proxy(nproxy->obj);
+        JS::RemoveObjectRoot(cx, &jsproxy->obj);
+
+#if CC_ENABLE_GC_FOR_NATIVE_OBJECTS
+        nativeObj->_rooted = false;
+
+        // FIXME BUG TODO: remove this line... but if I do so
+        // another misterious bug appears
         jsb_remove_proxy(nproxy, jsproxy);
+#else
+        // only remove when not using GC,
+        // otherwise finalize won't be able to find the proxy
+        jsb_remove_proxy(nproxy, jsproxy);
+#endif
     }
+    else CCLOG("removeJSObject: BUG: cannot find native object = %p", nativeObj);
 }
 
 void ScriptingCore::executeJSFunctionWithThisObj(JS::HandleValue thisObj, JS::HandleValue callback)
@@ -874,15 +886,16 @@ void ScriptingCore::removeScriptObjectByObject(Ref* pObj)
     nproxy = jsb_get_native_proxy(ptr);
     if (nproxy)
     {
-        JSContext *cx = ScriptingCore::getInstance()->getGlobalContext();
-        JS::RootedObject jsobj(cx, nproxy->obj);
-        jsproxy = jsb_get_js_proxy(jsobj);
+        JSContext *cx = getGlobalContext();
+        jsproxy = jsb_get_js_proxy(nproxy->obj);
         if (jsproxy)
         {
             RemoveObjectRoot(cx, &jsproxy->obj);
             jsb_remove_proxy(nproxy, jsproxy);
         }
+//        else CCLOG("removeScriptObjectByObject. BUG: jsproxy not found = %p", nproxy);
     }
+//    else CCLOG("removeScriptObjectByObject. BUG: nproxy not found = %p", nproxy);
 }
 
 
@@ -1185,7 +1198,6 @@ bool ScriptingCore::handleTouchesEvent(void* nativeObj, cocos2d::EventTouch::Eve
 
     JS::RootedObject jsretArr(_cx, JS_NewArrayObject(this->_cx, 0));
 
-//    AddNamedObjectRoot(this->_cx, &jsretArr, "touchArray");
     int count = 0;
 
     js_type_class_t *typeClassEvent = nullptr;
@@ -1214,8 +1226,6 @@ bool ScriptingCore::handleTouchesEvent(void* nativeObj, cocos2d::EventTouch::Eve
         ret = executeFunctionWithOwner(OBJECT_TO_JSVAL(p->obj), funcName.c_str(), 2, dataVal, jsvalRet);
     }
 
-//    JS_RemoveObjectRoot(this->_cx, &jsretArr);
-
     for (auto& touch : touches)
     {
         removeJSObject(this->_cx, touch);
@@ -1570,11 +1580,9 @@ void ScriptingCore::rootObject(Ref* ref)
         JSContext *cx = getGlobalContext();
         jsproxy = jsb_get_js_proxy(nproxy->obj);
         JS::AddNamedObjectRoot(cx, &jsproxy->obj, typeid(*ref).name());
+        ref->_rooted = true;
     }
-    else
-    {
-        CCLOG("BUG in rootObject");
-    }
+    else CCLOG("rootObject: BUG. native not found: %p",  ref);
 }
 
 void ScriptingCore::unrootObject(Ref* ref)
@@ -1588,11 +1596,9 @@ void ScriptingCore::unrootObject(Ref* ref)
         JSContext *cx = getGlobalContext();
         jsproxy = jsb_get_js_proxy(nproxy->obj);
         JS::RemoveObjectRoot(cx, &jsproxy->obj);
+        ref->_rooted = false;
     }
-    else
-    {
-        CCLOG("BUG in unrootObject");
-    }
+    else CCLOG("unrootObject: BUG. native not found: %p",  ref);
 }
 
 #pragma mark - Debug
@@ -1922,44 +1928,59 @@ js_proxy_t* jsb_new_proxy(void* nativeObj, JS::HandleObject jsObj)
 {
     js_proxy_t* p = nullptr;
     JSObject* ptr = jsObj.get();
-    do {
-        p = (js_proxy_t *)malloc(sizeof(js_proxy_t));
-        assert(p);
-        js_proxy_t* nativeObjJsObjtmp = NULL;
-        HASH_FIND_PTR(_native_js_global_ht, &nativeObj, nativeObjJsObjtmp);
-        assert(!nativeObjJsObjtmp);
-        p->ptr = nativeObj;
-        p->obj = ptr;
-        HASH_ADD_PTR(_native_js_global_ht, ptr, p);
-        p = (js_proxy_t *)malloc(sizeof(js_proxy_t));
-        assert(p);
-        nativeObjJsObjtmp = NULL;
-        HASH_FIND_PTR(_js_native_global_ht, &ptr, nativeObjJsObjtmp);
-        assert(!nativeObjJsObjtmp);
-        p->ptr = nativeObj;
-        p->obj = ptr;
-        HASH_ADD_PTR(_js_native_global_ht, obj, p);
-    } while(0);
+
+    // native to JS index
+    p = (js_proxy_t *)malloc(sizeof(js_proxy_t));
+    assert(p);
+    js_proxy_t* nativeObjJsObjtmp = NULL;
+    HASH_FIND_PTR(_native_js_global_ht, &nativeObj, nativeObjJsObjtmp);
+    assert(!nativeObjJsObjtmp);
+    p->ptr = nativeObj;
+    p->obj = ptr;
+    HASH_ADD_PTR(_native_js_global_ht, ptr, p);
+
+    // JS to native Index
+    p = (js_proxy_t *)malloc(sizeof(js_proxy_t));
+    assert(p);
+    nativeObjJsObjtmp = NULL;
+    HASH_FIND_PTR(_js_native_global_ht, &ptr, nativeObjJsObjtmp);
+    assert(!nativeObjJsObjtmp);
+    p->ptr = nativeObj;
+    p->obj = ptr;
+    HASH_ADD_PTR(_js_native_global_ht, obj, p);
+
     return p;
 }
 
 js_proxy_t* jsb_get_native_proxy(void* nativeObj)
 {
     js_proxy_t* p = nullptr;
-    JS_GET_PROXY(p, nativeObj);
+    HASH_FIND_PTR(_native_js_global_ht, &nativeObj, p);
     return p;
 }
 
 js_proxy_t* jsb_get_js_proxy(JSObject* jsObj)
 {
     js_proxy_t* p = nullptr;
-    JS_GET_NATIVE_PROXY(p, jsObj);
+    HASH_FIND_PTR(_js_native_global_ht, &jsObj, p);
     return p;
 }
 
 void jsb_remove_proxy(js_proxy_t* nativeProxy, js_proxy_t* jsProxy)
 {
-    JS_REMOVE_PROXY(nativeProxy, jsProxy);
+    if (nativeProxy)
+    {
+        HASH_DEL(_native_js_global_ht, nativeProxy);
+        free(nativeProxy);
+    }
+    else CCLOG("jsb_remove_proxy: BUG: nativeProxy is null");
+
+    if (jsProxy)
+    {
+        HASH_DEL(_js_native_global_ht, jsProxy);
+        free(jsProxy);
+    }
+    else CCLOG("jsb_remove_proxy: BUG: jsProxy is null");
 }
 
 //
@@ -2001,7 +2022,10 @@ JSObject* jsb_ref_get_or_create_jsobject(JSContext *cx, cocos2d::Ref *ref, js_ty
     js_proxy_t* newproxy = jsb_new_proxy(ref, js_obj);
 #if CC_ENABLE_GC_FOR_NATIVE_OBJECTS
     CC_UNUSED_PARAM(newproxy);
-    // don't retain it.
+
+    // retain first copy, and before "owning" to prevent it
+    // from calling "rootObject"
+    ref->retain();
     ref->_scriptOwned = true;
 #else
     // don't autorelease it
@@ -2029,7 +2053,7 @@ void jsb_ref_init(JSContext* cx, JS::Heap<JSObject*> *obj, Ref* ref, const char*
     (void)cx;
     (void)obj;
     ref->_scriptOwned = true;
-    // don't retain it.
+    // don't retain it, already retained
 #else
     // autorelease it
     ref->autorelease();
@@ -2043,9 +2067,10 @@ void jsb_ref_autoreleased_init(JSContext* cx, JS::Heap<JSObject*> *obj, Ref* ref
 #if CC_ENABLE_GC_FOR_NATIVE_OBJECTS
     (void)cx;
     (void)obj;
-    ref->_scriptOwned = true;
-    // retain it, since the object is autoreleased
+    // retain first copy, and before "owning" to prevent it
+    // from calling "rootObject"
     ref->retain();
+    ref->_scriptOwned = true;
 #else
     // don't autorelease it, since it is already autoreleased
     JS::AddNamedObjectRoot(cx, obj, debug);
@@ -2059,8 +2084,7 @@ void jsb_ref_finalize(JSFreeOp* fop, JSObject* obj)
     js_proxy_t* nproxy;
     js_proxy_t* jsproxy;
 
-    JS::RootedObject jsobj(fop->runtime(), obj);
-    jsproxy = jsb_get_js_proxy(jsobj);
+    jsproxy = jsb_get_js_proxy(obj);
     if (jsproxy)
     {
         auto ref = static_cast<cocos2d::Ref*>(jsproxy->ptr);
@@ -2074,6 +2098,7 @@ void jsb_ref_finalize(JSFreeOp* fop, JSObject* obj)
         else
             jsb_remove_proxy(nullptr, jsproxy);
     }
+    else CCLOG("jsb_ref_finalize: BUG: proxy not found for %p", obj);
 #else
 //    CCLOG("jsb_ref_finalize: JSObject address = %p", obj);
 #endif

cocos/scripting/js-bindings/manual/ScriptingCore.h

@@ -618,7 +618,4 @@ JSObject* jsb_ref_get_or_create_jsobject(JSContext *cx, cocos2d::Ref *ref, js_ty
  */
 JSObject* jsb_ref_autoreleased_get_or_create_jsobject(JSContext *cx, cocos2d::Ref *ref, js_type_class_t *typeClass, const char* debug);
 
-
-void removeJSObject(JSContext* cx, void* nativeObj);
-
 #endif /* __SCRIPTING_CORE_H__ */

cocos/scripting/js-bindings/manual/cocos2d_specifics.hpp

@@ -87,8 +87,7 @@ inline js_type_class_t *js_get_type_from_native(T* native_obj) {
  */
 template<class T>
 inline js_proxy_t *js_get_or_create_proxy(JSContext *cx, T *native_obj) {
-    js_proxy_t *proxy;
-    HASH_FIND_PTR(_native_js_global_ht, &native_obj, proxy);
+    js_proxy_t *proxy = jsb_get_native_proxy(native_obj);
     if (!proxy) {
         js_type_class_t *typeProxy = js_get_type_from_native<T>(native_obj);
         // Return NULL if can't find its type rather than making an assert.

cocos/scripting/js-bindings/manual/extension/jsb_cocos2dx_extension_manual.cpp

@@ -557,8 +557,7 @@ class JSB_ControlButtonTarget : public Ref
 
     virtual void onEvent(Ref *controlButton, Control::EventType event)
     {
-        js_proxy_t * p;
-        JS_GET_PROXY(p, controlButton);
+        js_proxy_t* p = jsb_get_native_proxy(controlButton);
         if (!p)
         {
             log("Failed to get proxy for control button");

cocos/scripting/js-bindings/manual/js_bindings_opengl.cpp

@@ -35,12 +35,10 @@ void GLNode::onDraw(Mat4 &transform, uint32_t flags)
     JSContext *cx = ScriptingCore::getInstance()->getGlobalContext();
 
     js_type_class_t *typeClass = js_get_type_from_native<cocos2d::GLNode>(this);
-    auto j = jsb_ref_get_or_create_jsobject(cx, this, typeClass, "cocos2d::GLNode");
+    JS::RootedObject jsObj(cx, jsb_ref_get_or_create_jsobject(cx, this, typeClass, "cocos2d::GLNode"));
 
-    if (j)
+    if (jsObj.get())
     {
-        JS::RootedObject jsObj(cx, j);
-
          bool found = false;
          JSB_AUTOCOMPARTMENT_WITH_GLOBAL_OBJCET
 

cocos/scripting/js-bindings/manual/js_manual_conversions.h

@@ -280,9 +280,12 @@ jsval meshVertexAttrib_to_jsval(JSContext* cx, const cocos2d::MeshVertexAttrib&
 jsval uniform_to_jsval(JSContext* cx, const cocos2d::Uniform* uniform);
 jsval resourcedata_to_jsval(JSContext* cx, const cocos2d::ResourceData& v);
 
-template<class T>
-js_proxy_t *js_get_or_create_proxy(JSContext *cx, T *native_obj);
 
+// forward declaration
+template <class T>
+js_type_class_t *js_get_type_from_native(T* native_obj);
+
+// Ref version of ccvector_to_jsval
 template <class T>
 jsval ccvector_to_jsval(JSContext* cx, const cocos2d::Vector<T>& v)
 {
@@ -294,9 +297,10 @@ jsval ccvector_to_jsval(JSContext* cx, const cocos2d::Vector<T>& v)
         JS::RootedValue arrElement(cx);
 
         //First, check whether object is associated with js object.
-        js_proxy_t* jsproxy = js_get_or_create_proxy(cx, obj);
-        if (jsproxy) {
-            arrElement = OBJECT_TO_JSVAL(jsproxy->obj);
+        js_type_class_t *typeClass = js_get_type_from_native(obj);
+        JS::RootedObject jsobject(cx, jsb_ref_get_or_create_jsobject(cx, obj, typeClass, typeid(*obj).name()));
+        if (jsobject.get()) {
+            arrElement = OBJECT_TO_JSVAL(jsobject);
         }
 
         if (!JS_SetElement(cx, jsretArr, i, arrElement)) {
@@ -322,9 +326,11 @@ jsval ccmap_string_key_to_jsval(JSContext* cx, const cocos2d::Map<std::string, T
         T obj = iter->second;
 
         //First, check whether object is associated with js object.
-        js_proxy_t* jsproxy = js_get_or_create_proxy(cx, obj);
-        if (jsproxy) {
-            element = OBJECT_TO_JSVAL(jsproxy->obj);
+        js_type_class_t *typeClass = js_get_type_from_native(obj);
+        JS::RootedObject jsobject(cx, jsb_ref_get_or_create_jsobject(cx, obj, typeClass, typeid(*obj).name()));
+
+        if (jsobject.get()) {
+            element = OBJECT_TO_JSVAL(jsobject);
         }
 
         if (!key.empty())

cocos/scripting/js-bindings/manual/network/jsb_socketio.cpp

@@ -164,8 +164,7 @@ bool js_cocos2dx_SocketIO_connect(JSContext* cx, uint32_t argc, jsval* vp)
             if (ret)
             {
                 // link the native object with the javascript object
-                js_proxy_t *p;
-                HASH_FIND_PTR(_native_js_global_ht, &ret, p);
+                js_proxy_t *p = jsb_get_native_proxy(ret);
                 if(!p)
                 {
                     //previous connection not found, create a new one