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

Issue 1745483002: Reland of Make Intl install properties more like how other builtins do (patchset #1 id:1 of https:/… (Closed)

Created:
4 years, 10 months ago by Dan Ehrenberg
Modified:
4 years, 10 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

Reland of Make Intl install properties more like how other builtins do (patchset #1 id:1 of https://codereview.chromium.org/1733293003/ ) This reland fixes a bug by pulling properties off the utils object, so that it can be garbage collected in nosnap builds. Original commit message: Intl has been somewhat of an oddball for how it integrates with V8. One aspect is that it largely didn't use utils to install itself into the snapshot, which led to some missing names, which new test262 tests check for, and duplicated code. This patch brings Intl a bit closer to how the rest of the builtins do things, though not entirely as it is currently structured to do unusual things, such as creating new constructors from JavaScript rather than C++. New test262 tests check for some of the names that are added in this patch. R=adamk CC=jshin BUG=v8:4778 LOG=Y Committed: https://crrev.com/88d7c59c45dbb65a42a2789748f051a6879282aa Cr-Commit-Position: refs/heads/master@{#34337}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+105 lines, -135 lines) Patch
M src/js/i18n.js View 25 chunks +105 lines, -135 lines 0 comments Download

Messages

Total messages: 10 (4 generated)
Dan Ehrenberg
4 years, 10 months ago (2016-02-26 19:18:34 UTC) #2
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1745483002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1745483002/1
4 years, 10 months ago (2016-02-26 19:18:39 UTC) #3
adamk
lgtm
4 years, 10 months ago (2016-02-26 19:21:44 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1745483002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1745483002/1
4 years, 10 months ago (2016-02-26 19:27:10 UTC) #7
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 10 months ago (2016-02-26 19:39:48 UTC) #8
commit-bot: I haz the power
4 years, 10 months ago (2016-02-26 19:41:03 UTC) #10
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/88d7c59c45dbb65a42a2789748f051a6879282aa
Cr-Commit-Position: refs/heads/master@{#34337}

Powered by Google App Engine
This is Rietveld 408576698