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
Brady Eidson
2016-04-21 17:16:28 PDT
Chris, you're pretty darned knowledgeable in this area - What can we do? (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. (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. (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 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; Created attachment 276999 [details]
Patch
Created attachment 277000 [details]
Patch
(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 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.
Also, thanks a lot for doing this! (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. Created attachment 277076 [details]
Patch
Created attachment 277077 [details]
Patch
Comment on attachment 277077 [details]
Patch
There’s a test failing, but no chance that it’s due to this patch.
Comment on attachment 277077 [details] Patch Clearing flags on attachment: 277077 Committed r199889: <http://trac.webkit.org/changeset/199889> All reviewed patches have been landed. Closing bug. |