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

Issue 1079713002: api: introduce SealHandleScope (Closed)

Created:
5 years, 8 months ago by fedor.indutny
Modified:
5 years, 8 months ago
Reviewers:
yangguo, Yang, indutny
CC:
v8-dev, Paweł Hajdan Jr.
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

api: introduce SealHandleScope When debugging Handle leaks in io.js we found it very convenient to be able to Seal some specific (root in our case) scope to prevent Handle allocations in it, and easily find leakage. R=yangguo BUG= Committed: https://crrev.com/1f85559a69fb385b9fd7d6724c6113e743ce6fec Cr-Commit-Position: refs/heads/master@{#27766}

Patch Set 1 #

Patch Set 2 : fix typo in a test #

Unified diffs Side-by-side diffs Delta from patch set Stats (+73 lines, -4 lines) Patch
M include/v8.h View 1 chunk +18 lines, -0 lines 0 comments Download
M src/api.h View 2 chunks +1 line, -4 lines 0 comments Download
M src/api.cc View 1 chunk +21 lines, -0 lines 0 comments Download
M test/cctest/cctest.status View 1 chunk +1 line, -0 lines 0 comments Download
M test/cctest/test-api.cc View 1 1 chunk +32 lines, -0 lines 0 comments Download

Messages

Total messages: 19 (5 generated)
indutny
Old CL: https://codereview.chromium.org/985873002/
5 years, 8 months ago (2015-04-10 21:57:55 UTC) #2
yangguo
On 2015/04/10 21:57:55, indutny wrote: > Old CL: https://codereview.chromium.org/985873002/ rubberstamp lgtm (assuming you simply took ...
5 years, 8 months ago (2015-04-10 22:00:45 UTC) #3
indutny
Yep, this is what I did :)
5 years, 8 months ago (2015-04-10 22:01:11 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1079713002/1
5 years, 8 months ago (2015-04-10 22:02:30 UTC) #6
fedor.indutny
Thank you!
5 years, 8 months ago (2015-04-10 22:02:35 UTC) #7
commit-bot: I haz the power
Try jobs failed on following builders: v8_presubmit on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/1914)
5 years, 8 months ago (2015-04-10 22:06:38 UTC) #9
fedor.indutny
Yang, I think it had a typo in a test. Fixed it in my fork. ...
5 years, 8 months ago (2015-04-10 22:11:00 UTC) #10
fedor.indutny
Updated, do I need another LGTM to start it over?
5 years, 8 months ago (2015-04-10 22:20:51 UTC) #11
Yang
On 2015/04/10 22:20:51, fedor wrote: > Updated, do I need another LGTM to start it ...
5 years, 8 months ago (2015-04-10 22:27:14 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1079713002/20001
5 years, 8 months ago (2015-04-10 22:37:12 UTC) #15
commit-bot: I haz the power
Committed patchset #2 (id:20001)
5 years, 8 months ago (2015-04-10 23:17:16 UTC) #16
commit-bot: I haz the power
Patchset 2 (id:??) landed as https://crrev.com/1f85559a69fb385b9fd7d6724c6113e743ce6fec Cr-Commit-Position: refs/heads/master@{#27766}
5 years, 8 months ago (2015-04-10 23:17:26 UTC) #17
fedor.indutny
On 2015/04/10 23:17:26, I haz the power (commit-bot) wrote: > Patchset 2 (id:??) landed as ...
5 years, 8 months ago (2015-04-11 08:16:18 UTC) #18
fedor.indutny
5 years, 8 months ago (2015-04-11 09:14:41 UTC) #19
Message was sent while issue was closed.
Yang,

One last question - is it going to be released in next 4.2?

Thank you,
Fedor.

Powered by Google App Engine
This is Rietveld 408576698