Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When InterfaceAdapter is used, the wrong thisObj is used #1453

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,8 @@ Object invokeImpl(
}
}
}
Scriptable thisObj = wf.wrapAsJavaObject(cx, topScope, thisObject, null);

Object result = function.call(cx, topScope, thisObj, args);
Object result = function.call(cx, topScope, (Scriptable) target, args);
Class<?> javaResultType = method.getReturnType();
if (javaResultType == Void.TYPE) {
result = null;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
package org.mozilla.javascript.tests;

import org.junit.Assert;
import org.junit.Test;
import org.mozilla.javascript.Scriptable;

public class JavaAdapterInvokeTest {

public interface AdapterInterface {
int m1(int i);

int m2();
}

public static class AdapterClass {
private AdapterInterface adapter;

public AdapterClass(AdapterInterface adapter) {
this.adapter = adapter;
}

public int doIt(int i) {
return this.adapter.m1(i) + this.adapter.m2();
}
}

/**
* This test creates a new {@link AdapterClass} and passes a javascript object with two
* functions (m1,m2) to the constructor.
*
* <p>It is expected, that rhino creates an {@link AdapterInterface}, that delegates the calls
* to m1/m2 back to the javascript implementation.
*/
@Test
public void testInvoke() {
String testCode =
"'use strict'\n"
+ "var impl = {"
+ " m1: function(i) { return i + 1 },\n"
+ " m2: function() { return 7 }\n"
+ "}\n"
+ "adapter = new Packages."
+ AdapterClass.class.getName()
+ "(impl)\n"
+ "adapter.doIt(42)";
Utils.runWithAllOptimizationLevels(
cx -> {
final Scriptable scope = cx.initStandardObjects();
Number result = (Number) cx.evaluateString(scope, testCode, "", 1, null);
Assert.assertEquals(50, result.intValue());
return null;
});
}

/** Similar to {@link #testInvoke()}, but we use a javascript object, created from prototype. */
@Test
public void testInvokeWithPrototype() {
String testCode =
"'use strict'\n"
+ "function Obj() {}\n"
+ "Obj.prototype.m1 = function(i) { return i + 1 }\n"
+ "Obj.prototype.m2 = function() { return 7 }\n"
+ "var impl = new Obj()\n"
+ "adapter = new Packages."
+ AdapterClass.class.getName()
+ "(impl)\n"
+ "adapter.doIt(42)";

Utils.runWithAllOptimizationLevels(
cx -> {
final Scriptable scope = cx.initStandardObjects();
Number result = (Number) cx.evaluateString(scope, testCode, "", 1, null);
Assert.assertEquals(50, result.intValue());
return null;
});
}

/**
* Test uses some value in the method that are either constructed in ctor or in prototype. This
* test fails before #1453
*/
@Test
public void testInvokeWithPrototypeAndObjs() {
String testCode =
"'use strict'\n"
+ "function Obj() { this.myObj = {one: 1} }\n"
+ "Obj.prototype.seven = 7\n"
+ "Obj.prototype.m1 = function(i) { return i + this.myObj.one }\n"
+ "Obj.prototype.m2 = function() { return this.seven }\n"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test shows the problem. this.myObj and this.seven were undefined, when the js object is passed in an InterfaceAdapter
The other tests are all passing without the fix and are for completeness, not to break anything else

+ "var impl = new Obj()\n"
+ "adapter = new Packages."
+ AdapterClass.class.getName()
+ "(impl)\n"
+ "adapter.doIt(42)";

Utils.runWithAllOptimizationLevels(
cx -> {
final Scriptable scope = cx.initStandardObjects();
Number result = (Number) cx.evaluateString(scope, testCode, "", 1, null);
Assert.assertEquals(50, result.intValue());
return null;
});
}

@Test
public void testJavaLangThread() {
String testCode =
"'use strict'\n"
+ "function MyRunnable() { this.myObj = {one: 1} }\n"
+ "MyRunnable.prototype.run = function() { this.myObj.one++ }\n"
+ "var runnable = new MyRunnable()\n"
+ "var thread = new java.lang.Thread(runnable)\n"
+ "thread.start()\n"
+ "thread.join()\n"
+ "runnable.myObj.one";

Utils.runWithAllOptimizationLevels(
cx -> {
final Scriptable scope = cx.initStandardObjects();
Number result = (Number) cx.evaluateString(scope, testCode, "", 1, null);
Assert.assertEquals(2, result.intValue());
return null;
});
}

/** Equivalent javascript code of {@link #testInvokeWithPrototypeAndObjs()} */
@Test
public void testInvokeJsOnly() {
String testCode =
"'use strict'\n"
+ "function Obj() { this.myObj = {one: 1} }\n"
+ "Obj.prototype.seven = 7\n"
+ "Obj.prototype.m1 = function(i) { return i + this.myObj.one }\n"
+ "Obj.prototype.m2 = function() { return this.seven }\n"
+ "function Adapter(adapter) { this.adapter = adapter }\n"
+ "Adapter.prototype.doIt = function(i) { return this.adapter.m1(i) + this.adapter.m2() }\n"
+ "var impl = new Obj()\n"
+ "adapter = new Adapter(impl)\n"
+ "adapter.doIt(42)";

Utils.runWithAllOptimizationLevels(
cx -> {
final Scriptable scope = cx.initStandardObjects();
Number result = (Number) cx.evaluateString(scope, testCode, "", 1, null);
Assert.assertEquals(50, result.intValue());
return null;
});
}
}
Loading