Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(761)

Issue 1583773005: Construct instances of base class from TypedArray.prototype.subarray (Closed)

Created:
4 years, 11 months ago by Dan Ehrenberg
Modified:
4 years, 11 months ago
Reviewers:
adamk
CC:
v8-reviews_googlegroups.com
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

Construct instances of base class from TypedArray.prototype.subarray Previous changes with subclassable builtins and @@species were a bit aggressive in making TypedArray.prototype.subarray act like the ES2016 specification in terms of returning an instance of the subclass as a result. It turns out that Node.js, and extracted libraries for the web, subclass TypedArrays but don't expect the subclass constructor to be called by subarray. @@species will provide an escape hatch, but it has not shipped yet, and will take some time for uptake by libraries. For now, this patch makes TypedArray.prototype.subarray fall back to constructing an instance of the parent TypedArray class, such as Uint8Array. R=adamk LOG=Y BUG=v8:4665 Committed: https://crrev.com/e13f2ff40bcf2dcfa94d4424438f6d0d3b32acee Cr-Commit-Position: refs/heads/master@{#33312}

Patch Set 1 #

Patch Set 2 : Add test case in bug as a regression test #

Unified diffs Side-by-side diffs Delta from patch set Stats (+40 lines, -3 lines) Patch
M src/js/typedarray.js View 2 chunks +4 lines, -3 lines 0 comments Download
M test/mjsunit/es6/legacy-subclassing.js View 1 chunk +3 lines, -0 lines 0 comments Download
A test/mjsunit/regress/regress-4665.js View 1 1 chunk +33 lines, -0 lines 0 comments Download

Messages

Total messages: 13 (5 generated)
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1583773005/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1583773005/1
4 years, 11 months ago (2016-01-14 05:29:42 UTC) #2
Dan Ehrenberg
4 years, 11 months ago (2016-01-14 05:38:25 UTC) #3
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1583773005/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1583773005/20001
4 years, 11 months ago (2016-01-14 05:50:35 UTC) #5
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
4 years, 11 months ago (2016-01-14 06:33:43 UTC) #7
adamk
lgtm, and thanks for the quick and minimal fix as a stop-gap
4 years, 11 months ago (2016-01-14 19:02:20 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1583773005/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1583773005/20001
4 years, 11 months ago (2016-01-14 19:21:45 UTC) #10
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 11 months ago (2016-01-14 19:22:50 UTC) #11
commit-bot: I haz the power
4 years, 11 months ago (2016-01-14 19:23:32 UTC) #13
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/e13f2ff40bcf2dcfa94d4424438f6d0d3b32acee
Cr-Commit-Position: refs/heads/master@{#33312}

Powered by Google App Engine
This is Rietveld 408576698