Bug 104800 - Layout Test inspector-protocol/take-heap-snapshot.html crashes in the Debug mode
Summary: Layout Test inspector-protocol/take-heap-snapshot.html crashes in the Debug mode
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yury Semikhatsky
URL:
Keywords:
: 104801 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-12-12 05:28 PST by Kentaro Hara
Modified: 2013-01-31 00:47 PST (History)
7 users (show)

See Also:

Attachments
Patch (7.03 KB, patch)
2013-01-31 00:18 PST, Yury Semikhatsky 		jochen: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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>