Some more comments below. Otherwise LGTM.

-Ivan


http://codereview.chromium.org/113121/diff/1017/29
File src/api.cc (right):

http://codereview.chromium.org/113121/diff/1017/29#newcode212
Line 212: return ApiCheck(v8::V8::Initialize(), location, "Error
initializing V8");
Thinking about it further it might be helpful to call IsDeadCheck here
to get a more meaningful error message. In a way recreating what
ImplementationUtilities::Undefined() had below.

http://codereview.chromium.org/113121/diff/1017/29#newcode3059
Line 3059: i::StatsTable::SetCounterFunction(callback);
Should we reintroduce the IsDeadCheck here?

http://codereview.chromium.org/113121/diff/1017/29#newcode3063
Line 3063: i::StatsTable::SetCreateHistogramFunction(callback);
ditto

http://codereview.chromium.org/113121/diff/1017/29#newcode3067
Line 3067: i::StatsTable::SetAddHistogramSampleFunction(callback);
ditto

http://codereview.chromium.org/113121/diff/1017/31
File src/v8.h (right):

http://codereview.chromium.org/113121/diff/1017/31#newcode99
Line 99: static bool has_been_disposed_;
Can you add comments about the exact meaning of these four flags. When
adding them we had plenty of discussion what each of them meant and it
might be good to spell that out here. The lack of documentation was
contributing to the time it took to implement this change. The other was
the distributed nature of the data which you are addressing here by
consolidating in one file.

http://codereview.chromium.org/113121

--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---

Reply via email to