https://chromiumcodereview.appspot.com/9316092/diff/3005/src/profile-generator.cc File src/profile-generator.cc (right):
https://chromiumcodereview.appspot.com/9316092/diff/3005/src/profile-generator.cc#newcode2801 src/profile-generator.cc:2801: "native", Does this look descriptive enough? If implicit refs are only for event handlers, perhaps we can call this more specific? https://chromiumcodereview.appspot.com/9316092/diff/3005/src/profile-generator.cc#newcode2825 src/profile-generator.cc:2825: if (EstimateObjectsCount() <= 0) { Let's rewrite this as follows to avoid cleanup code duplication: if (EstimateObjectsCount() > 0) { for(...) { } SetRootNative... } filler_ = NULL; return true; https://chromiumcodereview.appspot.com/9316092/diff/3005/test/cctest/test-heap-profiler.cc File test/cctest/test-heap-profiler.cc (right): https://chromiumcodereview.appspot.com/9316092/diff/3005/test/cctest/test-heap-profiler.cc#newcode813 test/cctest/test-heap-profiler.cc:813: for (int i = 0; i < 4; i++) nit: brackets https://chromiumcodereview.appspot.com/9316092/diff/3005/test/cctest/test-heap-profiler.cc#newcode832 test/cctest/test-heap-profiler.cc:832: v8::Persistent<v8::Object>::Cast(objects_[1]), &objects_[2], 2); Have you forgot to add 1->3 reference, or what does this comment mean? https://chromiumcodereview.appspot.com/9316092/diff/3005/test/cctest/test-heap-profiler.cc#newcode835 test/cctest/test-heap-profiler.cc:835: v8::Persistent<v8::Value> objects_[4]; I think, using an enum for test objects and their count will be more readable and robust. https://chromiumcodereview.appspot.com/9316092/diff/3005/test/cctest/test-heap-profiler.cc#newcode854 test/cctest/test-heap-profiler.cc:854: const v8::HeapGraphNode* o0 = GetProperty( This name looks weird. In other tests names like "obj0" are used. https://chromiumcodereview.appspot.com/9316092/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
