Please review changes for JDK-8014519: scriptpad sample does not work with nashorn

[Andreas Rieber]

Answers inline...

On 14.05.13 18:25, Attila Szegedi wrote:
> Hey Adreas,
>
> few things I'm curious about:
>
> -    if (! (callback instanceof Function)) {
> +    if (! (callback instanceof Function) && typeof callback !== "function") {
>
> When would you have a callback that is instanceof Function, but its typeof returns something other than "function"?
Patch is already some days old but as i remember it was Rhino/Nashorn 
differences and the case when someone would try:

     setTimeout( "print('something')", 2000);

>
> function mbean(objName, async) {
> +    var index;
>       objName = objectName(objName);
> -    var info = mbeanInfo(objName);
> +    var info = mbeanInfo(objName);
>       var attrs = info.attributes;
>       var attrMap = new Object;
> -    for (var index in attrs) {
> +    for (index in attrs) {
>           attrMap[attrs[index].name] = attrs[index];
>       }
>       var opers = info.operations;
>       var operMap = new Object;
> -    for (var index in opers) {
> +    for (index in opers) {
>           operMap[opers[index].name] = opers[index];
>       }
>
> What do we gain from extracting "var index" from loop declarations into a function-level variable?
The 'var index' has function scope, so a second 'var index' is wrong. 
There is no block scope in javascript ;-)

>
>   // allocate an integer array of "big enough" size!
>   var a = java.lang.reflect.Array.newInstance(
> -            java.lang.Integer.TYPE, 1024*1024);
> -
> -// loop forever!
> -while (true);
> +    java.lang.Integer.TYPE, input * 1024 * 1024);
>
> I know this doesn't pertain to the reason for this being in the patch, just wanted to note that in Nashorn you can use Java.type() to obtain the type for int[], and you can subsequently use that as a constructor to instantiate a new Java int array:
>
> var a = new (Java.type("int[]"))(input * 1024 * 1024)
>
> You don't have to go through java.lang.reflect.Array.
True, i didn't change that to keep Nashorn and Rhino working. Now it 
could be changed the Nashorn way.

cheers
Andreas

>
> Regards,
>    Attila.
>
> On May 14, 2013, at 5:13 PM, "A. Sundararajan" <sundararajan.athijegannathan at oracle.com> wrote:
>
>> Please review http://cr.openjdk.java.net/~sundar/8014519/
>>
>> This is a contribution by Andreas Rieber
>>
>> Thanks
>> -Sundar