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

Issue 6286060: Fix bugs 992, 1083 and 1092 (Closed)

Created:
9 years, 10 months ago by Peter Hallam
Modified:
9 years, 7 months ago
Reviewers:
Rico
CC:
v8-dev
Visibility:
Public.

Description

Fix bugs 992, 1083 and 1092 My previous patch added an assert which uncovered 1092 in the sputnik tests. This patch adds the fix for 1092, which is to ensure that NormalizeProperties does not get called for a JSGlobalProxy along all code paths. Add sputnik tests to .gitignore. BUG= TEST= Committed: http://code.google.com/p/v8/source/detail?r=6627

Patch Set 1 #

Total comments: 6

Patch Set 2 : Address review comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+335 lines, -38 lines) Patch
M .gitignore View 1 1 chunk +1 line, -0 lines 0 comments Download
M src/handles.cc View 1 chunk +1 line, -1 line 0 comments Download
M src/objects.cc View 1 2 chunks +5 lines, -0 lines 0 comments Download
M src/runtime.cc View 1 5 chunks +19 lines, -4 lines 0 comments Download
M src/v8natives.js View 4 chunks +58 lines, -31 lines 0 comments Download
M test/mjsunit/object-define-property.js View 3 chunks +135 lines, -2 lines 0 comments Download
A test/mjsunit/regress/regress-1083.js View 1 chunk +38 lines, -0 lines 0 comments Download
A test/mjsunit/regress/regress-1092.js View 1 1 chunk +35 lines, -0 lines 0 comments Download
A test/mjsunit/regress/regress-992.js View 1 chunk +43 lines, -0 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Peter Hallam
9 years, 10 months ago (2011-02-03 00:13:11 UTC) #1
Rico
LGTM with nit. Thanks a lot for fixing this. We should probably merge this to ...
9 years, 10 months ago (2011-02-03 06:49:47 UTC) #2
Peter Hallam
9 years, 10 months ago (2011-02-03 19:41:08 UTC) #3
http://codereview.chromium.org/6286060/diff/1/.gitignore
File .gitignore (right):

http://codereview.chromium.org/6286060/diff/1/.gitignore#newcode29
.gitignore:29: test/sputnik/sputniktests
On 2011/02/03 06:49:47, Rico wrote:
> I don't use git, but since this is a directory should it not be:
> /test/sputnik/sputniktests
> like the other directories?
> In addition, I think we should move it up right after /obj/ - the other
> directories seems to be in alphabetic order.

Done.

http://codereview.chromium.org/6286060/diff/1/test/mjsunit/regress/regress-10...
File test/mjsunit/regress/regress-1092.js (right):

http://codereview.chromium.org/6286060/diff/1/test/mjsunit/regress/regress-10...
test/mjsunit/regress/regress-1092.js:30: // JSGlobalProxy
On 2011/02/03 06:49:47, Rico wrote:
> Period at end of comment.

Done.

http://codereview.chromium.org/6286060/diff/1/test/mjsunit/regress/regress-10...
test/mjsunit/regress/regress-1092.js:32: 
On 2011/02/03 06:49:47, Rico wrote:
> Could we put each statement on individual lines?

Done.

Powered by Google App Engine
This is Rietveld 408576698