Have window.indexedDB.deleteDatabase return an IDBOpenDBRequest

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Brady Eidson]

Have window.indexedDB.deleteDatabase return an IDBOpenDBRequest

The request will be non-functional, and a database will never actually be deleted.

But an instance of the object will exist.

[Brady Eidson]

Created attachment 261322 [details]
Patch v1

[Alex Christensen]

Comment on attachment 261322 [details]
Patch v1

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

looks mostly good.  Please upload another patch.

> Source/WebCore/Modules/indexeddb/client/IDBFactoryImpl.cpp:71
> +PassRefPtr<WebCore::IDBRequest> IDBFactory::getDatabaseNames(ScriptExecutionContext*, ExceptionCode&)

Just return a RefPtr, no PassRefPtr.  Here and elsewhere in patch.  We're trying to get rid of PassRefPtr.

> Source/WebCore/Modules/indexeddb/client/IDBFactoryImpl.cpp:86
> +PassRefPtr<WebCore::IDBOpenDBRequest> IDBFactory::deleteDatabase(ScriptExecutionContext* context, const String& name, ExceptionCode& ec)

can context be nullptr?

> Source/WebCore/Modules/indexeddb/client/IDBRequestImpl.cpp:68
> +    static WTF::NeverDestroyed<String> readyState;

Will there only be one readyState for all IDBRequests?

[Brady Eidson]

(In reply to comment #2)
> Comment on attachment 261322 [details]
> Patch v1
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=261322&action=review
> 
> looks mostly good.  Please upload another patch.
> 
> > Source/WebCore/Modules/indexeddb/client/IDBFactoryImpl.cpp:71
> > +PassRefPtr<WebCore::IDBRequest> IDBFactory::getDatabaseNames(ScriptExecutionContext*, ExceptionCode&)
> 
> Just return a RefPtr, no PassRefPtr.  Here and elsewhere in patch.  We're
> trying to get rid of PassRefPtr.

This is a new concrete implementation of the IDB classes which has plenty of PassRefPtr usage. The LegacyIDB* implementation also uses it.

My plan is to finish the new implementation, remove Legacy, then give the IDL implementation layer a nice cleanup.

> 
> > Source/WebCore/Modules/indexeddb/client/IDBFactoryImpl.cpp:86
> > +PassRefPtr<WebCore::IDBOpenDBRequest> IDBFactory::deleteDatabase(ScriptExecutionContext* context, const String& name, ExceptionCode& ec)
> 
> can context be nullptr?

Nope. Man, the bindings generator should pass in references...

> > Source/WebCore/Modules/indexeddb/client/IDBRequestImpl.cpp:68
> > +    static WTF::NeverDestroyed<String> readyState;
> 
> Will there only be one readyState for all IDBRequests?

Right now, yes. In the future, this method will return one of 3 static strings. But the purpose of this patch (and handfuls of followups coming soon) is to just start filling in code, getting things building, etc etc.

[Brady Eidson]

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

[Ryosuke Niwa]

Rebaselined js/dom/global-constructors-attributes-idb.html for WK1 in https://trac.webkit.org/changeset/189904.

[Alexey Proskuryakov]

Please do wait for EWS, it's totally worth it.