Bug 96444 - IndexedDB: fire upgradeneeded even without an explicit integer version
Summary: IndexedDB: fire upgradeneeded even without an explicit integer version
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: 90411
Blocks: 96548 96952
  Show dependency tree / graph
 
Reported: 2012-09-11 16:06 PDT by David Grogan
Modified: 2012-09-19 13:28 PDT (History)
4 users (show)

See Also:

Attachments
Patch (36.18 KB, patch)
2012-09-11 16:06 PDT, David Grogan 		no flags Details | Formatted Diff | Diff
Patch (40.53 KB, patch)
2012-09-11 16:08 PDT, David Grogan 		no flags Details | Formatted Diff | Diff
Patch (41.34 KB, patch)
2012-09-11 17:32 PDT, David Grogan 		no flags Details | Formatted Diff | Diff
Patch (44.88 KB, patch)
2012-09-11 18:41 PDT, David Grogan 		no flags Details | Formatted Diff | Diff
Patch (47.45 KB, patch)
2012-09-12 15:59 PDT, David Grogan 		no flags Details | Formatted Diff | Diff
Patch (49.30 KB, patch)
2012-09-13 18:18 PDT, David Grogan 		no flags Details | Formatted Diff | Diff
Patch (49.30 KB, patch)
2012-09-13 19:32 PDT, David Grogan 		no flags Details | Formatted Diff | Diff
ToT (49.25 KB, patch)
2012-09-17 15:31 PDT, David Grogan 		no flags Details | Formatted Diff | Diff
Show Obsolete (7) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description David Grogan 2012-09-11 16:06:03 PDT 
IndexedDB: fire upgradeneeded even without an explicit integer version
Comment 1 David Grogan 2012-09-11 16:06:30 PDT 
Created attachment 163468 [details]
Patch
Comment 2 David Grogan 2012-09-11 16:08:08 PDT 
Created attachment 163469 [details]
Patch
Comment 3 David Grogan 2012-09-11 17:32:49 PDT 
Created attachment 163488 [details]
Patch
Comment 4 David Grogan 2012-09-11 18:41:34 PDT 
Created attachment 163492 [details]
Patch
Comment 5 David Grogan 2012-09-11 18:47:49 PDT 
With this patch we do indeed pass the w3 tests Josh separated out.

Except one, http://w3c-test.org/webapps/IndexedDB/tests/submissions/Ms2ger/idbfactory_open9.htm .  It fails because of a different int-version issue.

This patch doesn't apply to ToT because it is based on 90411.
Comment 6 David Grogan 2012-09-12 15:59:26 PDT 
Created attachment 163725 [details]
Patch
Comment 7 David Grogan 2012-09-12 16:03:56 PDT 
jsbell@, could you take a look at this? As we predicted there's not much code change, mostly tests.
Comment 8 Joshua Bell 2012-09-13 11:29:27 PDT 
Comment on attachment 163725 [details]
Patch

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

LGTM!

> Source/WebCore/Modules/indexeddb/IDBLevelDBBackingStore.cpp:-342
> -    // FIXME: Change this to strictly greater than 0 once we throw TypeError for

This patch does not alter the (erroneous) behavior for open(name, -1), correct? It's still an alias for open(name), and that will be fixed (on the front end) in a separate bug?
Comment 9 David Grogan 2012-09-13 18:18:58 PDT 
Created attachment 164020 [details]
Patch
Comment 10 WebKit Review Bot 2012-09-13 18:22:01 PDT 
Attachment 164020 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/http..." exit_code: 1
Source/WebCore/ChangeLog:9:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Source/WebCore/ChangeLog:11:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Total errors found: 2 in 30 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 11 David Grogan 2012-09-13 19:32:30 PDT 
Created attachment 164030 [details]
Patch
Comment 12 David Grogan 2012-09-13 19:39:32 PDT 
Comment on attachment 163725 [details]
Patch

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

>> Source/WebCore/Modules/indexeddb/IDBLevelDBBackingStore.cpp:-342
>> -    // FIXME: Change this to strictly greater than 0 once we throw TypeError for
> 
> This patch does not alter the (erroneous) behavior for open(name, -1), correct? It's still an alias for open(name), and that will be fixed (on the front end) in a separate bug?

That's right.
Comment 13 David Grogan 2012-09-17 15:31:59 PDT 
Created attachment 164462 [details]
ToT
Comment 14 David Grogan 2012-09-17 15:33:43 PDT 
Tony, could you review this?  I'm not going to land it until tomorrow so it's not urgent at all.
Comment 15 David Grogan 2012-09-17 15:34:05 PDT 
Tony, could you review this?  I'm not going to land it until tomorrow so it's not urgent at all.
Comment 16 WebKit Review Bot 2012-09-19 13:28:12 PDT 
Comment on attachment 164462 [details]
ToT

Clearing flags on attachment: 164462

Committed r129037: <http://trac.webkit.org/changeset/129037>
Comment 17 WebKit Review Bot 2012-09-19 13:28:16 PDT 
All reviewed patches have been landed.  Closing bug.