Open Bug 1435113 Opened 2 years ago Updated Last year

Remove usage of enablePrivilege from Talos

Categories

(Testing :: Talos, enhancement)

Version 3
enhancement
Not set

Tracking

(Not tracked)

People

(Reporter: mccr8, Unassigned)

References

(Depends on 1 open bug, Blocks 2 open bugs)

Details

(Keywords: meta)

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
Depends on: 1435115
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.
Depends on: 1435434
Depends on: 1435445
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.
Flags: needinfo?(agaynor)
(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.
Flags: needinfo?(agaynor)
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.
Flags: needinfo?(agaynor)
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.
Flags: needinfo?(agaynor)
Sounds good. Ping me if you need any help with that stuff.
You need to log in before you can comment on or make changes to this bug.