Bug 91414 - IndexedDB: Include intVersion when converting between WebCore and WebKit IDBMetadata types
Summary: IndexedDB: Include intVersion when converting between WebCore and WebKit IDBM...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 312.x
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Grogan
URL:
Keywords:
Depends on: 91408
Blocks:
  Show dependency tree / graph
 
Reported: 2012-07-16 12:30 PDT by David Grogan
Modified: 2012-07-20 14:56 PDT (History)
4 users (show)

See Also:

Attachments
Patch (4.62 KB, patch)
2012-07-16 12:32 PDT, David Grogan 		no flags Details | Formatted Diff | Diff
Patch (5.84 KB, patch)
2012-07-16 14:24 PDT, David Grogan 		no flags Details | Formatted Diff | Diff
Patch (5.93 KB, patch)
2012-07-16 14:28 PDT, David Grogan 		no flags Details | Formatted Diff | Diff
Patch (6.77 KB, patch)
2012-07-16 14:32 PDT, David Grogan 		no flags Details | Formatted Diff | Diff
Patch (6.00 KB, patch)
2012-07-17 16:13 PDT, David Grogan 		no flags Details | Formatted Diff | Diff
Patch (5.93 KB, patch)
2012-07-18 18:40 PDT, David Grogan 		no flags Details | Formatted Diff | Diff
Patch for landing (5.89 KB, patch)
2012-07-20 13:33 PDT, David Grogan 		no flags Details | Formatted Diff | Diff
Show Obsolete (6) 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-07-16 12:30:40 PDT 
IndexedDB: Include intVersion when converting between WebCore and WebKit IDBMetadata types
Comment 1 David Grogan 2012-07-16 12:32:23 PDT 
Created attachment 152587 [details]
Patch
Comment 2 WebKit Review Bot 2012-07-16 12:56:51 PDT 
Comment on attachment 152587 [details]
Patch

Attachment 152587 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13262069
Comment 3 David Grogan 2012-07-16 13:18:14 PDT 
This won't pass cr-linux until 91408 lands.
Comment 4 David Grogan 2012-07-16 14:24:56 PDT 
Created attachment 152611 [details]
Patch
Comment 5 David Grogan 2012-07-16 14:28:18 PDT 
Created attachment 152612 [details]
Patch
Comment 6 David Grogan 2012-07-16 14:32:42 PDT 
Created attachment 152614 [details]
Patch
Comment 7 WebKit Review Bot 2012-07-16 14:34:01 PDT 
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment 8 David Grogan 2012-07-17 16:13:43 PDT 
Created attachment 152862 [details]
Patch
Comment 9 David Grogan 2012-07-18 18:40:26 PDT 
Created attachment 153155 [details]
Patch
Comment 10 David Grogan 2012-07-18 18:42:44 PDT 
Josh or Alec, could you take a look at this?
Comment 11 Joshua Bell 2012-07-20 09:19:03 PDT 
Comment on attachment 153155 [details]
Patch

LGTM.

Having intVersion be an optional add-on in the API rather than the new default (and thus making the legacy string version optional) means there will be more "clean up" work later on, but this is probably the safest incremental approach to get the new support in and under test.
Comment 12 Joshua Bell 2012-07-20 09:23:00 PDT 
abarth@ can you r?
Comment 13 Adam Barth 2012-07-20 09:26:56 PDT 
Comment on attachment 153155 [details]
Patch

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

> Source/WebCore/Modules/indexeddb/IDBMetadata.h:50
> +        : intVersion(NoIntVersion) { }

The { } should each be on their own line.
Comment 14 David Grogan 2012-07-20 13:33:14 PDT 
Created attachment 153585 [details]
Patch for landing
Comment 15 David Grogan 2012-07-20 13:33:48 PDT 
Comment on attachment 153155 [details]
Patch

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

Adam and Josh, thanks for the review.

>> Source/WebCore/Modules/indexeddb/IDBMetadata.h:50
>> +        : intVersion(NoIntVersion) { }
> 
> The { } should each be on their own line.

Done.
Comment 16 WebKit Review Bot 2012-07-20 14:56:22 PDT 
Comment on attachment 153585 [details]
Patch for landing

Clearing flags on attachment: 153585

Committed r123267: <http://trac.webkit.org/changeset/123267>
Comment 17 WebKit Review Bot 2012-07-20 14:56:27 PDT 
All reviewed patches have been landed.  Closing bug.