Bug 185197 – Iterator of Array.keys() returns object in wrong order

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Amanda Ferraz]

Hi everyone,

according to ES6 specification (https://www.ecma-international.org/ecma-262/8.0/index.html#sec-createiterresultobject), the Iterator
of a TypedArray.keys() or an Array.keys() should return an object on this order {value: Integer, done: Boolean}.

Step to reproduce:
    var arr = new Int8Array([1,2,3]);
    var iterator = arr.keys();
    var a = iterator.next();
    var toString = JSON.stringify(a); // just to check if it is the same string
    if (!(toString === JSON.stringify({value: 0, done: false})))
        throw new Error('expected {"value": 0, "done": false}, got ' + toString)

Actual results:
Error: expected {"value": 0, "done": false}, got {"done": false, "value": 0}

Expected results:
Pass without failures

Other JavaScript engines such as V8, SpiderMonkey and Chakra works as expected.

[Amanda Ferraz]

Additional information (versions):
   OS: Ubuntu 17.04
   Version (tag): 606.1.9.4

[Yusuke Suzuki]

Created attachment 344759 [details]
Patch

[Keith Miller]

Comment on attachment 344759 [details]
Patch

r=me.

[Joseph Pecoraro]

Comment on attachment 344759 [details]
Patch

Neat!. Was this covered by test262?

[EWS Watchlist]

Comment on attachment 344759 [details]
Patch

Attachment 344759 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8505610

New failing tests:
http/tests/security/canvas-remote-read-remote-video-localhost.html

[EWS Watchlist]

Created attachment 344769 [details]
Archive of layout-test-results from ews204 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews204  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Yusuke Suzuki]

(In reply to Joseph Pecoraro from comment #4)
> Comment on attachment 344759 [details]
> Patch
> 
> Neat!. Was this covered by test262?

Interestingly, no! It would be fine if we can add it to test262 :D

[Saam Barati]

(In reply to Yusuke Suzuki from comment #7)
> (In reply to Joseph Pecoraro from comment #4)
> > Comment on attachment 344759 [details]
> > Patch
> > 
> > Neat!. Was this covered by test262?
> 
> Interestingly, no! It would be fine if we can add it to test262 :D

Is this even specified behavior? I thought ordering of properties was implementation dependent?

[WebKit Commit Bot]

Comment on attachment 344759 [details]
Patch

Clearing flags on attachment: 344759

Committed r233740: <https://trac.webkit.org/changeset/233740>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/42086770>

[Yusuke Suzuki]

(In reply to Saam Barati from comment #8)
> (In reply to Yusuke Suzuki from comment #7)
> > (In reply to Joseph Pecoraro from comment #4)
> > > Comment on attachment 344759 [details]
> > > Patch
> > > 
> > > Neat!. Was this covered by test262?
> > 
> > Interestingly, no! It would be fine if we can add it to test262 :D
> 
> Is this even specified behavior? I thought ordering of properties was
> implementation dependent?

I'm not sure The spec defines properties in particular order.
https://www.ecma-international.org/ecma-262/8.0/index.html#sec-createiterresultobject
I don't know whether this definition order would be visible in Object properties definition order.
While object needs to show properties in definition order, it is not related to that the above abstract operation requires that behavior (it just describes abstract operation as a example).
IMO, this is not required by the spec and this should be implementation dependent thing.

But anyway, aligning to the example algorithm in the spec here would be no harm (even if it is an implementation dependent).
If it becomes related to the performance problem, we should consider about it seriously (whether it is required in the spec).

[Keith Miller]

(In reply to Yusuke Suzuki from comment #12)
> (In reply to Saam Barati from comment #8)
> > (In reply to Yusuke Suzuki from comment #7)
> > > (In reply to Joseph Pecoraro from comment #4)
> > > > Comment on attachment 344759 [details]
> > > > Patch
> > > > 
> > > > Neat!. Was this covered by test262?
> > > 
> > > Interestingly, no! It would be fine if we can add it to test262 :D
> > 
> > Is this even specified behavior? I thought ordering of properties was
> > implementation dependent?
> 
> I'm not sure The spec defines properties in particular order.
> https://www.ecma-international.org/ecma-262/8.0/index.html#sec-
> createiterresultobject
> I don't know whether this definition order would be visible in Object
> properties definition order.
> While object needs to show properties in definition order, it is not related
> to that the above abstract operation requires that behavior (it just
> describes abstract operation as a example).
> IMO, this is not required by the spec and this should be implementation
> dependent thing.
> 
> But anyway, aligning to the example algorithm in the spec here would be no
> harm (even if it is an implementation dependent).
> If it becomes related to the performance problem, we should consider about
> it seriously (whether it is required in the spec).

I'm pretty sure we were wrong because of the way https://tc39.github.io/ecma262/#sec-ordinaryownpropertykeys works. We are be expected to enumerate the named properties in chronological order. Since createIteratorResultObject puts value first value should be enumerated first.

cinfuzz