Bug 149191

Summary:

Add empty IDBFactory implementation for Modern IDB

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:

149342

Bug Blocks:

149117

Attachments:

Description	Flags
Patch v1	beidson: review-
Patch v1	jer.noble: review+
Patch v2 - For EWS before landing	beidson: commit-queue+
Patch v3 - EWS/landing	beidson: commit-queue-
Patch v4 - EWS/landing	none

Brady Eidson

Reported 2015-09-15 15:27:30 PDT

Add empty IDBFactory implementation for Modern IDB This includes enabling DatabaseProvider to optionally choose it.

Attachments
Patch v1 (15.39 KB, patch) 2015-09-15 15:29 PDT, Brady Eidson	beidson: review-	Details Formatted Diff Diff
Patch v1 (16.30 KB, patch) 2015-09-15 15:31 PDT, Brady Eidson	jer.noble: review+	Details Formatted Diff Diff
Patch v2 - For EWS before landing (16.28 KB, patch) 2015-09-15 15:51 PDT, Brady Eidson	beidson: commit-queue+	Details Formatted Diff Diff
Patch v3 - EWS/landing (11.01 KB, patch) 2015-09-15 16:38 PDT, Brady Eidson	beidson: commit-queue-	Details Formatted Diff Diff
Patch v4 - EWS/landing (16.31 KB, patch) 2015-09-15 16:45 PDT, Brady Eidson	no flags	Details Formatted Diff Diff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.

Brady Eidson

Comment 1 2015-09-15 15:29:57 PDT

Created attachment 261250 [details] Patch v1

Brady Eidson

Comment 2 2015-09-15 15:31:40 PDT

Created attachment 261251 [details] Patch v1

Jer Noble

Comment 3 2015-09-15 15:48:14 PDT

Comment on attachment 261251 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=261251&action=review r=me with nits > Source/WebCore/Modules/indexeddb/client/IDBFactoryImpl.cpp:62 > +Ref<IDBFactory> IDBFactory::create() > +{ > + return adoptRef(*new IDBFactory); > +} > + > +IDBFactory::IDBFactory() > +{ > + > +} > + > +PassRefPtr<IDBRequest> IDBFactory::getDatabaseNames(ScriptExecutionContext*, ExceptionCode&) > +{ > + return nullptr; > +} > + > +PassRefPtr<IDBOpenDBRequest> IDBFactory::open(ScriptExecutionContext*, const String&, ExceptionCode&) > +{ > + return nullptr; > +} > + > +PassRefPtr<IDBOpenDBRequest> IDBFactory::open(ScriptExecutionContext*, const String&, unsigned long long, ExceptionCode&) > +{ > + return nullptr; > +} > + > +PassRefPtr<IDBOpenDBRequest> IDBFactory::deleteDatabase(ScriptExecutionContext*, const String&, ExceptionCode&) > +{ > + return nullptr; > +} Why isn't the ScriptExecutionContext* passed into the create() method, rather than into every method called on the IDBFactory? > Source/WebCore/Modules/indexeddb/client/IDBFactoryImpl.cpp:64 > +short IDBFactory::cmp(ScriptExecutionContext*, const Deprecated::ScriptValue&, const Deprecated::ScriptValue&, ExceptionCode&) We shouldn't add any more method names as bad as "cmp". Could we add a "ImplementedAs=compare" attribute to the IDL? > Source/WebCore/Modules/indexeddb/client/IDBFactoryImpl.h:46 > + virtual short cmp(ScriptExecutionContext*, const Deprecated::ScriptValue& first, const Deprecated::ScriptValue& second, ExceptionCode&) override final; Ditto.

Brady Eidson

Comment 4 2015-09-15 15:50:34 PDT

(In reply to comment #3) > Comment on attachment 261251 [details] > Patch v1 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=261251&action=review > > r=me with nits > > > Source/WebCore/Modules/indexeddb/client/IDBFactoryImpl.cpp:62 > > +Ref<IDBFactory> IDBFactory::create() > > +{ > > + return adoptRef(*new IDBFactory); > > +} > > + > > +IDBFactory::IDBFactory() > > +{ > > + > > +} > > + > > +PassRefPtr<IDBRequest> IDBFactory::getDatabaseNames(ScriptExecutionContext*, ExceptionCode&) > > +{ > > + return nullptr; > > +} > > + > > +PassRefPtr<IDBOpenDBRequest> IDBFactory::open(ScriptExecutionContext*, const String&, ExceptionCode&) > > +{ > > + return nullptr; > > +} > > + > > +PassRefPtr<IDBOpenDBRequest> IDBFactory::open(ScriptExecutionContext*, const String&, unsigned long long, ExceptionCode&) > > +{ > > + return nullptr; > > +} > > + > > +PassRefPtr<IDBOpenDBRequest> IDBFactory::deleteDatabase(ScriptExecutionContext*, const String&, ExceptionCode&) > > +{ > > + return nullptr; > > +} > > Why isn't the ScriptExecutionContext* passed into the create() method, > rather than into every method called on the IDBFactory? > > > Source/WebCore/Modules/indexeddb/client/IDBFactoryImpl.cpp:64 > > +short IDBFactory::cmp(ScriptExecutionContext*, const Deprecated::ScriptValue&, const Deprecated::ScriptValue&, ExceptionCode&) > > We shouldn't add any more method names as bad as "cmp". Could we add a > "ImplementedAs=compare" attribute to the IDL? > > > Source/WebCore/Modules/indexeddb/client/IDBFactoryImpl.h:46 > > + virtual short cmp(ScriptExecutionContext*, const Deprecated::ScriptValue& first, const Deprecated::ScriptValue& second, ExceptionCode&) override final; > > Ditto. These are great points to revisit while cleaning up the IDL, which will be part of https://bugs.webkit.org/show_bug.cgi?id=149117

Brady Eidson

Comment 5 2015-09-15 15:51:59 PDT

Created attachment 261254 [details] Patch v2 - For EWS before landing

Brady Eidson

Comment 6 2015-09-15 16:38:33 PDT

Created attachment 261260 [details] Patch v3 - EWS/landing

Brady Eidson

Comment 7 2015-09-15 16:45:12 PDT

Created attachment 261263 [details] Patch v4 - EWS/landing

Brady Eidson

Comment 8 2015-09-15 17:19:56 PDT

https://trac.webkit.org/changeset/189831

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