77856 - Enumerating Java arrays via (for..in) loops is failing

Status: VERIFIED FIXED

(Core Graveyard :: Java: Live Connect, defect)

Description

[Phil Schwartau]

These testcases are failing in the debug and optimized LiveConnect shells:

                        lc2/Arrays/array-003.js
                        lc2/Arrays/array-005.js
                        lc3/forin/array-001.js


This looks similar to bug 55114,

           "for... in loops not behaving as expected on Java strings"


However, in that bug the test failures were confined to two of the above 
tests, and looked like this: 

     Testcase lc2/Arrays/array-003.js failed 
     Failure messages were:
     number of elements enumerated: = 0 FAILED! expected: 5
     number of elements enumerated: = 0 FAILED! expected: 5

     Testcase lc3/forin/array-001.js failed 
     [ Previous Failure | Next Failure | Top of Page ]
     Failure messages were:
     new java.lang.String("ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789").getBytes();      
     length = 1 FAILED! expected: 36
     [1] = undefined FAILED! expected: 66
     [2] = undefined FAILED! expected: 67
     [3] = undefined FAILED! expected: 68
     [4] = undefined FAILED! expected: 69
     [5] = undefined FAILED! expected: 70
     [6] = undefined FAILED! expected: 71
     [7] = undefined FAILED! expected: 72
     [8] = undefined FAILED! expected: 73
     [9] = undefined FAILED! expected: 74

                    etc.
                    etc.


In the present case, the test failures look like this:

     Testcase lc2/Arrays/array-003.js failed 
     Expected exit code 0, got 3
     Testcase terminated with signal 0
     Complete testcase output was:
     ./lc2/Arrays/array-003.js:94: InternalError: Java class [B has no public                                                                            
                                      field or method named "PropertyIterator"

     Testcase lc2/Arrays/array-005.js failed 
     Expected exit code 0, got 3
     Testcase terminated with signal 0
     Complete testcase output was:
     ./lc2/Arrays/array-005.js:75: InternalError: Java class [B has no public                                                                             
                                     field or method named "PropertyIterator"

     Testcase lc3/forin/array-001.js failed 
     Expected exit code 0, got 3
     Testcase terminated with signal 0
     Complete testcase output was:
     ./lc3/forin/array-001.js:107: InternalError: Java class [B has no public                                                                             
                                     field or method named "PropertyIterator"



These three testcases all use for..in loops to iterate Java arrays.

Comment 1

[Phil Schwartau]

The line numbers in the error messages are found at: 

http://lxr.mozilla.org/mozilla/source/js/tests/lc2/Arrays/array-003.js#94
http://lxr.mozilla.org/mozilla/source/js/tests/lc2/Arrays/array-005.js#75
http://lxr.mozilla.org/mozilla/source/js/tests/lc3/forin/array-001.js#107


I only see a reference to the a class named "[B" in lc2/Arrays/array-003.js.
So I'm confused why it's mentioned in the error messages for all three tests.

Comment 2

[Phil Schwartau]

Patrick or Roger, are you able to duplicate this bug? Here's the way 
I'm running the tests on my WinNT box: ("shell.js" is a utility file)

[/d/JS_TRUNK/mozilla/js/src/liveconnect/WINNT4.0_OPT.OBJ] ./lcshell.exe
js> load('../../../tests/lc2/shell.js')
js> load('../../../tests/lc2/Arrays/array-003.js')
js> load('../../../tests/lc2/Arrays/array-005.js')
js> load('../../../tests/lc3/forin/array-001.js')

Comment 3

[Phil Schwartau]

Reduced testcase: 

js> var byte_array = (new java.lang.String("hi")).getBytes()

js> getClass(byte_array)
[JavaClass [B]

js> typeof(byte_array)
object

js> byte_array.toString()
[B@246bc4

js> byte_array[0]
104

js> byte_array.length
2

js> for (var p in byte_array) {print(p)}
12: InternalError: Java class [B has no public 
                                 field or method named "PropertyIterator"

Comment 4

[Patrick C. Beard]

Evidently, something in the way enumeration is handled as changed, and when 
LiveConnect is asked to look up the "PropertyIterator" property on a Java array, 
its failure mode is too strict. I tried having it return undefined rather than 
signalling an exception, and all is well. I'm guessing that enumerating 
properties in regular Java objects may also exhibit this problem.

Comment 5

[Patrick C. Beard]

Attachment: Fixes problem with enumerating Java arrays.

Comment 6

[Patrick C. Beard]

Ah, the reason this works for non-array Java objects in general is that 
JavaObject_lookupProperty() assigns NULL to its *objp and *propp parameters, and 
always returns JS_TRUE. A simpler change might be to do the same thing in 
JavaArray_lookupProperty. Opinions?

Comment 7

[Patrick C. Beard]

Attachment: Be consistent with jsj_JavaObject.c when property lookup fails.

Comment 8

[Patrick C. Beard]

Both patches work, but the second one is a more general solution.

Comment 9

[Patrick C. Beard]

One additional observation, JS_ReportErrorNumber() gets called from various 
places in both JSJ_JavaObject.c and JSJ_JavaArray.c, which seems unnecessary in 
the case of many property lookups, given the behavior of 
JavaObject_lookupProperty() and JavaArray_lookupProperty(), which just map these 
to undefined. Perhaps an error should only be signalled when setting a property, 
not when reading a property.

Comment 10

[Brendan Eich [:brendan]]

r/sr=[e-mail] on the second patch, it's necessary.  It's wrong to
return JS_TRUE but fail to set *objp and *propp, end of story.  Any other bad
JSObjectOps impls in liveconnect?

/be

Comment 11

[Patrick C. Beard]

shaver, care to provide an additional review?

Comment 12

[Phil Schwartau]

Need one more review to get this in for 0.9.1 ...

Comment 13

[Patrick C. Beard]

Fix checked in.

Comment 14

[Phil Schwartau]

VERIFIED FIXED on WinNT - the three testcases above no longer fail -