Bug 149234 - Have window.indexedDB.open return an IDBOpenDBRequest
Summary: Have window.indexedDB.open 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 15:10 PDT by Brady Eidson
Modified: 2015-09-16 21:28 PDT (History)
4 users (show)

See Also:


Attachments
Patch v1 (5.88 KB, patch)
2015-09-16 15:12 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-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.