Some indexeddb tests come to crash since r185091. > Regressions: Unexpected crashes (5) > storage/indexeddb/error-causes-abort-by-default.html [ Crash ] > storage/indexeddb/exception-in-event-aborts.html [ Crash ] > storage/indexeddb/mozilla/add-twice-failure.html [ Crash ] > storage/indexeddb/request-event-propagation.html [ Crash ] > storage/indexeddb/transaction-event-propagation.html [ Crash ]
Created attachment 254058 [details] Patch
Created attachment 254059 [details] Patch
Comment on attachment 254059 [details] Patch rs=me
Comment on attachment 254059 [details] Patch Clearing flags on attachment: 254059 Committed r185107: <http://trac.webkit.org/changeset/185107>
All reviewed patches have been landed. Closing bug.
Comment on attachment 254059 [details] Patch This doesn’t look right. It just looks like a storage leak to me. Could you explain what code calls deref on these errors?
I am almost certain this patch is wrong. It’s possible adding a call to WTF::move() around the create functions will fix things; the trick is to change a temporary Ref into a RefPtr. I don’t really understand why, but I am quite sure that by calling leakRef we are actually leaking the objects, so I know that can’t be right. I don’t fully understand why the original crash was happening, though. Should have worked fine.
Reopening to attach new patch.
Created attachment 254244 [details] Patch
(In reply to comment #6) > Comment on attachment 254059 [details] > Patch > > This doesn’t look right. It just looks like a storage leak to me. Could you > explain what code calls deref on these errors? I guessed the deref was called by AsyncRequest::completeRequest() though, now I think I can't be 100% sure about it. http://trac.webkit.org/browser/trunk/Source/WebKit2/Shared/AsyncRequest.h#L101 > I am almost certain this patch is wrong. It’s possible adding a call to > WTF::move() around the create functions will fix things; the trick is to > change a temporary Ref into a RefPtr. I don’t really understand why, but I > am quite sure that by calling leakRef we are actually leaking the objects, > so I know that can’t be right. I don’t fully understand why the original > crash was happening, though. Should have worked fine. I agree that my previous fix can cause a storage leak because AsyncRequest::completeRequest() may not handle the pointer of IDBDatabaseError instance. However I would like to fix this problem with correct solution. So I upload a revert patch only for this issue. When I have correct solution for this issue, I will upload it again.
Comment on attachment 254244 [details] Patch Clearing flags on attachment: 254244 Committed r185233: <http://trac.webkit.org/changeset/185233>