Bug 149191 – Add empty IDBFactory implementation for Modern IDB

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Brady Eidson]

Add empty IDBFactory implementation for Modern IDB

This includes enabling DatabaseProvider to optionally choose it.

[Brady Eidson]

Created attachment 261250 [details]
Patch v1

[Brady Eidson]

Created attachment 261251 [details]
Patch v1

[Jer Noble]

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]

(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]

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

[Brady Eidson]

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

[Brady Eidson]

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

[Brady Eidson]

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