Make sure to always set errors on failed requests

RESOLVED FIXED in mozilla18

Status

()

Product:
Core
Component:
DOM: IndexedDB
Type:
defect
Status:
RESOLVED FIXED
Opened:
7 years ago
Updated:
7 years ago

People

(Reporter: bent.mozilla, Assigned: bent.mozilla)

Tracking

Version:
Trunk
Target:
mozilla18
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Patch, v1
1.23 KB, patch
khuey
: review+
Details | Diff | Splinter Review
Posted patch Patch, v1 — — Splinter Review 
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.
Attachment #663011 - Flags: review?(khuey)
I'm not so sure about this one.  If our page is navigated why do we need to have working error reporting?
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.
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.
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.
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 on attachment 663011 [details] [diff] [review]
Patch, v1

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

Alright, I was convinced on IRC.
Attachment #663011 - Flags: review?(khuey) → review+
https://hg.mozilla.org/mozilla-central/rev/b899ee7782ea
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.