Bug 150839

Summary:

Modern IDB: Land empty IDBCursor/Index IDL implementations

Product:

WebKit

Reporter:

Brady Eidson <beidson>

Component:

WebCore Misc.

Assignee:

Brady Eidson <beidson>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

achristensen, commit-queue

Priority:

Version:

Safari 9

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

149117

Attachments:

Description	Flags
Patch v1	none
Patch v2	achristensen: review-, achristensen: commit-queue-
Patch v3	none

Brady Eidson

Reported 2015-11-03 09:11:32 PST

Modern IDB: Land IDBCursor/Index IDL implementations They'll basically be no-ops at this point, but just get a lot of template/boilerplate code out of the way, as well as project file changes.

Attachments
Patch v1 (38.04 KB, patch) 2015-11-03 09:36 PST, Brady Eidson	no flags	Details Formatted Diff Diff
Patch v2 (37.85 KB, patch) 2015-11-03 12:08 PST, Brady Eidson	achristensen: review- achristensen: commit-queue-	Details Formatted Diff Diff
Patch v3 (37.88 KB, patch) 2015-11-03 13:34 PST, Brady Eidson	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Brady Eidson

Comment 1 2015-11-03 09:36:33 PST

Created attachment 264695 [details] Patch v1

Brady Eidson

Comment 2 2015-11-03 12:08:03 PST

Created attachment 264707 [details] Patch v2

WebKit Commit Bot

Comment 3 2015-11-03 12:11:00 PST

Attachment 264707 [details] did not pass style-queue: ERROR: Source/WebCore/Modules/indexeddb/client/IDBCursorWithValueImpl.h:26: #ifndef header guard has wrong style, please use: IDBCursorWithValueImpl_h [build/header_guard] [5] ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 2 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style.

Alex Christensen

Comment 4 2015-11-03 12:34:15 PST

Comment on attachment 264707 [details] Patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=264707&action=review Looks mostly good. I'll take a glance at the next patch > Source/WebCore/ChangeLog:8 > + No new tests (OOPS!). remove > Source/WebCore/Modules/indexeddb/client/IDBCursorImpl.h:42 > + virtual const Deprecated::ScriptValue& key() const override final; new code with deprecated? > Source/WebCore/Modules/indexeddb/shared/IDBIndexInfo.cpp:33 > +namespace WebCore { > + > +} // namespace WebCore This file looks pretty exciting so far. > Source/WebCore/Modules/indexeddb/shared/IDBIndexInfo.h:44 > +private: > + IDBIndexInfo(); how do you make one of these?

Brady Eidson

Comment 5 2015-11-03 13:33:36 PST

(In reply to comment #4) > Comment on attachment 264707 [details] > Patch v2 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=264707&action=review > > Looks mostly good. I'll take a glance at the next patch > > > Source/WebCore/ChangeLog:8 > > + No new tests (OOPS!). > > remove Whoops. > > > Source/WebCore/Modules/indexeddb/client/IDBCursorImpl.h:42 > > + virtual const Deprecated::ScriptValue& key() const override final; > > new code with deprecated? > New code implementing old interface. After LegacyIDB is nuked would be a good time to do a sweep of all of that (including the JS bindings, unfortunately) https://bugs.webkit.org/show_bug.cgi?id=150854 for that > > Source/WebCore/Modules/indexeddb/shared/IDBIndexInfo.cpp:33 > > +namespace WebCore { > > + > > +} // namespace WebCore > > This file looks pretty exciting so far. It is not! > > > Source/WebCore/Modules/indexeddb/shared/IDBIndexInfo.h:44 > > +private: > > + IDBIndexInfo(); > > how do you make one of these? In this patch, you do not!

Brady Eidson

Comment 6 2015-11-03 13:34:11 PST

Created attachment 264719 [details] Patch v3

WebKit Commit Bot

Comment 7 2015-11-03 14:25:07 PST

Comment on attachment 264719 [details] Patch v3 Clearing flags on attachment: 264719 Committed r191980: <http://trac.webkit.org/changeset/191980>

WebKit Commit Bot

Comment 8 2015-11-03 14:25:12 PST

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.