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

Issue 61883002: Handlify ForceSetObjectProperty (Closed)

Created:
7 years, 1 month ago by rafaelw
Modified:
7 years, 1 month ago
Reviewers:
Michael Starzinger
CC:
v8-dev
Visibility:
Public.

Description

Handlify ForceSetObjectProperty Note that I've left the layering as is to make the diffs clear. Is it worth moving ForceSetObjectProperty to objects.cc? This code is clearly implementing part of the DefineOrRedefine steps from the spec, but it's still odd that it lives in Runtime. Note that handles.cc exposes a ForceSetProperty which just performs a CALL_HEAP_FUNCTION on the Runtime::ForceSetObjectProperty -- which is exposed to the api as v8::Object::ForceSet BUG= R=mstarzinger@chromium.org Committed: https://code.google.com/p/v8/source/detail?r=17529

Patch Set 1 #

Patch Set 2 : pre review #

Patch Set 3 : remove handlescope #

Total comments: 1

Patch Set 4 : cr changes #

Unified diffs Side-by-side diffs Delta from patch set Stats (+42 lines, -45 lines) Patch
M src/handles.cc View 1 chunk +2 lines, -6 lines 0 comments Download
M src/objects.h View 1 chunk +1 line, -0 lines 0 comments Download
M src/objects.cc View 1 2 3 2 chunks +3 lines, -1 line 0 comments Download
M src/runtime.h View 1 chunk +1 line, -1 line 0 comments Download
M src/runtime.cc View 1 2 3 7 chunks +35 lines, -37 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
rafaelw
7 years, 1 month ago (2013-11-06 12:37:39 UTC) #1
rafaelw
ptal
7 years, 1 month ago (2013-11-06 13:03:37 UTC) #2
Michael Starzinger
LGTM. Yes, to me personally it makes sense to move this functionality from the Runtime ...
7 years, 1 month ago (2013-11-06 14:56:59 UTC) #3
rafaelw
7 years, 1 month ago (2013-11-06 16:32:58 UTC) #4
Message was sent while issue was closed.
Committed patchset #4 manually as r17529 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698