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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 12033011: Add Isolate parameter to Persistent class. (Closed)

Created:
7 years, 11 months ago by Sven Panne
Modified:
7 years, 11 months ago
Reviewers:
Michael Starzinger
CC:
v8-dev
Visibility:
Public.

Description

Add Isolate parameter to Persistent class. BUG=v8:2487 Committed: http://code.google.com/p/v8/source/detail?r=13501

Patch Set 1 #

Patch Set 2 : Improved IsNearDeath and IsWeak #

Patch Set 3 : Added Isolate parameter to WrapperClassId stuff #

Patch Set 4 : Added new kind of callback #

Total comments: 11

Patch Set 5 : Feedback. Rebased. #

Patch Set 6 : Added explicit #

Unified diffs Side-by-side diffs Delta from patch set Stats (+624 lines, -503 lines) Patch
M include/v8.h View 1 2 3 4 15 chunks +147 lines, -78 lines 0 comments Download
M samples/lineprocessor.cc View 1 2 3 4 1 chunk +3 lines, -2 lines 0 comments Download
M samples/process.cc View 1 2 3 4 5 chunks +11 lines, -7 lines 0 comments Download
M samples/shell.cc View 1 chunk +1 line, -1 line 0 comments Download
M src/api.cc View 1 2 3 4 1 chunk +12 lines, -73 lines 0 comments Download
M src/d8.h View 1 2 3 1 chunk +3 lines, -1 line 0 comments Download
M src/d8.cc View 1 2 3 4 7 chunks +17 lines, -14 lines 0 comments Download
M src/debug.h View 1 2 3 2 chunks +6 lines, -2 lines 0 comments Download
M src/debug.cc View 1 2 3 5 chunks +16 lines, -12 lines 0 comments Download
M src/deoptimizer.h View 1 2 3 1 chunk +3 lines, -2 lines 0 comments Download
M src/deoptimizer.cc View 1 2 3 2 chunks +5 lines, -2 lines 0 comments Download
M src/global-handles.h View 1 2 3 4 1 chunk +2 lines, -1 line 0 comments Download
M src/global-handles.cc View 1 2 3 4 9 chunks +42 lines, -19 lines 0 comments Download
M src/handles.cc View 1 2 3 2 chunks +7 lines, -3 lines 0 comments Download
M src/mksnapshot.cc View 1 chunk +1 line, -1 line 0 comments Download
M src/profile-generator.h View 1 2 3 1 chunk +2 lines, -1 line 0 comments Download
M src/profile-generator.cc View 1 2 3 4 2 chunks +6 lines, -3 lines 0 comments Download
M test/cctest/cctest.h View 2 chunks +4 lines, -1 line 0 comments Download
M test/cctest/test-api.cc View 1 2 3 4 5 65 chunks +258 lines, -227 lines 0 comments Download
M test/cctest/test-debug.cc View 3 chunks +3 lines, -3 lines 0 comments Download
M test/cctest/test-decls.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M test/cctest/test-heap.cc View 1 2 3 4 9 chunks +15 lines, -11 lines 0 comments Download
M test/cctest/test-heap-profiler.cc View 1 2 3 4 6 chunks +22 lines, -17 lines 0 comments Download
M test/cctest/test-lockers.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M test/cctest/test-log.cc View 1 2 3 4 4 chunks +6 lines, -4 lines 0 comments Download
M test/cctest/test-mark-compact.cc View 1 2 3 5 chunks +13 lines, -2 lines 0 comments Download
M test/cctest/test-regexp.cc View 1 chunk +1 line, -1 line 0 comments Download
M test/cctest/test-serialize.cc View 1 2 3 4 3 chunks +3 lines, -3 lines 0 comments Download
M test/cctest/test-thread-termination.cc View 6 chunks +6 lines, -6 lines 0 comments Download
M test/cctest/test-weakmaps.cc View 1 2 3 2 chunks +5 lines, -2 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Sven Panne
Ready for a first round of comments, I think...
7 years, 11 months ago (2013-01-22 15:38:47 UTC) #1
Michael Starzinger
LGTM (with a few comments). https://chromiumcodereview.appspot.com/12033011/diff/6001/include/v8.h File include/v8.h (right): https://chromiumcodereview.appspot.com/12033011/diff/6001/include/v8.h#newcode397 include/v8.h:397: V8_INLINE(static Persistent<T> New(Handle<T> that)); ...
7 years, 11 months ago (2013-01-24 13:09:05 UTC) #2
Sven Panne
7 years, 11 months ago (2013-01-25 08:28:40 UTC) #3 
Feedback addressed, landing...

https://codereview.chromium.org/12033011/diff/6001/include/v8.h
File include/v8.h (right):

https://codereview.chromium.org/12033011/diff/6001/include/v8.h#newcode397
include/v8.h:397: V8_INLINE(static Persistent<T> New(Handle<T> that));
On 2013/01/24 13:09:05, Michael Starzinger wrote:
> Should this be marked deprecated?

Done.

https://codereview.chromium.org/12033011/diff/6001/include/v8.h#newcode466
include/v8.h:466: V8_INLINE(bool IsNearDeath() const);
On 2013/01/24 13:09:05, Michael Starzinger wrote:
> Should this be marked deprecated?

Done.

https://codereview.chromium.org/12033011/diff/6001/include/v8.h#newcode478
include/v8.h:478: V8_INLINE(void SetWrapperClassId(uint16_t class_id));
On 2013/01/24 13:09:05, Michael Starzinger wrote:
> Should this be marked deprecated?

Done.

https://codereview.chromium.org/12033011/diff/6001/src/global-handles.cc
File src/global-handles.cc (right):

https://codereview.chromium.org/12033011/diff/6001/src/global-handles.cc#newc...
src/global-handles.cc:473: NearDeathCallback near_death_callback) {
On 2013/01/24 13:09:05, Michael Starzinger wrote:
> Add an assertion similar to the following:
> 
> ASSERT((weak_reference_callback != NULL) !=
>        (near_death_callback != NULL));

Done.

https://codereview.chromium.org/12033011/diff/6001/test/cctest/test-api.cc
File test/cctest/test-api.cc (right):

https://codereview.chromium.org/12033011/diff/6001/test/cctest/test-api.cc#ne...
test/cctest/test-api.cc:5519: prev.MakeWeak(isolate, new Snorkel(),
&HandleWeakReference);
On 2013/01/24 13:09:05, Michael Starzinger wrote:
> Can we use info.GetIsolate() here?

Done.

Powered by Google App Engine
This is Rietveld 408576698