Bug 99486 - IndexedDB: Closing connection in upgradeneeded should result in error event
Summary: IndexedDB: Closing connection in upgradeneeded should result in error event
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Joshua Bell
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-16 11:43 PDT by Joshua Bell
Modified: 2012-10-17 16:51 PDT (History)
4 users (show)

See Also:


Attachments
Patch (7.13 KB, patch)
2012-10-16 11:47 PDT, Joshua Bell
no flags Details | Formatted Diff | Diff
Patch (7.14 KB, patch)
2012-10-17 15:43 PDT, Joshua Bell
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joshua Bell 2012-10-16 11:43:44 PDT
IndexedDB: Closing connection in upgradeneeded should result in error event
Comment 1 Joshua Bell 2012-10-16 11:47:27 PDT
Created attachment 168991 [details]
Patch
Comment 2 Joshua Bell 2012-10-16 11:49:16 PDT
This is a weird one. I think the behavior is more correct with this patch (and the updated test looks more correct), but I'm still feeling iffy about whether logic like this should go in enqueue vs. dispatch.
Comment 3 David Grogan 2012-10-16 15:52:11 PDT
Comment on attachment 168991 [details]
Patch

LGTM

In the chromium bug you mentioned that this might just be postponing the flakiness but I can't see how failures would occur after this patch.
Comment 4 Joshua Bell 2012-10-16 16:26:26 PDT
(In reply to comment #3)
> In the chromium bug you mentioned that this might just be postponing the flakiness but I can't see how failures would occur after this patch.

Yeah, I agree - I don't recall what edge cases I might have been thinking of.

As discussed offline, a more "elegant" approach than this patch would be to have the "complete" event dispatch send a message to the browser to unblock the success/error event, but this patch should have the equivalent from user script.
Comment 5 Joshua Bell 2012-10-16 17:14:15 PDT
Comment on attachment 168991 [details]
Patch

Actually, seeing some tests fail locally that may do an open and never see a success or an error.
Comment 6 Joshua Bell 2012-10-17 15:43:22 PDT
Created attachment 169279 [details]
Patch
Comment 7 Joshua Bell 2012-10-17 15:44:55 PDT
(In reply to comment #5)
> Actually, seeing some tests fail locally that may do an open and never see a success or an error.

Whoops, that was a false alarm - I was running NRWT with a manually set and very low timeout.

tony@ - r?
Comment 8 WebKit Review Bot 2012-10-17 16:51:08 PDT
Comment on attachment 169279 [details]
Patch

Clearing flags on attachment: 169279

Committed r131668: <http://trac.webkit.org/changeset/131668>
Comment 9 WebKit Review Bot 2012-10-17 16:51:12 PDT
All reviewed patches have been landed.  Closing bug.