Bug 104800

Summary: Layout Test inspector-protocol/take-heap-snapshot.html crashes in the Debug mode
Product: WebKit Reporter: Kentaro Hara <haraken>
Component: Tools / TestsAssignee: Yury Semikhatsky <yurys>
Status: RESOLVED FIXED    
Severity: Normal CC: alph, jochen, loislo, marja, pfeldman, vsevik, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch jochen: review+

Description Kentaro Hara 2012-12-12 05:28:01 PST 
The following layout test is failing on the Debug mode in all platforms

inspector-protocol/take-heap-snapshot.html

Probable cause:

crash log for DumpRenderTree (pid 17694):
STDOUT: <empty>
STDERR: 
STDERR: 
STDERR: #
STDERR: # Fatal error in v8/src/heap-inl.h, line 189
STDERR: # CHECK(allocation_allowed_ && gc_state_ == NOT_IN_GC) failed
STDERR: #
STDERR: 

For now, I'll mark the test as CRASH. Please investigate.
Comment 1 Yury Semikhatsky 2012-12-12 05:38:54 PST 
*** Bug 104801 has been marked as a duplicate of this bug. ***
Comment 2 Yury Semikhatsky 2013-01-30 06:58:42 PST 
Related issue: https://code.google.com/p/v8/issues/detail?id=2515
Comment 3 Yury Semikhatsky 2013-01-31 00:18:46 PST 
Created attachment 185699 [details]
Patch
Comment 4 Yury Semikhatsky 2013-01-31 00:19:59 PST 
Marja, can you confirm that disabling Profiler.reportHeapSnapshotProgress events by default doesn't break your tools?
Comment 5 Ilya Tikhonovsky 2013-01-31 00:26:44 PST 
Comment on attachment 185699 [details]
Patch

lgtm
Comment 6 jochen 2013-01-31 00:29:12 PST 
Comment on attachment 185699 [details]
Patch

Since the parameter is optional, and the default behavior is the old behavior, there shouldn't be a problem.

thx for the heads up
Comment 7 Yury Semikhatsky 2013-01-31 00:34:15 PST 
(In reply to comment #6)
> (From update of attachment 185699 [details])
> Since the parameter is optional, and the default behavior is the old behavior, there shouldn't be a problem.
> 
> thx for the heads up

Actually the behavior changed compared to the old one: we used to send the events by default but after the change they will only be sent if the flag is true. That's why I'm asking. I didn't want to add doNotReportProgress optional flag as we normally prefer features opted out by default and here it would be an opposite meaning.
Comment 8 jochen 2013-01-31 00:39:00 PST 
(In reply to comment #7)
> (In reply to comment #6)
> > (From update of attachment 185699 [details] [details])
> > Since the parameter is optional, and the default behavior is the old behavior, there shouldn't be a problem.
> > 
> > thx for the heads up
> 
> Actually the behavior changed compared to the old one: we used to send the events by default but after the change they will only be sent if the flag is true. That's why I'm asking. I didn't want to add doNotReportProgress optional flag as we normally prefer features opted out by default and here it would be an opposite meaning.

Thanks for clarifying.

However, we don't depend on that feature in any way, so please go ahead!
Comment 9 Yury Semikhatsky 2013-01-31 00:47:28 PST 
Committed r141388: <http://trac.webkit.org/changeset/141388>