Closed Bug 1435113 Opened 5 years ago Closed 3 years ago
[meta] Remove usage of enable
Privilege from Talos
Talos is the last major user of enablePrivilege. I don't think there's an active bug open for fixing that, so I thought I'd file one. This can serve as a meta bug. I don't think there's any fundamental things blocking us from doing the conversion. There's an existing SpecialPowers-ish mechanism in Talos, talos-powers.content.js, that can probably be used. I prototyped a patch to convert quit.js to use this instead, and it seems to work.
I did a brief triage of how enablePrivilege is used in various Talos files: pageloader/chrome/Profiler.js: scripts/Profiler.js: startup_test/tresize/addon/content/Profiler.js: devtools/addon/content/Profiler.js tests/tart/addon/content/Profiler.js These are all copies of the same file. Components.classes["@mozilla.org/tools/profiler;1"].getService(Components.interfaces.nsIProfiler); talos-powers/content/TalosContentProfiler.js Components.utils.import("resource://gre/modules/Services.jsm"); Services.profiler.AddMarker(marker); tests/a11y/a11y.js Components.classes["@mozilla.org/accessibilityService;1"]; pageloader/chrome/tscroll.js tests/gfx/benchmarks/rasterflood_gradient.html tests/gfx/benchmarks/rasterflood_svg.html layout/benchmarks/displaylist_mutate.htm tests/svgx/hixie-00*.xml Services.scriptloader.loadSubScript("chrome://talos-powers-content/content/TalosContentProfiler.js"); scripts/MozillaFileLogger.js: Cc[LF_CID].createInstance(Ci.nsIFile) Cc[FOSTREAM_CID].createInstance(Ci.nsIFileOutputStream) prefs.getCharPref("talos.logfile") where prefs is the prefsService startup_test/tspaint_test.html: Components.classes["@mozilla.org/consoleservice;1"].getService(Components.interfaces.nsIConsoleService); talos/talos/tests/quit.js Various systems-y stuff.
See Also: → 984012
Thanks for pushing on this! Just be aware of bug 715588, where converting Talos to SpecialPowers caused various issues with the measurements. I'm guessing that talos-powers doesn't suffer from the same issues but just wanted to make sure everyone's aware of that attempt five years ago.
It feels like the Profiler, TalosContentProfiler and a11y may require Cu.forcePermissiveCOWs(), because they all involve calling many methods on a privileged JS object.
Alex, can you work on any part of this? I know you had some interest in improving how we deal with this high-privilege testing code. I think the different parts in comment 1 can be worked on independently.
(In reply to Andrew McCreight [:mccr8] from comment #3) > It feels like the Profiler, TalosContentProfiler and a11y may require > Cu.forcePermissiveCOWs(), because they all involve calling many methods on a > privileged JS object. We certainly _could_ do that, but it's not clear to me why we couldn't just use the ExportHelpers machinery to expose the APIs we want.
Yes! I think I'll have some cycles starting later this week that I'll put into this.
Hmm, is there a way to make TalosContentPowers event dispatches to privileged JS able to be synchronous? There are a handful of places where needing to switch to a callback would be disruptive.
Yeah, I noticed the same thing. I'm not sure. Maybe the mechanism Bobby mentioned in comment 5 could be used instead for chrome code that needs to be called synchronously, but I haven't had time to look into it.
(In reply to Alex Gaynor [:Alex_Gaynor] from comment #7) > Hmm, is there a way to make TalosContentPowers event dispatches to > privileged JS able to be synchronous? There are a handful of places where > needing to switch to a callback would be disruptive. We can always have the frame script inject an API onto the window using exportFunction/cloneInto. Alex, are you planning to work on this, and are you blocked on anything? Would be good to get this done.
I'm working towards this as time permits. Figuring out how to handle places that needed synchronous replacements was a blocker, but it seems like exportFunction/cloneInto is the solution there. Hopefully I'll have some time to continue on this in the near future.
Sounds good. Ping me if you need any help with that stuff.
Summary: Remove usage of enablePrivilege from Talos → [meta] Remove usage of enablePrivilege from Talos
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.