Bug 102434 - IndexedDB: setVersion batch 8
Summary: IndexedDB: setVersion batch 8
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:
 
Reported: 2012-11-15 14:32 PST by David Grogan
Modified: 2012-11-15 17:14 PST (History)
4 users (show)

See Also:


Attachments
Patch (32.30 KB, patch)
2012-11-15 14:40 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 14:32:05 PST
IndexedDB: setVersion batch 8
Comment 1 David Grogan 2012-11-15 14:40:46 PST
Created attachment 174519 [details]
Patch
Comment 2 David Grogan 2012-11-15 14:43:13 PST
Josh, could you take a look?

After this patch there are 19 layout more tests that have setVersion warnings in their expected.txt.  There are a few more tests that have been disabled since before that warning was issued.  There are also a few setVersion instances in the chrome browser tests that need to be taken care of.
Comment 3 Joshua Bell 2012-11-15 16:31:59 PST
Comment on attachment 174519 [details]
Patch

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

lgtm

> LayoutTests/ChangeLog:18
> +          Again awkward to call indexedDBTest all over the place. The prefix

We can delete removeVendorPrefixes() whenever we want now, right? (i.e. after the last batch is done)

> LayoutTests/ChangeLog:19
> +          line's gotta go, and we could also pull a resetDatabase method out of

How about simpleIndexedDBTest(upgradeNeededHandler, callback) that has onsuccess = { db.close(); callback(); }

> LayoutTests/ChangeLog:39
> +        * storage/indexeddb/resources/versionchangerequest-activedomobject.js:

I don't know if this is exercising the same code as before (the test looks like it's watching for a specific regression), but it seems fine.
Comment 4 David Grogan 2012-11-15 16:48:37 PST
Comment on attachment 174519 [details]
Patch

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

>> LayoutTests/ChangeLog:18
>> +          Again awkward to call indexedDBTest all over the place. The prefix
> 
> We can delete removeVendorPrefixes() whenever we want now, right? (i.e. after the last batch is done)

I think we should keep the prefix check in until 24 goes to stable in the off chance that we want to open any of the layout tests in 23.  But we can remove its output via Quiet whenever.

>> LayoutTests/ChangeLog:19
>> +          line's gotta go, and we could also pull a resetDatabase method out of
> 
> How about simpleIndexedDBTest(upgradeNeededHandler, callback) that has onsuccess = { db.close(); callback(); }

That does seem promising.
Comment 5 David Grogan 2012-11-15 16:49:36 PST
Tony, could you review this?
Comment 6 WebKit Review Bot 2012-11-15 17:14:47 PST
Comment on attachment 174519 [details]
Patch

Clearing flags on attachment: 174519

Committed r134866: <http://trac.webkit.org/changeset/134866>
Comment 7 WebKit Review Bot 2012-11-15 17:14:50 PST
All reviewed patches have been landed.  Closing bug.