Bug 156883 – Support disabling at runtime IndexedDB constructors exposed to workers

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Brady Eidson]

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.

[Brady Eidson]

Chris, you're pretty darned knowledgeable in this area - What can we do?

[Chris Dumez]

(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.

[Chris Dumez]

(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.

[Chris Dumez]

(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

[Chris Dumez]

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;

[Chris Dumez]

Created attachment 276999 [details]
Patch

[Chris Dumez]

Created attachment 277000 [details]
Patch

[Brady Eidson]

(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

[Brady Eidson]

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.

[Brady Eidson]

Also, thanks a lot for doing this!

[Chris Dumez]

(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.

[Chris Dumez]

Created attachment 277076 [details]
Patch

[Chris Dumez]

Created attachment 277077 [details]
Patch

[Darin Adler]

Comment on attachment 277077 [details]
Patch

There’s a test failing, but no chance that it’s due to this patch.

[WebKit Commit Bot]

Comment on attachment 277077 [details]
Patch

Clearing flags on attachment: 277077

Committed r199889: <http://trac.webkit.org/changeset/199889>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.