Updates the use of weak references in the script-to-Java bridge for V8.

Currently, V8 uses weak references to the Java object owned by JObjectWrapper.
This is in contrast to JSC, which uses strong references in JObjectWrapper.
However, for the special-case where JObjectWrapper is used in JavaInstance in
WebCoreFrameBridge, JSC uses WeakJavaInstance to swap the strong references for
weak references.

This change updates the V8 version of JObjectWrapper to use strong references
to match the JSC version. To maintain the weak reference behavior where
JObjectWrapper is used in JavaInstance in WebCoreFrameBridge, V8 now make use of
WeakJavaInstance too. This requires changes to jni_npobject to call the
necessary methods on JavaInstance to swap the references when the object is
accessed.

Change-Id: I3724d7e6437588feb0268a2670b02a93b52e54f0

5 files changed, 56 insertions, 56 deletions

diff --git a/V8Binding/jni/jni_instance.cpp b/V8Binding/jni/jni_instance.cpp
index 657b8c4..70da861 100644
--- a/V8Binding/jni/jni_instance.cpp
+++ b/V8Binding/jni/jni_instance.cpp
@@ -53,14 +53,12 @@ JavaInstance::~JavaInstance ()
 JavaClass* JavaInstance::getClass() const 
 {
     if (_class == 0) {
-        jobject local_ref = getLocalRef();
-        _class = new JavaClass(local_ref);
-        getJNIEnv()->DeleteLocalRef(local_ref);
+        _class = new JavaClass(javaInstance());
     }
     return _class;
 }
 
-bool JavaInstance::invokeMethod(const char* methodName, const NPVariant* args, uint32_t count, NPVariant* resultValue)
+bool JavaInstance::invokeMethod(const char* methodName, const NPVariant* args, int count, NPVariant* resultValue)
 {
     int i;
     jvalue *jArgs;
@@ -108,7 +106,7 @@ bool JavaInstance::invokeMethod(const char* methodName, const NPVariant* args, u
     // The following code can be conditionally removed once we have a Tiger update that
     // contains the new Java plugin.  It is needed for builds prior to Tiger.
     {    
-        jobject obj = getLocalRef();
+        jobject obj = javaInstance();
         switch (jMethod->JNIReturnType()){
             case void_type:
                 callJNIMethodIDA<void>(obj, jMethod->methodID(obj), jArgs);
@@ -145,7 +143,6 @@ bool JavaInstance::invokeMethod(const char* methodName, const NPVariant* args, u
             default:
                 break;
         }
-        getJNIEnv()->DeleteLocalRef(obj);
     }
     
     convertJValueToNPVariant(result, jMethod->JNIReturnType(), jMethod->returnType(), resultValue);
@@ -163,34 +160,16 @@ JObjectWrapper::JObjectWrapper(jobject instance)
     // It'll be used to delete the reference.
     _env = getJNIEnv();
 
-    jclass localClsRef = _env->FindClass("java/lang/ref/WeakReference");
-    jmethodID weakRefInit = _env->GetMethodID(localClsRef, "<init>",
-                                    "(Ljava/lang/Object;)V");
-    mWeakRefGet = _env->GetMethodID(localClsRef, "get",
-                                   "()Ljava/lang/Object;");
+    _instance = _env->NewGlobalRef(instance);
 
-    jobject weakRef = _env->NewObject(localClsRef, weakRefInit, instance);
-
-    _instance = _env->NewGlobalRef(weakRef);
-    
     LOGV("new global ref %p for %p\n", _instance, instance);
 
     if  (_instance == NULL) {
         fprintf (stderr, "%s:  could not get GlobalRef for %p\n", __PRETTY_FUNCTION__, instance);
     }
-
-    _env->DeleteLocalRef(weakRef);
-    _env->DeleteLocalRef(localClsRef);
 }
 
 JObjectWrapper::~JObjectWrapper() {
     LOGV("deleting global ref %p\n", _instance);
     _env->DeleteGlobalRef(_instance);
 }
-
-jobject JObjectWrapper::getLocalRef() const {
-    jobject real = _env->CallObjectMethod(_instance, mWeakRefGet);
-    if (!real)
-        LOGE("The real object has been deleted");
-    return _env->NewLocalRef(real);
-}
diff --git a/V8Binding/jni/jni_instance.h b/V8Binding/jni/jni_instance.h
index f8b0d65..608b461 100644
--- a/V8Binding/jni/jni_instance.h
+++ b/V8Binding/jni/jni_instance.h
@@ -33,10 +33,6 @@
 #include <wtf/RefCounted.h>
 #include <wtf/RefPtr.h>
 
-namespace android {
-class WeakJavaInstance;
-}
-
 using namespace WTF;
 
 namespace JSC {
@@ -47,7 +43,15 @@ class JavaClass;
 
 class JObjectWrapper
 {
+friend class RefPtr<JObjectWrapper>;
+friend class JavaField;
+friend class JavaInstance;
+
 public:
+    jobject instance() const { return _instance; }
+    void setInstance(jobject instance) { _instance = instance; }
+
+protected:
     JObjectWrapper(jobject instance);    
     ~JObjectWrapper();
     
@@ -58,35 +62,37 @@ public:
             delete this; 
     }
 
-    jobject getLocalRef() const;
+    jobject _instance;
 
 private:
     JNIEnv *_env;
     unsigned int _refCount;
-    jobject _instance;  // it is a global weak reference.
-
-    jmethodID mWeakRefGet;  // cache WeakReference::Get method id
 };
 
 class JavaInstance : public RefCounted<JavaInstance>
 {
 public:
     JavaInstance(jobject instance);
-    ~JavaInstance();
+    virtual ~JavaInstance();
 
     JavaClass* getClass() const;
 
-    bool invokeMethod(const char* name, const NPVariant* args, uint32_t argsCount, NPVariant* result);
+    bool invokeMethod(const char* name, const NPVariant* args, int argsCount, NPVariant* result);
 
-    // Returns a local reference, and the caller must delete
-    // the returned reference after use.
-    jobject getLocalRef() const {
-        return _instance->getLocalRef();
-    }
+    jobject javaInstance() const { return _instance->_instance; }
 
-private:
+    // These functions are called before and after the main entry points into
+    // the native implementations.  They can be used to establish and cleanup
+    // any needed state.
+    void begin() { virtualBegin(); }
+    void end() { virtualEnd(); }
+
+protected:
     RefPtr<JObjectWrapper> _instance;
     mutable JavaClass* _class;
+
+    virtual void virtualBegin() {}
+    virtual void virtualEnd() {}
 };
 
 } // namespace Bindings
diff --git a/V8Binding/jni/jni_npobject.cpp b/V8Binding/jni/jni_npobject.cpp
index 3480ac5..8a49a19 100644
--- a/V8Binding/jni/jni_npobject.cpp
+++ b/V8Binding/jni/jni_npobject.cpp
@@ -93,7 +93,9 @@ bool JavaNPObject_HasMethod(NPObject* obj, NPIdentifier identifier) {
     if (name == 0)
         return false;
 
+    instance->begin();
     bool result = (instance->getClass()->methodsNamed(name).size() > 0);
+    instance->end();
 
     // TODO: use NPN_MemFree
     free(name);
@@ -110,7 +112,9 @@ bool JavaNPObject_Invoke(NPObject* obj, NPIdentifier identifier,
     if (name == 0)
         return false;
 
+    instance->begin();
     bool r = instance->invokeMethod(name, args, argCount, result);
+    instance->end();
 
     // TODO: use NPN_MemFree
     free(name);
@@ -125,7 +129,9 @@ bool JavaNPObject_HasProperty(NPObject* obj, NPIdentifier identifier) {
     NPUTF8* name = _NPN_UTF8FromIdentifier(identifier);
     if (name == 0)
         return false;
+    instance->begin();
     bool result = instance->getClass()->fieldNamed(name) != 0;
+    instance->end();
     free(name);
     return result;
 }
@@ -139,19 +145,19 @@ bool JavaNPObject_GetProperty(NPObject* obj, NPIdentifier identifier, NPVariant*
     if (name == 0)
         return false;
 
+    instance->begin();
     JavaField* field = instance->getClass()->fieldNamed(name);
+    instance->end();
     free(name);  // TODO: use NPN_MemFree
 
     if (field == 0) {
         return false;
     }
 
-    jobject local_ref = instance->getLocalRef();
-    jvalue value = getJNIField(local_ref,
+    jvalue value = getJNIField(instance->javaInstance(),
                                field->getJNIType(),
                                field->name(),
                                field->type());
-    getJNIEnv()->DeleteLocalRef(local_ref);
 
     convertJValueToNPVariant(value, field->getJNIType(), field->type(), result);
 
diff --git a/WebCore/bridge/jni/v8/jni_utility_private.cpp b/WebCore/bridge/jni/v8/jni_utility_private.cpp
index c58472e..6c8f018 100644
--- a/WebCore/bridge/jni/v8/jni_utility_private.cpp
+++ b/WebCore/bridge/jni/v8/jni_utility_private.cpp
@@ -48,7 +48,7 @@ jvalue convertNPVariantToJValue(NPVariant value, JNIType jniType, const char* ja
             if (type == NPVariantType_Object) {
                 NPObject* objectImp = NPVARIANT_TO_OBJECT(value);
                 if (JavaInstance* instance = ExtractJavaInstance(objectImp))
-                    result.l = instance->getLocalRef();
+                    result.l = instance->javaInstance();
             }
 
             // Now convert value to a string if the target type is a java.lang.string, and we're not
diff --git a/WebKit/android/jni/WebCoreFrameBridge.cpp b/WebKit/android/jni/WebCoreFrameBridge.cpp
index 85b0f34..ee39462 100644
--- a/WebKit/android/jni/WebCoreFrameBridge.cpp
+++ b/WebKit/android/jni/WebCoreFrameBridge.cpp
@@ -1195,20 +1195,30 @@ static jobject StringByEvaluatingJavaScriptFromString(JNIEnv *env, jobject obj,
     return env->NewString((unsigned short*)result.characters(), len);
 }
 
-#if USE(JSC)
 // Wrap the JavaInstance used when binding custom javascript interfaces. Use a
 // weak reference so that the gc can collect the WebView. Override virtualBegin
 // and virtualEnd and swap the weak reference for the real object.
 class WeakJavaInstance : public JavaInstance {
 public:
+#if USE(JSC)
     static PassRefPtr<WeakJavaInstance> create(jobject obj, PassRefPtr<RootObject> root)
     {
         return adoptRef(new WeakJavaInstance(obj, root));
     }
+#elif USE(V8)
+    static PassRefPtr<WeakJavaInstance> create(jobject obj)
+    {
+        return adoptRef(new WeakJavaInstance(obj));
+    }
+#endif
 
-protected:
+private:
+#if USE(JSC)
     WeakJavaInstance(jobject instance, PassRefPtr<RootObject> rootObject)
         : JavaInstance(instance, rootObject)
+#elif USE(V8)
+    WeakJavaInstance(jobject instance) : JavaInstance(instance)
+#endif
     {
         JNIEnv* env = getJNIEnv();
         // JavaInstance creates a global ref to instance in its constructor.
@@ -1248,7 +1258,6 @@ private:
     jobject _realObject;
     jobject _weakRef;
 };
-#endif  // USE(JSC)
 
 static void AddJavascriptInterface(JNIEnv *env, jobject obj, jint nativeFramePointer,
         jobject javascriptObj, jstring interfaceName)
@@ -1288,19 +1297,19 @@ static void AddJavascriptInterface(JNIEnv *env, jobject obj, jint nativeFramePoi
             checkException(env);
         }
     }
-#endif  // USE(JSC)
-
-#if USE(V8)
+#elif USE(V8)
     if (pFrame) {
+        PassRefPtr<JavaInstance> addedObject = WeakJavaInstance::create(javascriptObj);
         const char* name = getCharactersFromJStringInEnv(env, interfaceName);
-        NPObject* obj = JavaInstanceToNPObject(new JavaInstance(javascriptObj));
+        // Pass ownership of the added object to bindToWindowObject.
+        NPObject* obj = JavaInstanceToNPObject(addedObject.releaseRef());
         pFrame->script()->bindToWindowObject(pFrame, name, obj);
-        // JavaInstanceToNPObject calls NPN_RetainObject on the
-        // returned one (see CreateV8ObjectForNPObject in V8NPObject.cpp).
-        // BindToWindowObject also increases obj's ref count and decrease
+        // bindToWindowObject calls NPN_RetainObject on the
+        // returned one (see createV8ObjectForNPObject in V8NPObject.cpp).
+        // bindToWindowObject also increases obj's ref count and decreases
         // the ref count when the object is not reachable from JavaScript
         // side. Code here must release the reference count increased by
-        // JavaInstanceToNPObject.
+        // bindToWindowObject.
 
         // Note that while this function is declared in WebCore/bridge/npruntime.h, for V8 builds
         // we use WebCore/bindings/v8/npruntime.cpp (rather than