Reviewers: loislo, Michael Starzinger, Yang,
Description:
Do not read document and URL properties on global objects while taking heap
snapshot
This unsafe mechanism was replaced with a user provided callback in r13137
and
now we should remove old code.
Please review this at https://codereview.chromium.org/11519029/
SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/
Affected files:
M src/profile-generator.cc
M test/cctest/test-heap-profiler.cc
Index: src/profile-generator.cc
===================================================================
--- src/profile-generator.cc (revision 13202)
+++ src/profile-generator.cc (working copy)
@@ -2708,10 +2708,6 @@
Isolate* isolate = Isolate::Current();
GlobalObjectsEnumerator enumerator;
isolate->global_handles()->IterateAllRoots(&enumerator);
- Handle<String> document_string =
- isolate->factory()->NewStringFromAscii(CStrVector("document"));
- Handle<String> url_string =
- isolate->factory()->NewStringFromAscii(CStrVector("URL"));
const char** urls = NewArray<const char*>(enumerator.count());
for (int i = 0, l = enumerator.count(); i < l; ++i) {
if (global_object_name_resolver_) {
@@ -2720,25 +2716,7 @@
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));
- }
- }
}
}
Index: test/cctest/test-heap-profiler.cc
===================================================================
--- test/cctest/test-heap-profiler.cc (revision 13202)
+++ test/cctest/test-heap-profiler.cc (working copy)
@@ -1254,56 +1254,6 @@
}
-TEST(DocumentURL) {
- v8::HandleScope scope;
- LocalContext env;
-
- CompileRun("document = { URL:\"abcdefgh\" };");
-
- const v8::HeapSnapshot* snapshot =
- v8::HeapProfiler::TakeSnapshot(v8_str("document"));
- const v8::HeapGraphNode* global = GetGlobalObject(snapshot);
- CHECK_NE(NULL, global);
- CHECK_EQ("Object / abcdefgh",
- const_cast<i::HeapEntry*>(
- reinterpret_cast<const i::HeapEntry*>(global))->name());
-}
-
-
-TEST(DocumentWithException) {
- v8::HandleScope scope;
- LocalContext env;
-
- CompileRun(
- "this.__defineGetter__(\"document\", function() { throw new Error();
})");
- const v8::HeapSnapshot* snapshot =
- v8::HeapProfiler::TakeSnapshot(v8_str("document"));
- const v8::HeapGraphNode* global = GetGlobalObject(snapshot);
- CHECK_NE(NULL, global);
- CHECK_EQ("Object",
- const_cast<i::HeapEntry*>(
- reinterpret_cast<const i::HeapEntry*>(global))->name());
-}
-
-
-TEST(DocumentURLWithException) {
- v8::HandleScope scope;
- LocalContext env;
-
- CompileRun(
- "function URLWithException() {}\n"
- "URLWithException.prototype = { get URL() { throw new Error(); }
};\n"
- "document = { URL: new URLWithException() };");
- const v8::HeapSnapshot* snapshot =
- v8::HeapProfiler::TakeSnapshot(v8_str("document"));
- const v8::HeapGraphNode* global = GetGlobalObject(snapshot);
- CHECK_NE(NULL, global);
- CHECK_EQ("Object",
- const_cast<i::HeapEntry*>(
- reinterpret_cast<const i::HeapEntry*>(global))->name());
-}
-
-
TEST(NoHandleLeaks) {
v8::HandleScope scope;
LocalContext env;
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
