1435113 - [meta] Remove usage of enablePrivilege from Talos

Status: RESOLVED FIXED

(Testing :: Talos, enhancement, P3)

Description

[Andrew McCreight [:mccr8]]

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.

Comment 1

[Andrew McCreight [:mccr8]]

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.

Comment 2

[Bobby Holley (:bholley)]

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.

Comment 3

[Andrew McCreight [:mccr8]]

It feels like the Profiler, TalosContentProfiler and a11y may require Cu.forcePermissiveCOWs(), because they all involve calling many methods on a privileged JS object.

Comment 4

[Andrew McCreight [:mccr8]]

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.

Comment 5

[Bobby Holley (:bholley)]

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

Comment 6

[Alex Gaynor [:Alex_Gaynor]]

Yes! I think I'll have some cycles starting later this week that I'll put into this.

Comment 7

[Alex Gaynor [:Alex_Gaynor]]

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.

Comment 8

[Andrew McCreight [:mccr8]]

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.

Comment 9

[Bobby Holley (:bholley)]

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

Comment 10

[Alex Gaynor [:Alex_Gaynor]]

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.

Comment 11

[Bobby Holley (:bholley)]

Sounds good. Ping me if you need any help with that stuff.

Comment 12

[Masatoshi Kimura [:emk]]

enablePrivilege is gone.