Bug 95864

Summary: IndexedDB: Integer version lost after first open/close/open cycle
Product: WebKit Reporter: Joshua Bell <jsbell>
Component: New BugsAssignee: Joshua Bell <jsbell>
Status: RESOLVED FIXED    
Severity: Normal CC: alecflett, dgrogan, tony, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Repro layout test
none
Patch none

Description Joshua Bell 2012-09-05 10:04:59 PDT
IndexedDB: Integer version lost after first open/close/open cycle
Comment 1 Joshua Bell 2012-09-05 10:05:33 PDT
Created attachment 162274 [details]
Repro layout test
Comment 2 Joshua Bell 2012-09-05 10:08:31 PDT
Comment on attachment 162274 [details]
Repro layout test

As shown in the above test, the database's integer version is lost after an open/close/open cycle.

The close is required to let the backing store be released from memory.

The test must NOT be preceded by a delete, as that would create the on-disk backing store and bias the test result. Note that http://webkit.org/b/92166 will also bias this test if the test is run in a batch.
Comment 3 Joshua Bell 2012-09-05 10:23:55 PDT
Created attachment 162276 [details]
Patch
Comment 4 Joshua Bell 2012-09-05 10:24:21 PDT
dgrogan@ - please take a look
Comment 5 David Grogan 2012-09-05 12:22:36 PDT
Comment on attachment 162276 [details]
Patch

LGTM
Comment 6 Joshua Bell 2012-09-05 12:25:06 PDT
Comment on attachment 162276 [details]
Patch

another one:

tony@ - r? cq?
Comment 7 Tony Chang 2012-09-05 12:28:13 PDT
Comment on attachment 162276 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=162276&action=review

> Source/WebCore/Modules/indexeddb/IDBLevelDBBackingStore.cpp:141
> +    const int64_t latestSchemaVersion = 1;

We only need this value here? How will you remember to update this in the future?
Comment 8 Joshua Bell 2012-09-05 13:12:15 PDT
Comment on attachment 162276 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=162276&action=review

>> Source/WebCore/Modules/indexeddb/IDBLevelDBBackingStore.cpp:141
>> +    const int64_t latestSchemaVersion = 1;
> 
> We only need this value here? How will you remember to update this in the future?

It's only needed/used in this method. The method is responsible to doing the migration from all previous versions to the latest version. It currently handles only the 0 -> 1 case, but if we rev the schema again (i.e. to 2) we'll need to handle the 0 -> 2 and 1 -> 2 cases.
Comment 9 WebKit Review Bot 2012-09-05 18:07:35 PDT
Comment on attachment 162276 [details]
Patch

Clearing flags on attachment: 162276

Committed r127669: <http://trac.webkit.org/changeset/127669>
Comment 10 WebKit Review Bot 2012-09-05 18:07:38 PDT
All reviewed patches have been landed.  Closing bug.