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:

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

Brady Eidson

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

Attachments
Patch (9.85 KB, patch) 2016-04-21 20:00 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (9.96 KB, patch) 2016-04-21 20:05 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (15.73 KB, patch) 2016-04-22 10:52 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Patch (16.94 KB, patch) 2016-04-22 11:01 PDT, Chris Dumez	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Brady Eidson

Comment 1 2016-04-21 17:17:02 PDT

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

Chris Dumez

Comment 2 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.

Chris Dumez

Comment 3 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.

Chris Dumez

Comment 4 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

Chris Dumez

Comment 5 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;

Chris Dumez

Comment 6 2016-04-21 20:00:12 PDT

Created attachment 276999 [details] Patch

Chris Dumez

Comment 7 2016-04-21 20:05:37 PDT

Created attachment 277000 [details] Patch

Brady Eidson

Comment 8 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

Brady Eidson

Comment 9 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.

Brady Eidson

Comment 10 2016-04-21 23:05:27 PDT

Also, thanks a lot for doing this!

Chris Dumez

Comment 11 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.

Chris Dumez

Comment 12 2016-04-22 10:52:25 PDT

Created attachment 277076 [details] Patch

Chris Dumez

Comment 13 2016-04-22 11:01:16 PDT

Created attachment 277077 [details] Patch

Darin Adler

Comment 14 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.

WebKit Commit Bot

Comment 15 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>

WebKit Commit Bot

Comment 16 2016-04-22 12:22:41 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.