Bug 149229 - Have window.indexedDB.deleteDatabase return an IDBOpenDBRequest
Summary: Have window.indexedDB.deleteDatabase return an IDBOpenDBRequest
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords:
Depends on:
Blocks: 149117
  Show dependency treegraph
 
Reported: 2015-09-16 13:19 PDT by Brady Eidson
Modified: 2015-09-16 22:25 PDT (History)
2 users (show)

See Also:


Attachments
Patch v1 (40.44 KB, patch)
2015-09-16 13:25 PDT, Brady Eidson
achristensen: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2015-09-16 13:19:20 PDT
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.
Comment 1 Brady Eidson 2015-09-16 13:25:44 PDT
Created attachment 261322 [details]
Patch v1
Comment 2 Alex Christensen 2015-09-16 13:35:33 PDT
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?
Comment 3 Brady Eidson 2015-09-16 13:43:00 PDT
(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.
Comment 4 Brady Eidson 2015-09-16 13:45:59 PDT
https://trac.webkit.org/changeset/189879
Comment 5 Ryosuke Niwa 2015-09-16 22:23:48 PDT
Rebaselined js/dom/global-constructors-attributes-idb.html for WK1 in https://trac.webkit.org/changeset/189904.
Comment 6 Alexey Proskuryakov 2015-09-16 22:25:07 PDT
Please do wait for EWS, it's totally worth it.