Java objects now have stable identities in JavaScript in hosted mode. (They always had them in web mode.)
Suggested by: jaimeyap
Review by: knorton
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@2282 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/dev/core/src/com/google/gwt/dev/shell/CompilingClassLoader.java b/dev/core/src/com/google/gwt/dev/shell/CompilingClassLoader.java
index ba0102e..9304833 100644
--- a/dev/core/src/com/google/gwt/dev/shell/CompilingClassLoader.java
+++ b/dev/core/src/com/google/gwt/dev/shell/CompilingClassLoader.java
@@ -28,6 +28,8 @@
import com.google.gwt.dev.util.JsniRef;
import com.google.gwt.util.tools.Utility;
+import org.apache.commons.collections.map.AbstractReferenceMap;
+import org.apache.commons.collections.map.ReferenceIdentityMap;
import org.apache.commons.collections.map.ReferenceMap;
import java.io.IOException;
@@ -327,13 +329,15 @@
private final TreeLogger logger;
- private final Map<MethodAdaptor, Object> methodToDispatch = new HashMap<MethodAdaptor, Object>();
-
private final TypeOracle typeOracle;
@SuppressWarnings("unchecked")
+ private final Map<Object, Object> weakJavaWrapperCache = new ReferenceIdentityMap(
+ AbstractReferenceMap.WEAK, AbstractReferenceMap.WEAK);
+
+ @SuppressWarnings("unchecked")
private final Map<Integer, Object> weakJsoCache = new ReferenceMap(
- ReferenceMap.HARD, ReferenceMap.WEAK);
+ AbstractReferenceMap.HARD, AbstractReferenceMap.WEAK);
public CompilingClassLoader(TreeLogger logger, ByteCodeCompiler compiler,
TypeOracle typeOracle) throws UnableToCompleteException {
@@ -434,10 +438,16 @@
return dispClassInfoOracle.getDispId(jsniMemberRef);
}
- public Object getMethodDispatch(MethodAdaptor method) {
- synchronized (methodToDispatch) {
- return methodToDispatch.get(method);
- }
+ /**
+ * Retrieves the mapped wrapper for a given Java Object, provided the wrapper
+ * was previously cached and has not been garbage collected.
+ *
+ * @param javaObject the Object being wrapped
+ * @return the mapped wrapper, or <code>null</code> if the Java object
+ * mapped or if the wrapper has been garbage collected
+ */
+ public Object getWrapperForObject(Object javaObject) {
+ return weakJavaWrapperCache.get(javaObject);
}
/**
@@ -451,10 +461,14 @@
weakJsoCache.put(uniqueId, jso);
}
- public void putMethodDispatch(MethodAdaptor method, Object methodDispatch) {
- synchronized (methodToDispatch) {
- methodToDispatch.put(method, methodDispatch);
- }
+ /**
+ * Weakly caches a wrapper for a given Java Object.
+ *
+ * @param javaObject the Object being wrapped
+ * @param wrapper the mapped wrapper
+ */
+ public void putWrapperForObject(Object javaObject, Object wrapper) {
+ weakJavaWrapperCache.put(javaObject, wrapper);
}
@Override
@@ -499,11 +513,8 @@
void clear() {
weakJsoCache.clear();
+ weakJavaWrapperCache.clear();
dispClassInfoOracle.clear();
-
- synchronized (methodToDispatch) {
- methodToDispatch.clear();
- }
}
private String getBinaryName(JClassType type) {
diff --git a/dev/core/src/com/google/gwt/dev/shell/MethodAdaptor.java b/dev/core/src/com/google/gwt/dev/shell/MethodAdaptor.java
index 80ad04d..3be16a4 100644
--- a/dev/core/src/com/google/gwt/dev/shell/MethodAdaptor.java
+++ b/dev/core/src/com/google/gwt/dev/shell/MethodAdaptor.java
@@ -15,6 +15,7 @@
*/
package com.google.gwt.dev.shell;
+import java.lang.reflect.AccessibleObject;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
@@ -86,6 +87,10 @@
return returnType;
}
+ public AccessibleObject getUnderlyingObject() {
+ return (method != null) ? method : constructor;
+ }
+
@Override
public int hashCode() {
return hashCode;
diff --git a/dev/linux/src/com/google/gwt/dev/shell/moz/GeckoDispatchAdapter.java b/dev/linux/src/com/google/gwt/dev/shell/moz/GeckoDispatchAdapter.java
index 7c98a96..fbec94e 100644
--- a/dev/linux/src/com/google/gwt/dev/shell/moz/GeckoDispatchAdapter.java
+++ b/dev/linux/src/com/google/gwt/dev/shell/moz/GeckoDispatchAdapter.java
@@ -24,6 +24,7 @@
import com.google.gwt.dev.shell.moz.LowLevelMoz.DispatchMethod;
import com.google.gwt.dev.shell.moz.LowLevelMoz.DispatchObject;
+import java.lang.reflect.AccessibleObject;
import java.lang.reflect.Field;
/**
@@ -85,10 +86,11 @@
return;
} else {
MethodAdaptor method = javaDispatch.getMethod(dispId);
- DispatchMethod dispMethod = (DispatchMethod) classLoader.getMethodDispatch(method);
+ AccessibleObject obj = method.getUnderlyingObject();
+ DispatchMethod dispMethod = (DispatchMethod) classLoader.getWrapperForObject(obj);
if (dispMethod == null) {
dispMethod = new MethodDispatch(classLoader, method);
- classLoader.putMethodDispatch(method, dispMethod);
+ classLoader.putWrapperForObject(obj, dispMethod);
}
jsValue.setWrappedFunction(method.toString(), dispMethod);
}
diff --git a/dev/linux/src/com/google/gwt/dev/shell/moz/JsValueMoz.java b/dev/linux/src/com/google/gwt/dev/shell/moz/JsValueMoz.java
index da4173c..ed739ce 100644
--- a/dev/linux/src/com/google/gwt/dev/shell/moz/JsValueMoz.java
+++ b/dev/linux/src/com/google/gwt/dev/shell/moz/JsValueMoz.java
@@ -562,11 +562,15 @@
if (val instanceof DispatchObject) {
dispObj = (DispatchObject) val;
} else {
- dispObj = new GeckoDispatchAdapter(cl, val);
+ dispObj = (DispatchObject) cl.getWrapperForObject(val);
+ if (dispObj == null) {
+ dispObj = new GeckoDispatchAdapter(cl, val);
+ cl.putWrapperForObject(val, dispObj);
+ }
}
- Integer jsval = LowLevelMoz.sObjectToJsval.get(dispObj);
- if (jsval != null) {
- _setJsval(jsRootedValue, jsval);
+ Integer cached = LowLevelMoz.sObjectToJsval.get(dispObj);
+ if (cached != null) {
+ _setJsval(jsRootedValue, cached);
} else {
_setWrappedJavaObject(jsRootedValue, dispObj);
LowLevelMoz.sObjectToJsval.put(dispObj, _getJsval(jsRootedValue));
diff --git a/dev/linux/src/com/google/gwt/dev/shell/moz/LowLevelMoz.java b/dev/linux/src/com/google/gwt/dev/shell/moz/LowLevelMoz.java
index c06b166..d102fe8 100644
--- a/dev/linux/src/com/google/gwt/dev/shell/moz/LowLevelMoz.java
+++ b/dev/linux/src/com/google/gwt/dev/shell/moz/LowLevelMoz.java
@@ -17,7 +17,6 @@
import com.google.gwt.dev.shell.LowLevel;
-import java.util.Collections;
import java.util.IdentityHashMap;
import java.util.Map;
import java.util.Vector;
@@ -80,10 +79,11 @@
}
/**
- * Stores a map from DispatchObject/DispatchMethod to the live underlying jsval. This is used to
- * both preserve identity for the same Java Object and also prevent GC.
+ * Stores a map from DispatchObject/DispatchMethod to the live underlying
+ * jsval. This is used to both preserve identity for the same Java Object and
+ * also prevent GC.
*/
- static Map<Object, Integer> sObjectToJsval = Collections.synchronizedMap(new IdentityHashMap<Object, Integer>());
+ static Map<Object, Integer> sObjectToJsval = new IdentityHashMap<Object, Integer>();
private static Vector<ExternalFactory> sExternalFactories = new Vector<ExternalFactory>();
private static boolean sInitialized = false;
diff --git a/dev/mac/src/com/google/gwt/dev/shell/mac/JsValueSaf.java b/dev/mac/src/com/google/gwt/dev/shell/mac/JsValueSaf.java
index 36faef1..de7b7b6 100644
--- a/dev/mac/src/com/google/gwt/dev/shell/mac/JsValueSaf.java
+++ b/dev/mac/src/com/google/gwt/dev/shell/mac/JsValueSaf.java
@@ -92,11 +92,8 @@
@Override
public int getJavaScriptObjectPointer() {
- if (isJavaScriptObject()) {
- return jsval;
- } else {
- return 0;
- }
+ assert isJavaScriptObject();
+ return jsval;
}
public int getJsValue() {
@@ -196,17 +193,6 @@
setJsVal(LowLevelSaf.toJsNumber(LowLevelSaf.getCurrentJsContext(), val));
}
- /**
- * Set a new value. Unlock the previous value, but do *not* lock the new value
- * (see class comment).
- *
- * @param jsval the new value to set
- */
- public void setJsVal(int jsval) {
- LowLevelSaf.gcUnprotect(LowLevelSaf.getCurrentJsContext(), this.jsval);
- init(jsval);
- }
-
@Override
public void setNull() {
setJsVal(LowLevelSaf.getJsNull(LowLevelSaf.getCurrentJsContext()));
@@ -247,7 +233,11 @@
} else if (val instanceof DispatchObject) {
dispObj = (DispatchObject) val;
} else {
- dispObj = new WebKitDispatchAdapter(cl, val);
+ dispObj = (DispatchObject) cl.getWrapperForObject(val);
+ if (dispObj == null) {
+ dispObj = new WebKitDispatchAdapter(cl, val);
+ cl.putWrapperForObject(val, dispObj);
+ }
}
setJsVal(LowLevelSaf.wrapDispatchObject(LowLevelSaf.getCurrentJsContext(),
dispObj));
@@ -277,4 +267,15 @@
}
}
+ /**
+ * Set a new value. Unlock the previous value, but do *not* lock the new value
+ * (see class comment).
+ *
+ * @param jsval the new value to set
+ */
+ private void setJsVal(int jsval) {
+ LowLevelSaf.gcUnprotect(LowLevelSaf.getCurrentJsContext(), this.jsval);
+ init(jsval);
+ }
+
}
diff --git a/dev/mac/src/com/google/gwt/dev/shell/mac/ModuleSpaceSaf.java b/dev/mac/src/com/google/gwt/dev/shell/mac/ModuleSpaceSaf.java
index b8a8b11..1959d43 100644
--- a/dev/mac/src/com/google/gwt/dev/shell/mac/ModuleSpaceSaf.java
+++ b/dev/mac/src/com/google/gwt/dev/shell/mac/ModuleSpaceSaf.java
@@ -20,7 +20,6 @@
import com.google.gwt.dev.shell.JsValueGlue;
import com.google.gwt.dev.shell.ModuleSpace;
import com.google.gwt.dev.shell.ModuleSpaceHost;
-import com.google.gwt.dev.shell.mac.LowLevelSaf.DispatchObject;
/**
* An implementation of {@link com.google.gwt.dev.shell.ModuleSpace} for Safari.
@@ -105,19 +104,4 @@
protected Object getStaticDispatcher() {
return new WebKitDispatchAdapter(getIsolatedClassLoader());
}
-
- protected int wrapObjectAsJSObject(Object o) {
- if (o == null) {
- return LowLevelSaf.getJsNull(LowLevelSaf.getCurrentJsContext());
- }
-
- DispatchObject dispObj;
- if (o instanceof DispatchObject) {
- dispObj = (DispatchObject) o;
- } else {
- dispObj = new WebKitDispatchAdapter(getIsolatedClassLoader(), o);
- }
- return LowLevelSaf.wrapDispatchObject(LowLevelSaf.getCurrentJsContext(),
- dispObj);
- }
}
diff --git a/dev/mac/src/com/google/gwt/dev/shell/mac/WebKitDispatchAdapter.java b/dev/mac/src/com/google/gwt/dev/shell/mac/WebKitDispatchAdapter.java
index bc25905..69a0240 100644
--- a/dev/mac/src/com/google/gwt/dev/shell/mac/WebKitDispatchAdapter.java
+++ b/dev/mac/src/com/google/gwt/dev/shell/mac/WebKitDispatchAdapter.java
@@ -24,6 +24,7 @@
import com.google.gwt.dev.shell.mac.LowLevelSaf.DispatchMethod;
import com.google.gwt.dev.shell.mac.LowLevelSaf.DispatchObject;
+import java.lang.reflect.AccessibleObject;
import java.lang.reflect.Field;
/**
@@ -79,10 +80,11 @@
return jsval;
} else {
MethodAdaptor method = javaDispatch.getMethod(dispId);
- DispatchMethod dispMethod = (DispatchMethod) classLoader.getMethodDispatch(method);
+ AccessibleObject obj = method.getUnderlyingObject();
+ DispatchMethod dispMethod = (DispatchMethod) classLoader.getWrapperForObject(obj);
if (dispMethod == null) {
dispMethod = new MethodDispatch(classLoader, method);
- classLoader.putMethodDispatch(method, dispMethod);
+ classLoader.putWrapperForObject(obj, dispMethod);
}
// Native code eats the same ref it gave us.
return LowLevelSaf.wrapDispatchMethod(jsContext, method.toString(),
diff --git a/dev/windows/src/com/google/gwt/dev/shell/ie/IDispatchProxy.java b/dev/windows/src/com/google/gwt/dev/shell/ie/IDispatchProxy.java
index f30963a..f23df9d 100644
--- a/dev/windows/src/com/google/gwt/dev/shell/ie/IDispatchProxy.java
+++ b/dev/windows/src/com/google/gwt/dev/shell/ie/IDispatchProxy.java
@@ -26,6 +26,7 @@
import org.eclipse.swt.internal.ole.win32.IDispatch;
import org.eclipse.swt.ole.win32.Variant;
+import java.lang.reflect.AccessibleObject;
import java.lang.reflect.Field;
import java.lang.reflect.InvocationTargetException;
@@ -154,10 +155,11 @@
return callMethod(classLoader, getTarget(), params, method);
} else if (flags == COM.DISPATCH_PROPERTYGET) {
// The function is being accessed as a property.
- IDispatchImpl dispMethod = (IDispatchImpl) classLoader.getMethodDispatch(method);
+ AccessibleObject obj = method.getUnderlyingObject();
+ IDispatchImpl dispMethod = (IDispatchImpl) classLoader.getWrapperForObject(obj);
if (dispMethod == null || dispMethod.refCount < 1) {
dispMethod = new MethodDispatch(classLoader, method);
- classLoader.putMethodDispatch(method, dispMethod);
+ classLoader.putWrapperForObject(obj, dispMethod);
}
IDispatch disp = new IDispatch(dispMethod.getAddress());
disp.AddRef();
diff --git a/dev/windows/src/com/google/gwt/dev/shell/ie/JsValueIE6.java b/dev/windows/src/com/google/gwt/dev/shell/ie/JsValueIE6.java
index 175d3e9..89cded6 100644
--- a/dev/windows/src/com/google/gwt/dev/shell/ie/JsValueIE6.java
+++ b/dev/windows/src/com/google/gwt/dev/shell/ie/JsValueIE6.java
@@ -496,7 +496,11 @@
} else if (val instanceof IDispatchImpl) {
dispObj = (IDispatchImpl) val;
} else {
- dispObj = new IDispatchProxy(cl, val);
+ dispObj = (IDispatchImpl) cl.getWrapperForObject(val);
+ if (dispObj == null || dispObj.refCount < 1) {
+ dispObj = new IDispatchProxy(cl, val);
+ cl.putWrapperForObject(val, dispObj);
+ }
}
IDispatch disp = new IDispatch(dispObj.getAddress());
disp.AddRef();
diff --git a/dev/windows/src/com/google/gwt/dev/shell/ie/MethodDispatch.java b/dev/windows/src/com/google/gwt/dev/shell/ie/MethodDispatch.java
index 2d642e9..b6eeb57 100644
--- a/dev/windows/src/com/google/gwt/dev/shell/ie/MethodDispatch.java
+++ b/dev/windows/src/com/google/gwt/dev/shell/ie/MethodDispatch.java
@@ -23,6 +23,7 @@
import org.eclipse.swt.internal.ole.win32.IDispatch;
import org.eclipse.swt.ole.win32.Variant;
+import java.lang.reflect.AccessibleObject;
import java.lang.reflect.InvocationTargetException;
/**
@@ -114,10 +115,11 @@
throw new RuntimeException(
"Failed to get Object.toString() method", e);
}
- IDispatchImpl dispMethod = (IDispatchImpl) classLoader.getMethodDispatch(toStringMethod);
- if (dispMethod == null) {
+ AccessibleObject obj = toStringMethod.getUnderlyingObject();
+ IDispatchImpl dispMethod = (IDispatchImpl) classLoader.getWrapperForObject(obj);
+ if (dispMethod == null || dispMethod.refCount < 1) {
dispMethod = new MethodDispatch(classLoader, toStringMethod);
- classLoader.putMethodDispatch(toStringMethod, dispMethod);
+ classLoader.putWrapperForObject(obj, dispMethod);
}
IDispatch disp = new IDispatch(dispMethod.getAddress());
disp.AddRef();
diff --git a/user/test/com/google/gwt/dev/jjs/test/HostedTest.java b/user/test/com/google/gwt/dev/jjs/test/HostedTest.java
index d5409e5..efe1e2c 100644
--- a/user/test/com/google/gwt/dev/jjs/test/HostedTest.java
+++ b/user/test/com/google/gwt/dev/jjs/test/HostedTest.java
@@ -144,6 +144,18 @@
return s.length;
}-*/;
+ private static native boolean jsIdentical(Object o1, Object o2) /*-{
+ return o1 === o2;
+ }-*/;
+
+ private static native boolean jsniInstanceFunctionsIdentical(Object o) /*-{
+ return o.@java.lang.Object::toString() === o.@java.lang.Object::toString();
+ }-*/;
+
+ private static native boolean jsniStaticFunctionsIdentical() /*-{
+ return @com.google.gwt.dev.jjs.test.HostedTest::sFoo(Ljava/lang/String;) === @com.google.gwt.dev.jjs.test.HostedTest::sFoo(Ljava/lang/String;);
+ }-*/;
+
private static native int passThroughInt(int val) /*-{
return val;
}-*/;
@@ -200,7 +212,7 @@
assertTrue(HostedTest.class.desiredAssertionStatus());
}
}
-
+
/*
* Test that returning JavaScript boxed primitives works as expected. Note
* that Boolean and Number cannot be supported properly in web mode, so we do
@@ -322,6 +334,13 @@
assertEquals(1, b.getUsingBinaryRef());
}
+ public void testJavaObjectIdentityInJS() {
+ Object o = new Object();
+ assertTrue(jsIdentical(o, o));
+ assertTrue(jsniInstanceFunctionsIdentical(o));
+ assertTrue(jsniStaticFunctionsIdentical());
+ }
+
public void testJsniFormats() {
jsniA();
jsniB();
