Bug 96548 - Unprefix IndexedDB
Summary: Unprefix IndexedDB
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: WebExposed
Depends on: 96444 96575
Blocks:
  Show dependency tree / graph
 
Reported: 2012-09-12 13:13 PDT by David Grogan
Modified: 2012-09-24 10:17 PDT (History)
9 users (show)

See Also:

Attachments
Patch (4.32 KB, patch)
2012-09-12 13:13 PDT, David Grogan 		no flags Details | Formatted Diff | Diff
unprefixdomwindow (8.45 KB, patch)
2012-09-12 13:51 PDT, David Grogan 		no flags Details | Formatted Diff | Diff
Patch (17.51 KB, patch)
2012-09-20 15:23 PDT, David Grogan 		no flags Details | Formatted Diff | Diff
Patch for landing (17.51 KB, patch)
2012-09-24 09:48 PDT, David Grogan 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) 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-12 13:13:03 PDT 
Unprefix IndexedDB
Comment 1 David Grogan 2012-09-12 13:13:22 PDT 
Created attachment 163678 [details]
Patch
Comment 2 Adam Barth 2012-09-12 13:21:26 PDT 
Comment on attachment 163678 [details]
Patch

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

> Source/WebCore/Modules/indexeddb/DOMWindowIndexedDatabase.h:45
>      static IDBFactory* webkitIndexedDB(DOMWindow*);
> +    static IDBFactory* indexedDB(DOMWindow*);

You should just delete webkitIndexedDB.  It's not needed.

> Source/WebCore/Modules/indexeddb/DOMWindowIndexedDatabase.idl:45
> +        readonly attribute IDBFactory indexedDB;

The webkitIndexedDB entrypoint should use https://trac.webkit.org/wiki/WebKitIDL#ImplementedAs
Comment 3 Adam Barth 2012-09-12 13:21:57 PDT 
Comment on attachment 163678 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        Additional information of the change such as approach, rationale. Please add per-function descriptions below (OOPS!).

Please fill out this part of the ChangeLog.  For example, do we pass a certain test suite?
Comment 4 David Grogan 2012-09-12 13:22:58 PDT 
Sorry, I shouldn't have marked this r?, but thanks for the early feedback!
Comment 5 David Grogan 2012-09-12 13:51:52 PDT 
Created attachment 163685 [details]
unprefixdomwindow
Comment 6 Joshua Bell 2012-09-12 15:22:46 PDT 
Reminder: we should ensure setVersion() and any other legacy/nonstandard APIs generate console warnings, or have webkit prefixes.

(I can't think of any other that setVersion() we haven't already done this for.)
Comment 7 Adam Barth 2012-09-14 10:53:09 PDT 
By the way, would you be willing to add a histogram to compare prefixed use of IDB to unprefixed use?  (Like we have for webktiSlice: http://trac.webkit.org/browser/trunk/Source/WebCore/fileapi/Blob.cpp#L89)

Note: That might mean that you won't be able to use the slick ImplementedAs IDL attribute.  Also, should we show a deprecation message encouraging folks to move off the prefixed API?
Comment 8 David Grogan 2012-09-20 15:23:55 PDT 
Created attachment 164992 [details]
Patch
Comment 9 David Grogan 2012-09-20 15:30:19 PDT 
jsbell, abarth, anyone, comments?

After taking a look at the generated code, it appears that [ImplementedAs=indexedDB,V8MeasureAs=PrefixedIndexedDB] will do what we want.
Comment 10 Joshua Bell 2012-09-20 15:43:27 PDT 
Comment on attachment 164992 [details]
Patch

lgtm
Comment 11 David Grogan 2012-09-24 09:48:38 PDT 
Created attachment 165401 [details]
Patch for landing
Comment 12 WebKit Review Bot 2012-09-24 10:17:03 PDT 
Comment on attachment 165401 [details]
Patch for landing

Clearing flags on attachment: 165401

Committed r129385: <http://trac.webkit.org/changeset/129385>
Comment 13 WebKit Review Bot 2012-09-24 10:17:07 PDT 
All reviewed patches have been landed.  Closing bug.