When reporting exceptions, we sometimes perform a string conversion on an exception object. That string conversion can fail if an exception is thrown during conversion. In that case an empty handle is returned. We need to check for that. See comment 1 and 3 of http://crbug.com/24130.
Created attachment 41863 [details] Add null check.
Comment on attachment 41863 [details] Add null check. Test?
A test would indeed be great. I haven't been able to reproduce this crash yet, but there are a number of crashes reported where we get into this situation. I'll spent some more time tomorrow attempting to reproduce.
You can't just throw an exception in the toString?
Yeah, that is not the problem. The problem is getting into this case in the code first. I need to be in a situation where the TryCatch has no message *and* the ToString operations throws an exception. The first part is the hard part - most of the time a TryCatch should have a message. I need something like a stack overflow at an unfortunate time to hit this path.
So, I'm supposed to lean on you to write a test, but if you complain enough that it's hard to write the test, then we can go ahead with the patch anyway. In any case, you don't need me to lecture you about testing. :)
Created attachment 41942 [details] Add null check and test case.
Comment on attachment 41942 [details] Add null check and test case. Beautiful patch. Thanks!
Comment on attachment 41942 [details] Add null check and test case. Rejecting patch 41942 from commit-queue. Failed to run "['/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', '--reviewer', 'Adam Barth', '--force']" exit_code: 2 patching file WebCore/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file WebCore/bindings/v8/V8Utilities.cpp patching file LayoutTests/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file LayoutTests/fast/dom/error-to-string-stack-overflow-expected.txt Died at /Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply line 438, <> line 109.
Investigating the strange failure...
} elsif (isGit()) { system "git", "add", $path or die; } I'm just not sure why it died yet, but I will investigate.
My apologies. False rejection. I'm working on a fix to svn-apply right now. bug 30826
Comment on attachment 41942 [details] Add null check and test case. svn-apply fixed. Trying again.
Comment on attachment 41942 [details] Add null check and test case. Clearing flags on attachment: 41942 Committed r50160: <http://trac.webkit.org/changeset/50160>
All reviewed patches have been landed. Closing bug.