Custom WrapFactory to handle byte[] no longer working in 1.9.0

[CardContact]

In OpenSCDP we use a lot of binary data and the Global Platform Scripting Spec that we implement
has a dedicated ByteString and ByteBuffer host class.

In our embedding we use a custom WrapFactory that converts a byte[] returned from native
Java invocations into ByteString instances:

>var bs = java.lang.reflect.Array.newInstance(java.lang.Byte.TYPE, 10);
>bs instanceof ByteString
true

With 1.9 this stops working, as WrapFactory.wrap() is no longer called for byte[] objects. Instead this version seems to generate NativeJavaArray objects.

Before we rewrite the host objects to support NativeJavaArray, I want to make sure that there is no other way to restore the previous logic.

[aardvark179]

WrapFactory has certainly seen some work, but I believe wrap should still be called in most cases, but I see @ZZZank short circuited it and called wrapAsJavaObject directly in InterfaceAdapter and NativeJavaObject. I'd suggest moving your customisation their as that has been a public method for a long time, and documented as being called by wrap.

I think you may be doing some fairly uncommon things in your embedding, so it would be really helpful if you can turn those into unit tests so we have a better idea of the behaviour you are depending on, and if we change it then it should be more obvious from the changes in the tests.

[aardvark179]

Actually, I see quite a few custom wrap factories, we may need look at how much behaviour we are covering for custom wrap factories and make sure we aren't causing accidental regressions.

[CardContact]

I replaced wrap with wrapAsJavaObject and that is working fine.

[ZZZank]

Cna you elaborate how did you customize the WrapFactory? In the provided code example, byte[] is created via Array.newInstance(...) method call, which will indeed call wrap instead of wrapAsJavaObject to wrap the result:

rhino/rhino/src/main/java/org/mozilla/javascript/NativeJavaMethod.java

Lines 209 to 213 in c9b182a

	Object wrapped =
	cx.getWrapFactory()
	.wrap(
	cx, scope,
	returnValue, returnType);

(see Line 211)

[CardContact]

Yes, that used to work up until 1.7

From 1.8 I could see in the debugger that wrap() was not called, but wrapAsJavaObject was called instead.

[ZZZank]

I think I get it. WrapFactory after Rhino 1.8.0 provides two wrap(...) methods:

rhino/rhino/src/main/java/org/mozilla/javascript/WrapFactory.java

Lines 49 to 54 in c9b182a

	public Object wrap(Context cx, Scriptable scope, Object obj, Class<?> staticType) {
	return wrap(cx, scope, obj, TypeInfoFactory.GLOBAL.create(staticType));
	}
	public Object wrap(Context cx, Scriptable scope, Object obj, TypeInfo staticType) {
	if (obj == null || obj == Undefined.instance || obj instanceof Scriptable) {

The TypeInfo one is newly added, which is also the "main" method that should be overridden. But unfortunately, there's no "1.8.0 migration guide" or similar things to notify Rhino user about this, and due to old methods not being removed, there wont be even any compiler warnings.

So you might want to check the wrap(...) method you customized, and replace the Class<?> param type, like so:

public class CustomWrapFactory extends WrapFactory {
    @Override
    public Object wrap(Context cx, Scriptable scope, Object obj, TypeInfo staticType) {
        // Replace Class<?> with TypeInfo --------------------->  ^^^^^^
        if (obj instanceof byte[]) {
            return null;
        }
        return super.wrap(cx, scope, obj, staticType);
    }
}