Heap snapshot: Update user roots definition.

src/heap-snapshot-generator.cc

Index: src/heap-snapshot-generator.cc
diff --git a/src/heap-snapshot-generator.cc b/src/heap-snapshot-generator.cc
index 217d1ca25ecfa9dae22b7270b973700c67a2a09f..a4c31121460f729ade5384c9b7f4f6e3044fcec1 100644
--- a/src/heap-snapshot-generator.cc
+++ b/src/heap-snapshot-generator.cc
@@ -961,7 +961,7 @@ void V8HeapExplorer::ExtractReferences(HeapObject* obj) {
 
   bool extract_indexed_refs = true;
   if (obj->IsJSGlobalProxy()) {
-    ExtractJSGlobalProxyReferences(JSGlobalProxy::cast(obj));
+    ExtractJSGlobalProxyReferences(entry, JSGlobalProxy::cast(obj));
   } else if (obj->IsJSObject()) {
     ExtractJSObjectReferences(entry, JSObject::cast(obj));
   } else if (obj->IsString()) {
@@ -994,19 +994,11 @@ void V8HeapExplorer::ExtractReferences(HeapObject* obj) {
 }
 
 
-void V8HeapExplorer::ExtractJSGlobalProxyReferences(JSGlobalProxy* proxy) {
-  // We need to reference JS global objects from snapshot's root.
-  // We use JSGlobalProxy because this is what embedder (e.g. browser)
-  // uses for the global object.
-  Object* object = proxy->map()->prototype();
-  bool is_debug_object = false;
-#ifdef ENABLE_DEBUGGER_SUPPORT
-  is_debug_object = object->IsGlobalObject() &&
-      Isolate::Current()->debug()->IsDebugGlobal(GlobalObject::cast(object));
-#endif
-  if (!is_debug_object) {
-    SetUserGlobalReference(object);
-  }
+void V8HeapExplorer::ExtractJSGlobalProxyReferences(
+    int entry, JSGlobalProxy* proxy) {
+  SetInternalReference(proxy, entry,
+                       "native_context", proxy->native_context(),

[yurys, 2013/06/27 16:26:00]

Can we have a test for this?

As far as I understand global object in Blink consists of 3 objects [1] which
look as a single entity from the application's point of view. One of them is
created to keep global object identity on navigation while the shadow one
actually keeps all user properties. Which one of them is pointed at by the
native_context link?


[1]
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

[alph, 2013/06/28 11:58:04]

Don't think checking that we've reported this the only propery of JSGlobalProxy
deserves a test.

+                       JSGlobalProxy::kNativeContextOffset);
 }
 
 
@@ -1760,6 +1752,30 @@ void V8HeapExplorer::SetGcSubrootReference(
           snapshot_->gc_subroot(tag)->index(),
           child_entry);
     }
+
+    // Add a shortcut to JS global object reference at snapshot root.
+    if (child_obj->IsNativeContext()) {
+      Context* context = Context::cast(child_obj);
+      JSObject* proxy = context->global_proxy();
+      if (proxy->IsJSGlobalProxy()) {
+        // We use JSGlobalProxy because that's what embedder (e.g. browser)
+        // uses for the global object.
+        // TODO(alph): consider extracting it as just proxy->global_object()
+        Object* object = proxy->map()->prototype();
+        if (object->IsJSGlobalObject()) {
+          JSGlobalObject* global = JSGlobalObject::cast(object);
+          CHECK(global == context->global_object());

[yurys, 2013/06/28 09:42:18]

Ok, I looked more closely at how we setup global object in bootstraper.cc, in
particular the inner global object [1] and I think we should use
context->global_object() instead.

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/v8/src/bootstrappe...

[alph, 2013/06/28 11:58:04]

Done.

+          bool is_debug_object = false;
+#ifdef ENABLE_DEBUGGER_SUPPORT
+          is_debug_object = Isolate::Current()->debug()->IsDebugGlobal(global);

[yurys, 2013/06/27 16:26:00]

Please use heap_->isolate() instead of Isolate::Current()

[alph, 2013/06/28 11:58:04]

Done.

+#endif
+          if (!is_debug_object && !user_roots_.Contains(global)) {
+            user_roots_.Insert(global);
+            SetUserGlobalReference(global);
+          }
+        }
+      }
+    }
   }
 }