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

The $262 object does not use LiveConnect any more #1664

Merged

Conversation

rPraml
Copy link
Contributor

@rPraml rPraml commented Sep 30, 2024

I noticed, that the $262 object, which defines the host functions was wrapped into a NativeJavaObject.

In other words, the @JsMethod annotations were useless, instead, the methods were exposed by LiveConnect.

In this PR, I've changed the $262 to a subclass of ScriptableObject, so that the annotations take place and we do not longer depend on LC when running the 262 tests.


// same as cx.evaluateString(scope, "var $262 = new __262__()", "<init>", 1, null);
BaseFunction ctor = (BaseFunction) scope.get("__262__", scope);
$262.install(scope, ctor.construct(cx, scope, new Object[0]));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The initialization looks a bit strange to me, maybe some experts can give me hints how do I write cleaner code here?
I also took __262__ as class name, which is somehow required

@gbrail
Copy link
Collaborator

gbrail commented Oct 1, 2024

The initialization makes sense to me if it's using the reflection based attributes there. You could also just create a ScriptableObject and attach LambdaFunctions to it if you'd like to do it a different way -- the latter is a bit faster once it's set up. There's a method in ScriptableObject that does it quickly but it's protected -- but it could be public.

proto.setPrototype(getObjectPrototype(scope));
proto.setParentScope(scope);

proto.defineProperty("toSource", "Math", DONTENUM | READONLY | PERMANENT);
Copy link
Collaborator

Choose a reason for hiding this comment

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

this looks strange to me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy-paste error

@rbri
Copy link
Collaborator

rbri commented Oct 1, 2024

Great - the lambda thing is much better - at least from my point of view - thanks a lot

@rPraml rPraml force-pushed the test-262-no-dependency-on-liveconnect branch from 0492c78 to 9caa579 Compare October 1, 2024 08:29
@gbrail
Copy link
Collaborator

gbrail commented Oct 2, 2024

Thanks -- looks good!

@gbrail gbrail merged commit ed9af7e into mozilla:master Oct 2, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants