Bug 156883

Summary: Support disabling at runtime IndexedDB constructors exposed to workers
Product: WebKit Reporter: Brady Eidson <beidson>
Component: WebCore Misc.Assignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: alecflett, cdumez, commit-queue, esprehn+autocc, jsbell, kondapallykalyan
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 149117    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Description Brady Eidson 2016-04-21 17:16:28 PDT 
Disable exposing IDBVersionChangeEvent and IDBKeyRange to WorkerGlobalScope when IDB-in-Workers is disabled

I ran the workers interfaces test:
http://w3c-test.org/IndexedDB/interfaces.worker

...with the "Indexed DB in workers" runtime flag off, yet the interfaces for these two objects came up.

In both of their IDLs they are exposed to both Window and Workers. But I need the exposure to "Workers" to be based on the IDB-in-workers runtime enabled flag.

If these interfaces exist even when IDB-in-worker is disabled, IDB-in-workers cannot be properly polyfilled.
Comment 1 Brady Eidson 2016-04-21 17:17:02 PDT 
Chris, you're pretty darned knowledgeable in this area - What can we do?
Comment 2 Chris Dumez 2016-04-21 18:31:53 PDT 
(In reply to comment #1)
> Chris, you're pretty darned knowledgeable in this area - What can we do?

The easy way to do this right now would be to do:
1. Drop the "Exposed=(Window,Worker)" from IDBVersionChangeEvent and IDBKeyRange so that they are only exposed to Window by default.

then

2. Add explicit Constructor attributes on WorkerGlobalScope (for IDBVersionChangeEvent and IDBKeyRange) that are either [Conditional=INDEXED_DATABASE] or [EnabledAtRuntime=FetchAPI] depending if IDB-in-Workers is a compile-time or a runtime thing.
Comment 3 Chris Dumez 2016-04-21 18:33:10 PDT 
(In reply to comment #2)
> (In reply to comment #1)
> > Chris, you're pretty darned knowledgeable in this area - What can we do?
> 
> The easy way to do this right now would be to do:
> 1. Drop the "Exposed=(Window,Worker)" from IDBVersionChangeEvent and
> IDBKeyRange so that they are only exposed to Window by default.
> 
> then
> 
> 2. Add explicit Constructor attributes on WorkerGlobalScope (for
> IDBVersionChangeEvent and IDBKeyRange) that are either
> [Conditional=INDEXED_DATABASE] or [EnabledAtRuntime=FetchAPI] depending if
> IDB-in-Workers is a compile-time or a runtime thing.

Please disregard the INDEXED_DATABASE / FetchAPI I used in my example. I assume you have a flag for IDB-In-Workers already.
Comment 4 Chris Dumez 2016-04-21 18:38:40 PDT 
(In reply to comment #3)
> (In reply to comment #2)
> > (In reply to comment #1)
> > > Chris, you're pretty darned knowledgeable in this area - What can we do?
> > 
> > The easy way to do this right now would be to do:
> > 1. Drop the "Exposed=(Window,Worker)" from IDBVersionChangeEvent and
> > IDBKeyRange so that they are only exposed to Window by default.
> > 
> > then
> > 
> > 2. Add explicit Constructor attributes on WorkerGlobalScope (for
> > IDBVersionChangeEvent and IDBKeyRange) that are either
> > [Conditional=INDEXED_DATABASE] or [EnabledAtRuntime=FetchAPI] depending if
> > IDB-in-Workers is a compile-time or a runtime thing.
> 
> Please disregard the INDEXED_DATABASE / FetchAPI I used in my example. I
> assume you have a flag for IDB-In-Workers already.

Ok, I think I found your flag so basically
2. add:
[Conditional=INDEXED_DATABASE, EnabledAtRuntime=IndexedDBWorkers] attribute IDBVersionChangeEventConstructor IDBVersionChangeEvent;
[Conditional=INDEXED_DATABASE, EnabledAtRuntime=IndexedDBWorkers] attribute IDBKeyRangeConstructor IDBKeyRange;

to WorkerGlobalScope.idl
Comment 5 Chris Dumez 2016-04-21 18:39:54 PDT 
BTW, how about all the other IDB constructors exposed on WorkerGlobalScope?

I see:
[Conditional=INDEXED_DATABASE, EnabledAtRuntime=IndexedDB] attribute IDBCursorConstructor IDBCursor;
[Conditional=INDEXED_DATABASE, EnabledAtRuntime=IndexedDB] attribute IDBCursorWithValueConstructor IDBCursorWithValue;
[Conditional=INDEXED_DATABASE, EnabledAtRuntime=IndexedDB] attribute IDBDatabaseConstructor IDBDatabase;
[Conditional=INDEXED_DATABASE, EnabledAtRuntime=IndexedDB] attribute IDBFactoryConstructor IDBFactory;
[Conditional=INDEXED_DATABASE, EnabledAtRuntime=IndexedDB] attribute IDBIndexConstructor IDBIndex;
[Conditional=INDEXED_DATABASE, EnabledAtRuntime=IndexedDB] attribute IDBKeyRangeConstructor IDBKeyRange;
[Conditional=INDEXED_DATABASE, EnabledAtRuntime=IndexedDB] attribute IDBObjectStoreConstructor IDBObjectStore;
[Conditional=INDEXED_DATABASE, EnabledAtRuntime=IndexedDB] attribute IDBOpenDBRequestConstructor IDBOpenDBRequest;
[Conditional=INDEXED_DATABASE, EnabledAtRuntime=IndexedDB] attribute IDBRequestConstructor IDBRequest;
[Conditional=INDEXED_DATABASE, EnabledAtRuntime=IndexedDB] attribute IDBTransactionConstructor IDBTransaction;
[Conditional=INDEXED_DATABASE, EnabledAtRuntime=IndexedDB] attribute IDBVersionChangeEventConstructor IDBVersionChangeEvent;
Comment 6 Chris Dumez 2016-04-21 20:00:12 PDT 
Created attachment 276999 [details]
Patch
Comment 7 Chris Dumez 2016-04-21 20:05:37 PDT 
Created attachment 277000 [details]
Patch
Comment 8 Brady Eidson 2016-04-21 23:03:05 PDT 
(In reply to comment #5)
> BTW, how about all the other IDB constructors exposed on WorkerGlobalScope?
> 
> I see:
> [Conditional=INDEXED_DATABASE, EnabledAtRuntime=IndexedDB] attribute
> IDBCursorConstructor IDBCursor;
> [Conditional=INDEXED_DATABASE, EnabledAtRuntime=IndexedDB] attribute
> IDBCursorWithValueConstructor IDBCursorWithValue;
> [Conditional=INDEXED_DATABASE, EnabledAtRuntime=IndexedDB] attribute
> IDBDatabaseConstructor IDBDatabase;
> [Conditional=INDEXED_DATABASE, EnabledAtRuntime=IndexedDB] attribute
> IDBFactoryConstructor IDBFactory;
> [Conditional=INDEXED_DATABASE, EnabledAtRuntime=IndexedDB] attribute
> IDBIndexConstructor IDBIndex;
> [Conditional=INDEXED_DATABASE, EnabledAtRuntime=IndexedDB] attribute
> IDBKeyRangeConstructor IDBKeyRange;
> [Conditional=INDEXED_DATABASE, EnabledAtRuntime=IndexedDB] attribute
> IDBObjectStoreConstructor IDBObjectStore;
> [Conditional=INDEXED_DATABASE, EnabledAtRuntime=IndexedDB] attribute
> IDBOpenDBRequestConstructor IDBOpenDBRequest;
> [Conditional=INDEXED_DATABASE, EnabledAtRuntime=IndexedDB] attribute
> IDBRequestConstructor IDBRequest;
> [Conditional=INDEXED_DATABASE, EnabledAtRuntime=IndexedDB] attribute
> IDBTransactionConstructor IDBTransaction;
> [Conditional=INDEXED_DATABASE, EnabledAtRuntime=IndexedDB] attribute
> IDBVersionChangeEventConstructor IDBVersionChangeEvent;

They aren't yet.

I imagine IDBVersionChangeEvent got exposed because it is actually constructible.

I don't know why IDBKeyRange got exposed - Maybe because some methods are static...? /me shrugs
Comment 9 Brady Eidson 2016-04-21 23:05:09 PDT 
Comment on attachment 277000 [details]
Patch

I trust you that this works, but I wonder if we could test it, too?

storage/indexeddb/modern/workers-enable.html and storage/indexeddb/modern/workers-disabled.html would be great tests to add to for this.
Comment 10 Brady Eidson 2016-04-21 23:05:27 PDT 
Also, thanks a lot for doing this!
Comment 11 Chris Dumez 2016-04-22 08:13:31 PDT 
(In reply to comment #9)
> Comment on attachment 277000 [details]
> Patch
> 
> I trust you that this works, but I wonder if we could test it, too?
> 
> storage/indexeddb/modern/workers-enable.html and
> storage/indexeddb/modern/workers-disabled.html would be great tests to add
> to for this.

Sure, I'll take a look at those tests.
Comment 12 Chris Dumez 2016-04-22 10:52:25 PDT 
Created attachment 277076 [details]
Patch
Comment 13 Chris Dumez 2016-04-22 11:01:16 PDT 
Created attachment 277077 [details]
Patch
Comment 14 Darin Adler 2016-04-22 11:30:11 PDT 
Comment on attachment 277077 [details]
Patch

There’s a test failing, but no chance that it’s due to this patch.
Comment 15 WebKit Commit Bot 2016-04-22 12:22:34 PDT 
Comment on attachment 277077 [details]
Patch

Clearing flags on attachment: 277077

Committed r199889: <http://trac.webkit.org/changeset/199889>
Comment 16 WebKit Commit Bot 2016-04-22 12:22:41 PDT 
All reviewed patches have been landed.  Closing bug.