The slice method is required for standards conformance.

Comment on attachment 8698958 [details] [diff] [review] SharedArrayBuffer.prototype.slice I'm going to do things in the opposite order (subclassing first, then slice). Cancelling review for now. Sorry for the trouble.

The dev doc update once this lands is at least the following: A new page about the slice method, see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/SharedArrayBuffer#SharedArrayBuffer_prototype_object

Implements %SharedArrayBuffer%.prototype.slice, and also the species accessor which is needed for proper subclassing behavior.

Test cases for %SharedArrayBuffer%.prototype.slice, and also one for byteLength.

Comment on attachment 8805902 [details] [diff] [review] bug1232966-sab-slice.patch Review of attachment 8805902 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/vm/CommonPropertyNames.h @@ +273,5 @@ > macro(SetIterator, SetIterator, "Set Iterator") \ > macro(setPrefix, setPrefix, "set ") \ > macro(setPrototypeOf, setPrototypeOf, "setPrototypeOf") \ > macro(shape, shape, "shape") \ > + macro(SharedArrayBufferSpecies, SharedArrayBufferSpecies, "SharedArrayBufferSpecies") \ This isn't used anywhere in this patch. would be better removing for now.

Comment on attachment 8805903 [details] [diff] [review] bug1232966-sab-slice-tests.patch Review of attachment 8805903 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit-test/tests/sharedbuf/methods.js @@ +4,5 @@ > + quit(0); > + > +load(libdir + "asserts.js"); > + > +// Test the byteLength method It should be better separating files per each method. byteLength.js and slice.js, or something like that with prefix. so that you don't need the block statement, and it would become clear what goes wrong if some of them hit regression. @@ +7,5 @@ > + > +// Test the byteLength method > + > +{ > + let v = new SharedArrayBuffer(137); can this variable be named `buffer` or something? same for other 1-2 length variables (except `i`), they need more descriptive name.

You will have to update the test for slice in test_xrayToJS.xul, that I just landed in bug 1130988.

https://hg.mozilla.org/integration/mozilla-inbound/rev/f16586fdc3fd11dbf541f1f8b12296165b3b0d86 Bug 1232966 - SharedArrayBuffer.prototype.slice. r=arai https://hg.mozilla.org/integration/mozilla-inbound/rev/6892ec7e77f3e244473498e3c0c3731939227170 Bug 1232966 - test cases for SharedArrayBuffer.prototype.slice. r=arai

sorry had to backout for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=38469517&repo=mozilla-inbound

In a debug build, fails with --ion-eager --ion-offthread-compile=off: Assertion failure: fromBuffer->byteLength() >= fromIndex + count, at /Users/lhansen/moz/mozilla-inbound/js/src/vm/SharedArrayObject.cpp:337 (Will also add a patch for Tom's case.)

Backout by cbook@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/f3334bb262d3 Backed out changeset 6892ec7e77f3 https://hg.mozilla.org/integration/mozilla-inbound/rev/017b9ab11023 Backed out changeset f16586fdc3fd for arm bustage

A failure on my part to set up inlinable intrinsics correctly. There's really no particular reason for these to be inlinable; they are not going to be hot. So it's an easy fix.

With this patch (suitably corrected) there's a fascinating new failure in an existing test: in ecma_6/TypedArray/Tconstructor-fromTypedArray-byteLength.js. The direct cause of the failure is that an object that is an ArrayBuffer (it prints as "[ArrayBuffer]" and believes itself to be that) has a prototype that is SharedArrayBuffer.prototype. Hence when we call the byteLength getter on that object there's a failure, since the byteLength getter is nongeneric and SharedArrayBuffer's byteLength fails the type check when presented with an arrayBuffer. How we end up with the wrong prototype is unknown. What is known is that the patch on the present bug fixes subclassing of SharedArrayBuffer, which was not previously working. There could be a bug in that fix, or the fix could be uncovering a previous bug, either in the VM or in the test case. The test case relies on some particularly gnarly reflection to construct a set of constructors for TypedArrays that carry SharedArrayBuffers... See ecma_{6,7}/TypedArray/shell.js.

The fascinating failure is being addressed in bug 1314564.

Adaptation of Xrays test case.

Comment on attachment 8807147 [details] [diff] [review] bug1232966-sab-slice-xray.patch Review of attachment 8807147 [details] [diff] [review]: ----------------------------------------------------------------- I am glad to see this being fixed now!

Comment on attachment 8807147 [details] [diff] [review] bug1232966-sab-slice-xray.patch Boris, evilpie approved this but it probably needs your stamp - one more property going onto SharedArrayBuffer.

Comment on attachment 8807147 [details] [diff] [review] bug1232966-sab-slice-xray.patch r=me

Try build is green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=95505b97467d

https://hg.mozilla.org/integration/mozilla-inbound/rev/334620bedacab85bcfed7934a714d95d9945455d Bug 1232966 - SharedArrayBuffer.prototype.slice. r=arai https://hg.mozilla.org/integration/mozilla-inbound/rev/55398c6f895777f83bdec5cc4bbef3af9f958930 Bug 1232966 - test cases for SharedArrayBuffer.prototype.slice. r=arai https://hg.mozilla.org/integration/mozilla-inbound/rev/00c0fdd359040cf9c5515522a620f06ea26cc6ba Bug 1232966 - Adapt Xrays tests for SharedArrayBuffer.prototype.slice. r=bz

https://hg.mozilla.org/mozilla-central/rev/334620bedaca https://hg.mozilla.org/mozilla-central/rev/55398c6f8957 https://hg.mozilla.org/mozilla-central/rev/00c0fdd35904

New page: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/SharedArrayBuffer/slice Updated: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/SharedArrayBuffer#Methods As this isn't in a Firefox release yet, it won't appear on the "Firefox for developers" release notes. Shared memory is listed as an experimental feature, though: https://developer.mozilla.org/en-US/Firefox/Experimental_features#JavaScript