Bug 53687 - [v8] Bail out if to string conversion returned empty handle
Summary: [v8] Bail out if to string conversion returned empty handle
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: anton muhin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-03 09:33 PST by anton muhin
Modified: 2011-02-03 21:04 PST (History)
1 user (show)

See Also:

Attachments
Patch (2.76 KB, patch)
2011-02-03 09:46 PST, anton muhin 		no flags Details | Formatted Diff | Diff
Patch (2.85 KB, patch)
2011-02-03 10:24 PST, anton muhin 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) 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 anton muhin 2011-02-03 09:33:26 PST 
[v8] Bail out if to string conversion returned empty handle
Comment 1 anton muhin 2011-02-03 09:46:41 PST 
Created attachment 81071 [details]
Patch
Comment 2 Adam Barth 2011-02-03 10:16:49 PST 
Comment on attachment 81071 [details]
Patch

Generally speaking, we'd rather fix the root cause of the issue that bandaid over it like this.  Maybe add an ASSERT_NOT_REACHED to emphasize that it shouldn't happen?  Ideally we'd remove this code once we fixed the underlying bug...
Comment 3 anton muhin 2011-02-03 10:24:16 PST 
Created attachment 81076 [details]
Patch
Comment 4 anton muhin 2011-02-03 10:26:02 PST 
(In reply to comment #2)
> (From update of attachment 81071 [details])
> Generally speaking, we'd rather fix the root cause of the issue that bandaid over it like this.  Maybe add an ASSERT_NOT_REACHED to emphasize that it shouldn't happen?  Ideally we'd remove this code once we fixed the underlying bug...

Definitely.  I've got something which looks pretty much like a reduction for the core problem, but it's pure v8 API test.  I am starting to work on it immediately.  When issue in v8 is fixed, we'll run automated test with v8 patch applied and this patch removed.  If things go smooth, I'll revert this patch.

And ASSERT added
Comment 5 anton muhin 2011-02-03 10:28:40 PST 
Comment on attachment 81076 [details]
Patch

Thank you again, Adam.
Comment 6 WebKit Commit Bot 2011-02-03 21:04:28 PST 
Comment on attachment 81076 [details]
Patch

Clearing flags on attachment: 81076

Committed r77597: <http://trac.webkit.org/changeset/77597>
Comment 7 WebKit Commit Bot 2011-02-03 21:04:33 PST 
All reviewed patches have been landed.  Closing bug.