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: | P2 | ||||||||||||||
Version: | Safari 9 | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Bug Depends on: | 149342 | ||||||||||||||
Bug Blocks: | 149117 | ||||||||||||||
Attachments: |
|
Description
Brady Eidson
2015-09-15 15:27:30 PDT
Created attachment 261250 [details]
Patch v1
Created attachment 261251 [details]
Patch v1
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. (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 Created attachment 261254 [details]
Patch v2 - For EWS before landing
Created attachment 261260 [details]
Patch v3 - EWS/landing
Created attachment 261263 [details]
Patch v4 - EWS/landing
|