Bug 103076 – Web Inspector: Heap snapshot crashes on any page in MacOS Canary

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Alexei Filippov]

Open any page (cnn.com) in Chrome Canary (tried on 25.0.1332.0) on MacOS and take a heap snapshot.
The page crashes (crash id be1ab3fe31429c73).

Call stack:
0x020d33a1	 [Google Chrome Framework]	 - ../../../../../v8/include/v8.h:4234]	WebCore::DOMDataStore::current
0x0263c9a9	 [Google Chrome Framework]	 - ../bindings/v8/DOMDataStore.h:68]	WebCore::DOMDataStore::getNode
0x026a3212	 [Google Chrome Framework]	 - V8Document.h:76]	WebCore::DOMWindowV8Internal::documentAttrGetter
0x01c49114	 [Google Chrome Framework]	 - objects.cc:207]	v8::internal::JSObject::GetPropertyWithCallback
0x01c48ef5	 [Google Chrome Framework]	 - objects.cc:652]	v8::internal::Object::GetProperty
0x01c48bd7	 [Google Chrome Framework]	 - objects.cc:159]	v8::internal::Object::GetPropertyWithReceiver
0x01ca7c26	 [Google Chrome Framework]	 - ../../src/objects-inl.h:873]	v8::internal::V8HeapExplorer::TagGlobalObjects
0x01ca8c1c	 [Google Chrome Framework]	 - profile-generator.cc:3094]	v8::internal::HeapSnapshotGenerator::GenerateSnapshot
0x01b9630d	 [Google Chrome Framework]	 - heap-profiler.cc:135]	v8::internal::HeapProfiler::TakeSnapshotImpl
0x01b963f4	 [Google Chrome Framework]	 - heap-profiler.cc:153]	v8::internal::HeapProfiler::TakeSnapshot
0x01b2e96c	 [Google Chrome Framework]	 - api.cc:6413]	v8::HeapProfiler::TakeSnapshot
0x020e6276	 [Google Chrome Framework]	 - ScriptProfiler.cpp:155]	WebCore::ScriptProfiler::takeHeapSnapshot
0x0232cdea	 [Google Chrome Framework]	 - InspectorProfilerAgent.cpp:434]	WebCore::InspectorProfilerAgent::takeHeapSnapshot

[Yury Semikhatsky]

Downstream bug: https://code.google.com/p/chromium/issues/detail?id=162121

[Yury Semikhatsky]

This was likely broken in http://trac.webkit.org/changeset/135230

[Adam Barth]

It's probably not that exact patch since that one just moves code around, but it looks like the heap snapshot is breaking the assumption that we never grab the wrapper for a node unless we're inside a V8 context.

The correct fix is to do this work inside a V8 context.  It's not sensible to call functions like DOMWindowV8Internal::documentAttrGetter outside a V8 context.

[Adam Barth]

Actually, you're right.  That was the step that removed the inContext check.

[Adam Barth]

We can add the inContext check back, but it will slow down every DOM operation.

[Yury Semikhatsky]

We need to fix this in v8 profile generator and it's going to be a two-sided change so it may make sense to return the check until we have a proper fix.

[Adam Barth]

Created attachment 176283 [details]
Likely fix but no test

[Adam Barth]

I've attached a patch that should fix the problem.  Would you be willing to write a test that shows the issue?  I'm not familiar with how to write tests of the web inspector's heap snapshot feature.

[Yury Semikhatsky]

Hm, the issue is 100% repeatable on the canary build 
Google Chrome	25.0.1337.1 (Official Build 169970) canary
OS	Mac OS X 
WebKit	537.20 (@135920)
but I cannot reproduce it neither on tip-of-tree build nor on http://commondatastorage.googleapis.com/chromium-browser-snapshots/index.html?path=Mac/169960/
http://commondatastorage.googleapis.com/chromium-browser-snapshots/index.html?path=Mac/169996/
continuos builds. I'm attaching the page where the bug is reproducible.

[Yury Semikhatsky]

Created attachment 176609 [details]
Sample page (doc.html)

[Yury Semikhatsky]

Created attachment 176751 [details]
Patch

[Yury Semikhatsky]

(In reply to comment #11)
> Created an attachment (id=176751) [details]
> Patch

This patch depends on V8 change https://codereview.chromium.org/11415203/ and won't compile on the current Chromium source base.

[Adam Barth]

Thanks.  This looks like a good approach.

[Peter Beverloo (cr-android ews)]

Comment on attachment 176751 [details]
Patch

Attachment 176751 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/15027904

[WebKit Review Bot]

Comment on attachment 176751 [details]
Patch

Attachment 176751 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15029919

[Yury Semikhatsky]

Committed r137433: <http://trac.webkit.org/changeset/137433>