Bug 149234 – Have window.indexedDB.open return an IDBOpenDBRequest

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Brady Eidson]

Have window.indexedDB.open return an IDBOpenDBRequest

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

But an instance of the object will exist.

[Brady Eidson]

Created attachment 261330 [details]
Patch v1

[Brady Eidson]

The iOS build break is weird and seems unrelated to this patch. The logs don't even tell me what broke. (Maybe it's in there but not obvious to me...?)

[Alex Christensen]

Comment on attachment 261330 [details]
Patch v1

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

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

You really ought to make these ScriptExecutionContext&

> Source/WebCore/Modules/indexeddb/client/IDBFactoryImpl.cpp:108
> +    ASSERT(context->securityOrigin());
> +    ASSERT(context->topOrigin());

Why are these always non-null?

[Brady Eidson]

(In reply to comment #3)
> Comment on attachment 261330 [details]
> Patch v1
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=261330&action=review
> 
> > Source/WebCore/Modules/indexeddb/client/IDBFactoryImpl.cpp:76
> > +PassRefPtr<WebCore::IDBOpenDBRequest> IDBFactory::open(ScriptExecutionContext* context, const String& name, ExceptionCode& ec)
> 
> You really ought to make these ScriptExecutionContext&

Definitely part of 149117. But hacking on IDL/binding generation is not my priority right now.

> 
> > Source/WebCore/Modules/indexeddb/client/IDBFactoryImpl.cpp:108
> > +    ASSERT(context->securityOrigin());
> > +    ASSERT(context->topOrigin());
> 
> Why are these always non-null?

In practice, the only ScriptExecutionContext ever used is Document, and one can verify through code inspection these are non-null by the time script runs.

The Legacy IDB code blindly assumed they were non-null, but the ASSERTs are definitely warranted in case that ever changes. Or, also to capture any non-document context starting to be used where one/both are null.

Thanks for the review!

[WebKit Commit Bot]

Comment on attachment 261330 [details]
Patch v1

Clearing flags on attachment: 261330

Committed r189901: <http://trac.webkit.org/changeset/189901>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.