Bug 102458 - IndexedDB: convert delete-closed-database-object to upgradeneeded
Summary: IndexedDB: convert delete-closed-database-object to upgradeneeded
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: David Grogan
URL:
Keywords:
Depends on:
Blocks: 94972
  Show dependency treegraph
 
Reported: 2012-11-15 19:31 PST by David Grogan
Modified: 2012-11-19 17:59 PST (History)
5 users (show)

See Also:


Attachments
Patch (4.69 KB, patch)
2012-11-15 19:32 PST, David Grogan
no flags Details | Formatted Diff | Diff
Patch (4.97 KB, patch)
2012-11-16 15:20 PST, Joshua Bell
no flags Details | Formatted Diff | Diff
Patch (5.29 KB, patch)
2012-11-19 14:59 PST, David Grogan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Grogan 2012-11-15 19:31:30 PST
IndexedDB: convert delete-closed-database-object to upgradeneeded
Comment 1 David Grogan 2012-11-15 19:32:46 PST
Created attachment 174592 [details]
Patch
Comment 2 David Grogan 2012-11-15 19:33:56 PST
Josh, you looked at this one the other day. My instinct is to land it as disabled in TestExpectations.
Comment 3 David Grogan 2012-11-15 19:34:21 PST
Any other thoughts or suggestions?
Comment 4 WebKit Review Bot 2012-11-15 21:06:00 PST
Comment on attachment 174592 [details]
Patch

Attachment 174592 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14843865

New failing tests:
storage/indexeddb/delete-closed-database-object.html
Comment 5 Joshua Bell 2012-11-16 15:20:50 PST
Created attachment 174769 [details]
Patch
Comment 6 Joshua Bell 2012-11-16 15:23:49 PST
I did some poking around and found the proximal cause (but not the root cause) - open(name, version) apparently leaks(?) the connection, whereas open(name) does not. So open(name)/setVersion(version) works fine.

I uploaded a variant patch (as proof of concept) that does;

db = open(name); // implicitly calls open(name, 1) behind the scenes
db.close();
db = open(name); // open w/o version
gc();
db = open(name, version); // no blocked event fired

The patch I uploaded should be tweaked with a ChangeLog description and more evalAndLog() usage. We should also file a bug to track down the connection leak.
Comment 7 David Grogan 2012-11-19 14:59:04 PST
Created attachment 175048 [details]
Patch
Comment 8 David Grogan 2012-11-19 14:59:56 PST
Josh, I added some evalAndLog calls to your patch. Could you take a look?
Comment 9 Joshua Bell 2012-11-19 16:49:01 PST
Comment on attachment 175048 [details]
Patch

lgtm - not sure why it's flaky in content_shell
Comment 10 David Grogan 2012-11-19 16:50:33 PST
Tony, could you review this?
Comment 11 WebKit Review Bot 2012-11-19 17:59:26 PST
Comment on attachment 175048 [details]
Patch

Clearing flags on attachment: 175048

Committed r135222: <http://trac.webkit.org/changeset/135222>
Comment 12 WebKit Review Bot 2012-11-19 17:59:29 PST
All reviewed patches have been landed.  Closing bug.