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();