From 893a8f05eed24b95e232cb28a9c7f1b4a3ff3c40 Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Wed, 28 Feb 2024 13:40:57 +0100 Subject: [PATCH 1/5] When InterfaceAdapter is used, the wrong thisObj is used --- .../mozilla/javascript/InterfaceAdapter.java | 3 +- .../tests/JavaAdapterInvokeTest.java | 111 ++++++++++++++++++ 2 files changed, 112 insertions(+), 2 deletions(-) create mode 100644 testsrc/org/mozilla/javascript/tests/JavaAdapterInvokeTest.java diff --git a/src/org/mozilla/javascript/InterfaceAdapter.java b/src/org/mozilla/javascript/InterfaceAdapter.java index 513101920b..e850522a42 100644 --- a/src/org/mozilla/javascript/InterfaceAdapter.java +++ b/src/org/mozilla/javascript/InterfaceAdapter.java @@ -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; diff --git a/testsrc/org/mozilla/javascript/tests/JavaAdapterInvokeTest.java b/testsrc/org/mozilla/javascript/tests/JavaAdapterInvokeTest.java new file mode 100644 index 0000000000..6ef569f6ed --- /dev/null +++ b/testsrc/org/mozilla/javascript/tests/JavaAdapterInvokeTest.java @@ -0,0 +1,111 @@ +package org.mozilla.javascript.tests; + +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.mozilla.javascript.Context; +import org.mozilla.javascript.Scriptable; + +public class JavaAdapterInvokeTest { + Context cx = null; + Scriptable topScope = null; + + @Before + public void enterContext() { + cx = Context.enter(); + cx.setOptimizationLevel(-1); + topScope = cx.initStandardObjects(); + } + + @After + public void exitContext() { + Context.exit(); + } + + 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(); + } + } + + @Test + public void testInvoke() throws NoSuchMethodException { + 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)"; + + Number result = (Number) cx.evaluateString(topScope, testCode, "", 1, null); + Assert.assertEquals(50, result.intValue()); + } + + @Test + public void testInvokeWithPrototype() throws NoSuchMethodException { + 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)"; + + Number result = (Number) cx.evaluateString(topScope, testCode, "", 1, null); + Assert.assertEquals(50, result.intValue()); + } + + @Test + public void testInvokeWithPrototypeAndCtor() throws NoSuchMethodException { + String testCode = + "'use strict'\n" + + "function Obj() { this.myObj = {one: 1} }\n" + + "Obj.prototype.m1 = function(i) { return i + this.myObj.one }\n" + + "Obj.prototype.m2 = function() { return 7 }\n" + + "var impl = new Obj()\n" + + "adapter = new Packages." + + AdapterClass.class.getName() + + "(impl)\n" + + "adapter.doIt(42)"; + + Number result = (Number) cx.evaluateString(topScope, testCode, "", 1, null); + Assert.assertEquals(50, result.intValue()); + } + + @Test + public void testInvokeJsOnly() throws NoSuchMethodException { + String testCode = + "'use strict'\n" + + "function Obj() { this.myObj = {one: 1} }\n" + + "Obj.prototype.m1 = function(i) { return i + this.myObj.one }\n" + + "Obj.prototype.m2 = function() { return 7 }\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)"; + + Number result = (Number) cx.evaluateString(topScope, testCode, "", 1, null); + Assert.assertEquals(50, result.intValue()); + } +} From 1c765271d237f9808d77fe533999dee9074173f2 Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Thu, 22 Aug 2024 08:13:35 +0200 Subject: [PATCH 2/5] Refactored test --- .../tests/JavaAdapterInvokeTest.java | 81 +++++++++++-------- 1 file changed, 49 insertions(+), 32 deletions(-) diff --git a/tests/src/test/java/org/mozilla/javascript/tests/JavaAdapterInvokeTest.java b/tests/src/test/java/org/mozilla/javascript/tests/JavaAdapterInvokeTest.java index 6ef569f6ed..f7c7159ac6 100644 --- a/tests/src/test/java/org/mozilla/javascript/tests/JavaAdapterInvokeTest.java +++ b/tests/src/test/java/org/mozilla/javascript/tests/JavaAdapterInvokeTest.java @@ -1,27 +1,10 @@ package org.mozilla.javascript.tests; -import org.junit.After; import org.junit.Assert; -import org.junit.Before; import org.junit.Test; -import org.mozilla.javascript.Context; import org.mozilla.javascript.Scriptable; public class JavaAdapterInvokeTest { - Context cx = null; - Scriptable topScope = null; - - @Before - public void enterContext() { - cx = Context.enter(); - cx.setOptimizationLevel(-1); - topScope = cx.initStandardObjects(); - } - - @After - public void exitContext() { - Context.exit(); - } public interface AdapterInterface { int m1(int i); @@ -41,8 +24,15 @@ public int doIt(int i) { } } + /** + * This test creates a new {@link AdapterClass} and passes a javascript object with two + * functions (m1,m2) to the constructor. + * + *

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() throws NoSuchMethodException { + public void testInvoke() { String testCode = "'use strict'\n" + "var impl = {" @@ -53,13 +43,18 @@ public void testInvoke() throws NoSuchMethodException { + AdapterClass.class.getName() + "(impl)\n" + "adapter.doIt(42)"; - - Number result = (Number) cx.evaluateString(topScope, testCode, "", 1, null); - Assert.assertEquals(50, result.intValue()); + 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() throws NoSuchMethodException { + public void testInvokeWithPrototype() { String testCode = "'use strict'\n" + "function Obj() {}\n" @@ -71,41 +66,63 @@ public void testInvokeWithPrototype() throws NoSuchMethodException { + "(impl)\n" + "adapter.doIt(42)"; - Number result = (Number) cx.evaluateString(topScope, testCode, "", 1, null); - Assert.assertEquals(50, result.intValue()); + 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 testInvokeWithPrototypeAndCtor() throws NoSuchMethodException { + 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 7 }\n" + + "Obj.prototype.m2 = function() { return this.seven }\n" + "var impl = new Obj()\n" + "adapter = new Packages." + AdapterClass.class.getName() + "(impl)\n" + "adapter.doIt(42)"; - Number result = (Number) cx.evaluateString(topScope, testCode, "", 1, null); - Assert.assertEquals(50, result.intValue()); + Utils.runWithAllOptimizationLevels( + cx -> { + final Scriptable scope = cx.initStandardObjects(); + Number result = (Number) cx.evaluateString(scope, testCode, "", 1, null); + Assert.assertEquals(50, result.intValue()); + return null; + }); } + /** Equivalent javascript code of {@link #testInvokeWithPrototypeAndObjs()} */ @Test - public void testInvokeJsOnly() throws NoSuchMethodException { + 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 7 }\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)"; - Number result = (Number) cx.evaluateString(topScope, testCode, "", 1, null); - Assert.assertEquals(50, result.intValue()); + Utils.runWithAllOptimizationLevels( + cx -> { + final Scriptable scope = cx.initStandardObjects(); + Number result = (Number) cx.evaluateString(scope, testCode, "", 1, null); + Assert.assertEquals(50, result.intValue()); + return null; + }); } } From 7b9cade1006ee2813ae4297605275221af7532fd Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Tue, 24 Sep 2024 15:10:14 +0200 Subject: [PATCH 3/5] add more realistic test with java-lang standards --- .../tests/JavaAdapterInvokeTest.java | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/src/test/java/org/mozilla/javascript/tests/JavaAdapterInvokeTest.java b/tests/src/test/java/org/mozilla/javascript/tests/JavaAdapterInvokeTest.java index f7c7159ac6..56ff46033b 100644 --- a/tests/src/test/java/org/mozilla/javascript/tests/JavaAdapterInvokeTest.java +++ b/tests/src/test/java/org/mozilla/javascript/tests/JavaAdapterInvokeTest.java @@ -102,6 +102,27 @@ public void testInvokeWithPrototypeAndObjs() { }); } + @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"; // we do not ue start here (as we do not catch the error + + 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() { From e2699bda93319bf55bd6baf2fe921ede5869ca8a Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Tue, 24 Sep 2024 15:15:34 +0200 Subject: [PATCH 4/5] remove wrong comment --- .../org/mozilla/javascript/tests/JavaAdapterInvokeTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/src/test/java/org/mozilla/javascript/tests/JavaAdapterInvokeTest.java b/tests/src/test/java/org/mozilla/javascript/tests/JavaAdapterInvokeTest.java index 56ff46033b..4d389acfdf 100644 --- a/tests/src/test/java/org/mozilla/javascript/tests/JavaAdapterInvokeTest.java +++ b/tests/src/test/java/org/mozilla/javascript/tests/JavaAdapterInvokeTest.java @@ -112,7 +112,7 @@ public void testJavaLangThread() { + "var thread = new java.lang.Thread(runnable)\n" + "thread.start()\n" + "thread.join()\n" - + "runnable.myObj.one"; // we do not ue start here (as we do not catch the error + + "runnable.myObj.one"; Utils.runWithAllOptimizationLevels( cx -> { From 70bba4d973990bf03644e60d9ecfac638f406d1a Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Tue, 24 Sep 2024 15:17:55 +0200 Subject: [PATCH 5/5] Spotless --- .../org/mozilla/javascript/tests/JavaAdapterInvokeTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/src/test/java/org/mozilla/javascript/tests/JavaAdapterInvokeTest.java b/tests/src/test/java/org/mozilla/javascript/tests/JavaAdapterInvokeTest.java index 4d389acfdf..5d42fb5d09 100644 --- a/tests/src/test/java/org/mozilla/javascript/tests/JavaAdapterInvokeTest.java +++ b/tests/src/test/java/org/mozilla/javascript/tests/JavaAdapterInvokeTest.java @@ -117,7 +117,7 @@ public void testJavaLangThread() { Utils.runWithAllOptimizationLevels( cx -> { final Scriptable scope = cx.initStandardObjects(); - Number result = (Number) cx.evaluateString(scope, testCode, "", 1, null); + Number result = (Number) cx.evaluateString(scope, testCode, "", 1, null); Assert.assertEquals(2, result.intValue()); return null; });