Bug 149234

Summary: Have window.indexedDB.open return an IDBOpenDBRequest
Product: WebKit Reporter: Brady Eidson <beidson>
Component: WebCore Misc.Assignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, alecflett, commit-queue, jsbell
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 149117    
Attachments:
Description Flags
Patch v1 none

Description Brady Eidson 2015-09-16 15:10:49 PDT
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.
Comment 1 Brady Eidson 2015-09-16 15:12:57 PDT
Created attachment 261330 [details]
Patch v1
Comment 2 Brady Eidson 2015-09-16 16:20:54 PDT
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...?)
Comment 3 Alex Christensen 2015-09-16 20:29:48 PDT
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?
Comment 4 Brady Eidson 2015-09-16 20:40:54 PDT
(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!
Comment 5 WebKit Commit Bot 2015-09-16 21:28:01 PDT
Comment on attachment 261330 [details]
Patch v1

Clearing flags on attachment: 261330

Committed r189901: <http://trac.webkit.org/changeset/189901>
Comment 6 WebKit Commit Bot 2015-09-16 21:28:05 PDT
All reviewed patches have been landed.  Closing bug.