Chromium Code Reviews
Help | Chromium Project | Sign in
(1079)

Issue 11340010: Correctly visit all external strings. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 5 months ago by Yang
Modified:
1 year, 5 months ago
Reviewers:
erikcorry, Erik Corry
CC:
v8-dev_googlegroups.com, yurys
Visibility:
Public.

Description

Correctly visit all external strings.

BUG=

Committed: https://code.google.com/p/v8/source/detail?r=12841

Patch Set 1 #

Total comments: 2

Patch Set 2 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+75 lines, -32 lines) Lint Patch
M include/v8.h View 1 1 chunk +2 lines, -2 lines 0 comments 0 errors Download
M src/heap.cc View 1 2 chunks +33 lines, -5 lines 0 comments 0 errors Download
M test/cctest/test-api.cc View 1 3 chunks +40 lines, -25 lines 0 comments 0 errors Download
Trybot results:
Commit:

Messages

Total messages: 5
Yang
Please take a look. This solves the issue in https://chromiumcodereview.appspot.com/11315004/ that we have discussed about. ...
1 year, 5 months ago #1
loislo
sounds good to me
1 year, 5 months ago #2
loislo
On 2012/10/31 15:19:50, loislo wrote: > sounds good to me ping
1 year, 5 months ago #3
erikcorry
LGTM https://chromiumcodereview.appspot.com/11340010/diff/1/src/heap.cc File src/heap.cc (right): https://chromiumcodereview.appspot.com/11340010/diff/1/src/heap.cc#newcode1601 src/heap.cc:1601: } external_string_table_visitor(visitor); Please add a blank line here ...
1 year, 5 months ago #4
erikcorry
1 year, 5 months ago #5
One more thing:  the comment in include/v8.h mentions that this may be slow,
which it probably isn't.  On the other hand it should probably mention that we
don't do a GC as part of this call, so you may get callbacks for resources that
are actually dead.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1280:2d3e6564b7b6