Introduce callback for resolving global object name while taking heap snapshot

Heap profiler currently gets "document" of global objects while taking snapshot (to later retrieve its "URL"). This is unsafe as there may be no current v8 context when the property is requested while corresponding property accessor may make some assumptions about the context stack during its invokation. Several crashes were reported due to this problem:
https://bugs.webkit.org/show_bug.cgi?id=103076
https://crbug.com/162121
https://crbug.com/132727

This patch adds a callback for resolving global object names and avoid the crashes.
Review URL: https://codereview.chromium.org/11415203

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@13137 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

include/v8-profiler.h

@@ -406,6 +406,20 @@ class V8EXPORT HeapProfiler {
    */
   static const SnapshotObjectId kUnknownObjectId = 0;
 
+  /**
+   * Callback interface for retrieving user friendly names of global objects.
+   */
+  class ObjectNameResolver {
+  public:
+    /**
+     * Returns name to be used in the heap snapshot for given node. Returned
+     * string must stay alive until snapshot collection is completed.
+     */
+    virtual const char* GetName(Handle<Object> object) = 0;
+  protected:
+    virtual ~ObjectNameResolver() {}
+  };
+
   /**
    * Takes a heap snapshot and returns it. Title may be an empty string.
    * See HeapSnapshot::Type for types description.
@@ -413,7 +427,8 @@ class V8EXPORT HeapProfiler {
   static const HeapSnapshot* TakeSnapshot(
       Handle<String> title,
       HeapSnapshot::Type type = HeapSnapshot::kFull,
-      ActivityControl* control = NULL);
+      ActivityControl* control = NULL,
+      ObjectNameResolver* global_object_name_resolver = NULL);
 
   /**
    * Starts tracking of heap objects population statistics. After calling

src/api.cc

@@ -6405,7 +6405,8 @@ SnapshotObjectId HeapProfiler::GetSnapshotObjectId(Handle<Value> value) {
 
 const HeapSnapshot* HeapProfiler::TakeSnapshot(Handle<String> title,
                                                HeapSnapshot::Type type,
-                                               ActivityControl* control) {
+                                               ActivityControl* control,
+                                               ObjectNameResolver* resolver) {
   i::Isolate* isolate = i::Isolate::Current();
   IsDeadCheck(isolate, "v8::HeapProfiler::TakeSnapshot");
   i::HeapSnapshot::Type internal_type = i::HeapSnapshot::kFull;
@@ -6418,7 +6419,7 @@ const HeapSnapshot* HeapProfiler::TakeSnapshot(Handle<String> title,
   }
   return reinterpret_cast<const HeapSnapshot*>(
       i::HeapProfiler::TakeSnapshot(
-          *Utils::OpenHandle(*title), internal_type, control));
+          *Utils::OpenHandle(*title), internal_type, control, resolver));
 }
 
 

src/heap-profiler.cc

@@ -65,23 +65,29 @@ void HeapProfiler::TearDown() {
 }
 
 
-HeapSnapshot* HeapProfiler::TakeSnapshot(const char* name,
-                                         int type,
-                                         v8::ActivityControl* control) {
+HeapSnapshot* HeapProfiler::TakeSnapshot(
+    const char* name,
+    int type,
+    v8::ActivityControl* control,
+    v8::HeapProfiler::ObjectNameResolver* resolver) {
   ASSERT(Isolate::Current()->heap_profiler() != NULL);
   return Isolate::Current()->heap_profiler()->TakeSnapshotImpl(name,
                                                                type,
-                                                               control);
+                                                               control,
+                                                               resolver);
 }
 
 
-HeapSnapshot* HeapProfiler::TakeSnapshot(String* name,
-                                         int type,
-                                         v8::ActivityControl* control) {
+HeapSnapshot* HeapProfiler::TakeSnapshot(
+    String* name,
+    int type,
+    v8::ActivityControl* control,
+    v8::HeapProfiler::ObjectNameResolver* resolver) {
   ASSERT(Isolate::Current()->heap_profiler() != NULL);
   return Isolate::Current()->heap_profiler()->TakeSnapshotImpl(name,
                                                                type,
-                                                               control);
+                                                               control,
+                                                               resolver);
 }
 
 
@@ -122,16 +128,18 @@ v8::RetainedObjectInfo* HeapProfiler::ExecuteWrapperClassCallback(
 }
 
 
-HeapSnapshot* HeapProfiler::TakeSnapshotImpl(const char* name,
-                                             int type,
-                                             v8::ActivityControl* control) {
+HeapSnapshot* HeapProfiler::TakeSnapshotImpl(
+    const char* name,
+    int type,
+    v8::ActivityControl* control,
+    v8::HeapProfiler::ObjectNameResolver* resolver) {
   HeapSnapshot::Type s_type = static_cast<HeapSnapshot::Type>(type);
   HeapSnapshot* result =
       snapshots_->NewSnapshot(s_type, name, next_snapshot_uid_++);
   bool generation_completed = true;
   switch (s_type) {
     case HeapSnapshot::kFull: {
-      HeapSnapshotGenerator generator(result, control);
+      HeapSnapshotGenerator generator(result, control, resolver);
       generation_completed = generator.GenerateSnapshot();
       break;
     }
@@ -147,10 +155,13 @@ HeapSnapshot* HeapProfiler::TakeSnapshotImpl(const char* name,
 }
 
 
-HeapSnapshot* HeapProfiler::TakeSnapshotImpl(String* name,
-                                             int type,
-                                             v8::ActivityControl* control) {
-  return TakeSnapshotImpl(snapshots_->names()->GetName(name), type, control);
+HeapSnapshot* HeapProfiler::TakeSnapshotImpl(
+    String* name,
+    int type,
+    v8::ActivityControl* control,
+    v8::HeapProfiler::ObjectNameResolver* resolver) {
+  return TakeSnapshotImpl(snapshots_->names()->GetName(name), type, control,
+                          resolver);
 }
 
 void HeapProfiler::StartHeapObjectsTrackingImpl() {

src/heap-profiler.h

@@ -51,12 +51,16 @@ class HeapProfiler {
 
   static size_t GetMemorySizeUsedByProfiler();
 
-  static HeapSnapshot* TakeSnapshot(const char* name,
-                                    int type,
-                                    v8::ActivityControl* control);
-  static HeapSnapshot* TakeSnapshot(String* name,
-                                    int type,
-                                    v8::ActivityControl* control);
+  static HeapSnapshot* TakeSnapshot(
+      const char* name,
+      int type,
+      v8::ActivityControl* control,
+      v8::HeapProfiler::ObjectNameResolver* resolver);
+  static HeapSnapshot* TakeSnapshot(
+      String* name,
+      int type,
+      v8::ActivityControl* control,
+      v8::HeapProfiler::ObjectNameResolver* resolver);
 
   static void StartHeapObjectsTracking();
   static void StopHeapObjectsTracking();
@@ -81,12 +85,16 @@ class HeapProfiler {
  private:
   HeapProfiler();
   ~HeapProfiler();
-  HeapSnapshot* TakeSnapshotImpl(const char* name,
-                                 int type,
-                                 v8::ActivityControl* control);
-  HeapSnapshot* TakeSnapshotImpl(String* name,
-                                 int type,
-                                 v8::ActivityControl* control);
+  HeapSnapshot* TakeSnapshotImpl(
+      const char* name,
+      int type,
+      v8::ActivityControl* control,
+      v8::HeapProfiler::ObjectNameResolver* resolver);
+  HeapSnapshot* TakeSnapshotImpl(
+      String* name,
+      int type,
+      v8::ActivityControl* control,
+      v8::HeapProfiler::ObjectNameResolver* resolver);
   void ResetSnapshots();
 
   void StartHeapObjectsTrackingImpl();

src/profile-generator.cc

@@ -1644,12 +1644,14 @@ HeapObject* const V8HeapExplorer::kLastGcSubrootObject =
 
 V8HeapExplorer::V8HeapExplorer(
     HeapSnapshot* snapshot,
-    SnapshottingProgressReportingInterface* progress)
+    SnapshottingProgressReportingInterface* progress,
+    v8::HeapProfiler::ObjectNameResolver* resolver)
     : heap_(Isolate::Current()->heap()),
       snapshot_(snapshot),
       collection_(snapshot_->collection()),
       progress_(progress),
-      filler_(NULL) {
+      filler_(NULL),
+      global_object_name_resolver_(resolver) {
 }
 
 
@@ -2712,21 +2714,30 @@ void V8HeapExplorer::TagGlobalObjects() {
       isolate->factory()->NewStringFromAscii(CStrVector("URL"));
   const char** urls = NewArray<const char*>(enumerator.count());
   for (int i = 0, l = enumerator.count(); i < l; ++i) {
-    urls[i] = NULL;
-    HandleScope scope;
-    Handle<JSGlobalObject> global_obj = enumerator.at(i);
-    Object* obj_document;
-    if (global_obj->GetProperty(*document_string)->ToObject(&obj_document) &&
-        obj_document->IsJSObject()) {
-      // FixMe: Workaround: SharedWorker's current Isolate has NULL context.
-      // As result GetProperty(*url_string) will crash.
-      if (!Isolate::Current()->context() && obj_document->IsJSGlobalProxy())
-        continue;
-      JSObject* document = JSObject::cast(obj_document);
-      Object* obj_url;
-      if (document->GetProperty(*url_string)->ToObject(&obj_url) &&
-          obj_url->IsString()) {
-        urls[i] = collection_->names()->GetName(String::cast(obj_url));
+    if (global_object_name_resolver_) {
+      HandleScope scope;
+      Handle<JSGlobalObject> global_obj = enumerator.at(i);
+      urls[i] = global_object_name_resolver_->GetName(
+          Utils::ToLocal(Handle<JSObject>::cast(global_obj)));
+    } else {
+      // TODO(yurys): This branch is going to be removed once Chromium migrates
+      // to the new name resolver.
+      urls[i] = NULL;
+      HandleScope scope;
+      Handle<JSGlobalObject> global_obj = enumerator.at(i);
+      Object* obj_document;
+      if (global_obj->GetProperty(*document_string)->ToObject(&obj_document) &&
+          obj_document->IsJSObject()) {
+        // FixMe: Workaround: SharedWorker's current Isolate has NULL context.
+        // As result GetProperty(*url_string) will crash.
+        if (!Isolate::Current()->context() && obj_document->IsJSGlobalProxy())
+          continue;
+        JSObject* document = JSObject::cast(obj_document);
+        Object* obj_url;
+        if (document->GetProperty(*url_string)->ToObject(&obj_url) &&
+            obj_url->IsString()) {
+          urls[i] = collection_->names()->GetName(String::cast(obj_url));
+        }
       }
     }
   }
@@ -3081,11 +3092,13 @@ class SnapshotFiller : public SnapshotFillerInterface {
 };
 
 
-HeapSnapshotGenerator::HeapSnapshotGenerator(HeapSnapshot* snapshot,
-                                             v8::ActivityControl* control)
+HeapSnapshotGenerator::HeapSnapshotGenerator(
+    HeapSnapshot* snapshot,
+    v8::ActivityControl* control,
+    v8::HeapProfiler::ObjectNameResolver* resolver)
     : snapshot_(snapshot),
       control_(control),
-      v8_heap_explorer_(snapshot_, this),
+      v8_heap_explorer_(snapshot_, this, resolver),
       dom_explorer_(snapshot_, this) {
 }
 

src/profile-generator.h

@@ -851,7 +851,8 @@ class SnapshottingProgressReportingInterface {
 class V8HeapExplorer : public HeapEntriesAllocator {
  public:
   V8HeapExplorer(HeapSnapshot* snapshot,
-                 SnapshottingProgressReportingInterface* progress);
+                 SnapshottingProgressReportingInterface* progress,
+                 v8::HeapProfiler::ObjectNameResolver* resolver);
   virtual ~V8HeapExplorer();
   virtual HeapEntry* AllocateEntry(HeapThing ptr);
   void AddRootEntries(SnapshotFillerInterface* filler);
@@ -945,6 +946,7 @@ class V8HeapExplorer : public HeapEntriesAllocator {
   SnapshotFillerInterface* filler_;
   HeapObjectsSet objects_tags_;
   HeapObjectsSet strong_gc_subroot_names_;
+  v8::HeapProfiler::ObjectNameResolver* global_object_name_resolver_;
 
   static HeapObject* const kGcRootsObject;
   static HeapObject* const kFirstGcSubrootObject;
@@ -1021,7 +1023,8 @@ class NativeObjectsExplorer {
 class HeapSnapshotGenerator : public SnapshottingProgressReportingInterface {
  public:
   HeapSnapshotGenerator(HeapSnapshot* snapshot,
-                        v8::ActivityControl* control);
+                        v8::ActivityControl* control,
+                        v8::HeapProfiler::ObjectNameResolver* resolver);
   bool GenerateSnapshot();
 
  private:

test/cctest/test-heap-profiler.cc

@@ -1227,6 +1227,33 @@ TEST(DeleteHeapSnapshot) {
 }
 
 
+class NameResolver : public v8::HeapProfiler::ObjectNameResolver {
+ public:
+  virtual const char* GetName(v8::Handle<v8::Object> object) {
+    return "Global object name";
+  }
+};
+
+TEST(GlobalObjectName) {
+  v8::HandleScope scope;
+  LocalContext env;
+
+  CompileRun("document = { URL:\"abcdefgh\" };");
+
+  NameResolver name_resolver;
+  const v8::HeapSnapshot* snapshot =
+      v8::HeapProfiler::TakeSnapshot(v8_str("document"),
+      v8::HeapSnapshot::kFull,
+      NULL,
+      &name_resolver);
+  const v8::HeapGraphNode* global = GetGlobalObject(snapshot);
+  CHECK_NE(NULL, global);
+  CHECK_EQ("Object / Global object name" ,
+           const_cast<i::HeapEntry*>(
+               reinterpret_cast<const i::HeapEntry*>(global))->name());
+}
+
+
 TEST(DocumentURL) {
   v8::HandleScope scope;
   LocalContext env;