Heap profiler should report implicit references

test/cctest/test-heap-profiler.cc

Index: test/cctest/test-heap-profiler.cc
===================================================================
--- test/cctest/test-heap-profiler.cc	(revision 10596)
+++ test/cctest/test-heap-profiler.cc	(working copy)
@@ -805,6 +805,73 @@
 }
 
 
+class GraphWithImplicitRefs {
+ public:
+  explicit GraphWithImplicitRefs(LocalContext* env) {
+    CHECK_EQ(NULL, instance_);
+    instance_ = this;
+    for (int i = 0; i < 4; i++)

[mnaganov (inactive), 2012/02/03 13:57:19]

nit: brackets

[yurys, 2012/02/03 14:09:26]

Done. Are they required by V8 coding style?

[mnaganov (inactive), 2012/02/03 14:23:41]

Google C++ coding style says "conditional or loop statements with complex
conditions or statements may be more readable with curly braces". In V8, curly
braces are in general can be omitted only for trivial early returns from
functions.

+      objects_[i] = v8::Persistent<v8::Object>::New(v8::Object::New());
+    (*env)->Global()->Set(v8_str("root_object"), objects_[0]);
+  }
+  ~GraphWithImplicitRefs() {
+    instance_ = NULL;
+  }
+
+  static void gcPrologue() {
+    instance_->AddImplicitReferences();
+  }
+
+ private:
+  void AddImplicitReferences() {
+    // 0 -> 1
+    v8::V8::AddImplicitReferences(
+        v8::Persistent<v8::Object>::Cast(objects_[0]), &objects_[1], 1);
+    // 1 -> 2, 1 -> 3
+    v8::V8::AddImplicitReferences(
+        v8::Persistent<v8::Object>::Cast(objects_[1]), &objects_[2], 2);

[mnaganov (inactive), 2012/02/03 13:57:19]

Have you forgot to add 1->3 reference, or what does this comment mean?

[yurys, 2012/02/03 14:09:26]

No, the comment is perfectly fine, although Ilya asked the same question. Note
that in the second call we pass array of length 2.

+  }
+
+  v8::Persistent<v8::Value> objects_[4];

[mnaganov (inactive), 2012/02/03 13:57:19]

I think, using an enum for test objects and their count will be more readable
and robust.

[yurys, 2012/02/03 14:09:26]

Well, I started with storing each object in its own field but it turned out that
the only thing we need of them is their ordinal number. What kind of enum
constants do you suggest? I'd leave this part as is.

+  static GraphWithImplicitRefs* instance_;
+};
+
+GraphWithImplicitRefs* GraphWithImplicitRefs::instance_ = NULL;
+
+
+TEST(HeapSnapshotImplicitReferences) {
+  v8::HandleScope scope;
+  LocalContext env;
+
+  GraphWithImplicitRefs graph(&env);
+  v8::V8::SetGlobalGCPrologueCallback(&GraphWithImplicitRefs::gcPrologue);
+
+  const v8::HeapSnapshot* snapshot =
+      v8::HeapProfiler::TakeSnapshot(v8_str("implicit_refs"));
+
+  const v8::HeapGraphNode* global_object = GetGlobalObject(snapshot);
+  // Use kShortcut type to skip intermediate JSGlobalPropertyCell
+  const v8::HeapGraphNode* o0 = GetProperty(

[mnaganov (inactive), 2012/02/03 13:57:19]

This name looks weird. In other tests names like "obj0" are used.

[yurys, 2012/02/03 14:09:26]

Done.

+      global_object, v8::HeapGraphEdge::kShortcut, "root_object");
+  CHECK(o0);
+  CHECK_EQ(v8::HeapGraphNode::kObject, o0->GetType());
+  const v8::HeapGraphNode* o1 = GetProperty(
+      o0, v8::HeapGraphEdge::kInternal, "native");
+  CHECK(o1);
+  int implicit_targets_count = 0;
+  for (int i = 0, count = o1->GetChildrenCount(); i < count; ++i) {
+    const v8::HeapGraphEdge* prop = o1->GetChild(i);
+    v8::String::AsciiValue prop_name(prop->GetName());
+    if (prop->GetType() == v8::HeapGraphEdge::kInternal &&
+        strcmp("native", *prop_name) == 0) {
+      ++implicit_targets_count;
+    }
+  }
+  CHECK_EQ(2, implicit_targets_count);
+  v8::V8::SetGlobalGCPrologueCallback(NULL);
+}
+
+
 TEST(DeleteAllHeapSnapshots) {
   v8::HandleScope scope;
   LocalContext env;