Bug 149191 - Add empty IDBFactory implementation for Modern IDB
Summary: Add empty IDBFactory implementation for Modern IDB
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: Safari 9
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords:
Depends on: 149342
Blocks: 149117
  Show dependency treegraph
 
Reported: 2015-09-15 15:27 PDT by Brady Eidson
Modified: 2015-09-18 05:42 PDT (History)
2 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 2015-09-15 15:27:30 PDT
Add empty IDBFactory implementation for Modern IDB

This includes enabling DatabaseProvider to optionally choose it.
Comment 1 Brady Eidson 2015-09-15 15:29:57 PDT
Created attachment 261250 [details]
Patch v1
Comment 2 Brady Eidson 2015-09-15 15:31:40 PDT
Created attachment 261251 [details]
Patch v1
Comment 3 Jer Noble 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.
Comment 4 Brady Eidson 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
Comment 5 Brady Eidson 2015-09-15 15:51:59 PDT
Created attachment 261254 [details]
Patch v2 - For EWS before landing
Comment 6 Brady Eidson 2015-09-15 16:38:33 PDT
Created attachment 261260 [details]
Patch v3 - EWS/landing
Comment 7 Brady Eidson 2015-09-15 16:45:12 PDT
Created attachment 261263 [details]
Patch v4 - EWS/landing
Comment 8 Brady Eidson 2015-09-15 17:19:56 PDT
https://trac.webkit.org/changeset/189831