some comments
https://codereview.chromium.org/16578008/diff/11001/src/api.cc
File src/api.cc (right):
https://codereview.chromium.org/16578008/diff/11001/src/api.cc#newcode5150
src/api.cc:5150: bool v8::V8::SetFunctionEntryHook(FunctionEntryHook
entry_hook) {
API calls should take an Isolate parameter if you need it. To deprecate
the other API, you might need to have both the Isolate and non-isolate
version for a while, the non-isolate version calling the isolate version
with "i::Isolate::Current();"
https://codereview.chromium.org/16578008/diff/11001/src/api.cc#newcode5155
src/api.cc:5155: if (entry_hook != NULL &&
isolate->HasFunctionEntryHook())
Seems like this should be a ASSERT(entry_hook != NULL); and a if
(isolate->HasFunctionEntryHook()) {
https://codereview.chromium.org/16578008/diff/11001/src/api.cc#newcode5158
src/api.cc:5158: isolate->SetFunctionEntryHook(entry_hook);
It would be great to introduce some checks to make sure that this
routine is called at a time that is acceptable. It must be invoked on an
Isolate before the first code object is generated, otherwise you will
get strange effects. Is there any way to police that? Or perhaps
introduce a new isolate constructor that takes the hook to make sure
that it's always set from the very beginning? Something like that might
make this functionality harder to abuse.
https://codereview.chromium.org/16578008/diff/11001/src/frames.cc
File src/frames.cc (right):
https://codereview.chromium.org/16578008/diff/11001/src/frames.cc#newcode46
src/frames.cc:46: ReturnAddressLocationResolver
return_address_location_resolver = NULL;
Shouldn't this also be isolate based now?
https://codereview.chromium.org/16578008/diff/11001/src/isolate.cc
File src/isolate.cc (right):
https://codereview.chromium.org/16578008/diff/11001/src/isolate.cc#newcode2171
src/isolate.cc:2171:
Stray whitespace change?
https://codereview.chromium.org/16578008/diff/11001/src/isolate.cc#newcode2458
src/isolate.cc:2458: if (function_entry_hook == NULL &&
function_entry_hook_ != NULL) {
Might it be a little clearer if you can never pass a non-null value to
function_entry_hook? Instead have an explicit DisableFunctionEntryHook
that resets function_entry_hook_ to NullFunctionEntryHook?
https://codereview.chromium.org/16578008/diff/11001/src/snapshot-common.cc
File src/snapshot-common.cc (right):
https://codereview.chromium.org/16578008/diff/11001/src/snapshot-common.cc#newcode84
src/snapshot-common.cc:84: bool Snapshot::Initialize(const char*
snapshot_file) {
If you need the isolate, you should pass it in as a parameter.
https://codereview.chromium.org/16578008/diff/11001/src/snapshot-common.cc#newcode88
src/snapshot-common.cc:88: if (isolate &&
isolate->HasFunctionEntryHook())
nit: isolate != NULL
https://codereview.chromium.org/16578008/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
