792834 - Make sure to always set errors on failed requests

Status: RESOLVED FIXED

(Core :: Storage: IndexedDB, defect)

Description

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Attachment: Patch, v1

Found while looking at bug 792547. We have an early return where we associate an error code with a failed request and it causes later error reporting code to fail. Simple patch.

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I'm not so sure about this one.  If our page is navigated why do we need to have working error reporting?

Comment 2

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Well, currently all failed requests end up firing Error events, here:

http://mxr.mozilla.org/mozilla-central/source/dom/indexedDB/AsyncConnectionHelper.cpp#464

If left unhandled that error event bubbles to the window and we eventually end up here:

http://mxr.mozilla.org/mozilla-central/source/dom/indexedDB/IndexedDatabaseManager.cpp#356

At this point the request has no idea about its error code, so we don't report anything.

Comment 3

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Sure, I understand that.  But if CheckInnerWindowCorrectness fails, the window is already "gone".  The page has been navigated to something new.  Running script on the page after CheckInnerWindowCorrectness fails seems like a bad idea.

Comment 4

[Ben Turner (not reading bugmail, use the needinfo flag!)]

It's not "gone". It's in the bfcache. I think we kick pages out of the bfcache in certain cases when DOM events are fired, but there's nothing special going on here. The event listener code should handle this.

Comment 5

[Olli Pettay [:smaug][[e-mail]] (vacation-ish -> Aug 1)]

Would we run something in the "gone" page or just propagate the error so that
there would be something in the error console?
(I'm not familiar with IDB code)

Note, event listeners aren't called if innerwindow check fails.

Comment 6

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 663011 [details] [diff] [review]
Patch, v1

Review of attachment 663011 [details] [diff] [review]:
-----------------------------------------------------------------

Alright, I was convinced on IRC.

Comment 7

[Ben Turner (not reading bugmail, use the needinfo flag!)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b899ee7782ea

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b899ee7782ea