The code calls GetConstructorObject() without a pref check. As we port APIs that do have prefs deciding whether they are enabled/disabled, the check should be performed. Kyle, anyway to automate this in Codegen or in dom/workers/?

This is also a little scary because it sounds like right now if you have a class on workers that specifies PrefEnabled, it will work, but just not check the pref. I guess we're protected to some extent by it being hard to implement things on workers. ;)

The easiest way out is, in WorkerScope.cpp, when various APIs are initialized, use the: NotificationBinding::ConstructorEnabled(aCx, global) && !NotificationBinding::GetConstructorObject(aCx, global) form. Also the corresponding API must ensure that it's PrefEnabled or custom function does thread safe operations. Using the Preferences service from the worker thread is not safe, but there is a SyncRunnable class that will allow blocking the worker thread while a custom runnable reads out the value on the main thread.

So this is specifically about interface object checks, not the checks for individual members, right? I would expect the latter _are_ performed on workers, but might not be safe to perform, right?

Hey guys, I'm new in DOM/Workers. I added the NotificationBinding::ConstructorEnabled(aCx, global) && !NotificationBinding::GetConstructorObject(aCx, global) but I didn't know how to use the SyncRunnable. Do you mean by syncRunnable WorkerSyncRunnable? I saw the code of classes that uses WorkerSyncRunnable like dom/workers/URL.cpp but it was really complicated (at least 4 classes were created to be able to use WorkerSyncRunnable) I also saw the syncRunnable in xpcom/threads/SyncRunnable.h but no one seems to use it in dom/workers so I get confused.

Comment on attachment 826287 [details] [diff] [review] performCheck.diff This is not OK, for the reasons described in comment 2. Please feel free to catch me on IRC on Monday if you'd like to talk about this in detail...

(In reply to Boris Zbarsky [:bz] from comment #3) > So this is specifically about interface object checks, not the checks for > individual members, right? I would expect the latter _are_ performed on > workers, but might not be safe to perform, right? I don't think these are done either, since of the top of my head I can't think of WebIDL implemented APIs currently exposed on workers that use per member PrefEnabled.

The notification patch in bug 916893 adds a thread safe version using SyncRunnable, but I'm not a fan of that re-engineering for every API. We should consider patching Preferences to allow a mutex protected opt-in for certain preferences, which would then read out the value in a thread safe manner. Let's talk on IRC or in the DOM meeting.

> I don't think these are done either, Well, just because there is no IDL using them, right? If there were, they would be.

(In reply to Boris Zbarsky [:bz] from comment #9) > > I don't think these are done either, > > Well, just because there is no IDL using them, right? If there were, they > would be. I'm assuming, if there was, it would be using the prefs in a non threadsafe manner too?

If it used prefs, yes.

Is this still a valid bug Boris?

I think this got effectively wontfixed or fixed or something in bug 1017988: if you use [Pref] on something exposed in a worker, codegen (well, webidl parser validation, but the effect is the same) will fail.