Rewrite SpecialPowers as a restartless addon

RESOLVED FIXED in Firefox 45

Status

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jgriffin, Assigned: ahal)

Tracking

unspecified
mozilla45
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox45 fixed)

Details

Attachments

(1 attachment)

Once addon signing is enforced, all test extensions will either need to be signed, or to be installed at runtime using a mechanism which will only work with restartless addons - see bug 1209338.

We don't want to sign SpecialPowers because of its potential for abuse, so we should turn it into a restartless addon.
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
Looks like converting it into a restartless should be fairly easy, but I keep getting stuck on minor details.

The issue I'm on now is that the SpecialPowersObserver.js XPCOM component needs to get registered programatically using nsIComponentsRegistrar. That is easy, but I'm having trouble importing it from the chrome://specialpowers/content/ directory in the extension's bootstrap.js. I guess I could just define the XPCOM component directly in bootstrap.js in the worst case.

Anyway, once I figure out how to get this working, we'll need to change the build system packaging to generate it as a restartless addon. And we'll need to figure out some way to install it on the fly (probably using marionette).

One thing to consider is that if we're going to require marionette anyway, it might be better to just use marionette to inject the scripts directly, and forgo using any addons at all. The reason I say this is because if XUL/xpcom based extensions are deprecated in favor of webextensions, we'll need to do this anyway.
The loading issue was PEBKAC.

So I think I have a restartless version of specialpowers. By that I mean, I can zip the directory up with a .xpi extension, install it in my firefox, and access the various scripts in the browser console. I still haven't tried actually injecting it into content yet, but presumably the normal marionette/test suite setup that does this should work as usual.

I still have to figure out the build packaging and installation pieces.
Ah, my patch won't quite work just yet. The XPCOM component listens for 'profile-after-change' to do the initialization and framescript loading. Because it is being installed after the fact, it will never receive this event.

We'll have to call the initialization manually, similar to how the b2g mochitests do in b2g_start_script.js. This will require a minor refactor.
Sweet, figured out the packaging issues and verified it's accessible from content. It's now set up so that simply installing the addon does all the necessary frame script injecting automatically.

Now, we just need to install the addon in all the test suites after Firefox launch but before SUITE-START. B2G/android could be trickier.
Comment on attachment 8684415 [details]
MozReview Request: Bug 1219442 - Re-write specialpowers as a restartless addon, r=jmaher

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24565/diff/1-2/
Depends on: 1223171
I have bug 1223171 more or less finished, just pending review. Can now install specialpowers via marionette.

The last (and probably the hardest) piece is going to be updating all the test harnesses to use this new marionette + restartless specialpowers combo. This could be a bit trickier for suites that don't yet use marionette for anything. I know this is going to include a small refactor of the mochitest JS harness.
(In reply to Andrew Halberstadt [:ahal] from comment #5)
> Now, we just need to install the addon in all the test suites after Firefox
> launch but before SUITE-START. B2G/android could be trickier.

I take this back, desktop is trickier. This is because we use xul overlays to automatically launch MochiKit/reftest.js when the browser starts. I'll need to switch desktop over to the b2g method of executing a script to kick things off so we have a chance to install specialpowers first. We'll need to remove the xul overlays anyway (unless we start signing the mochitest/reftest extensions), so that's a side bonus.
Depends on: 1224294
Filed bug 1224294 to track mochitest, and noticed that someone already made the reftest extension restartless.. sweet!
Hit another little snag here. This patch breaks all the addon update tests, as special-powers@mozilla.org is no longer an "application scope" (i.e owned by firefox). Took me awhile to debug, but discovered those tests are disabling all user installed addons:
https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/tests/chrome/utils.js#1305

Adding an exception for "special-powers@mozilla.org" there lets the tests advance, but results in more failures later on. Hopefully I can get these working with minimal effort, but may need some feedback from addon folks.
One way to workaround the above issue is to change 'maxVersion' in the specialpower's install.rdf to *. Assuming this isn't some sort of security concern, I think this is good enough.
Since sp won't be signed, I think this should be totally acceptable.
Comment on attachment 8684415 [details]
MozReview Request: Bug 1219442 - Re-write specialpowers as a restartless addon, r=jmaher

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24565/diff/2-3/
Latest incarnation of the patch fixes a b2g error and should be ready for review + landing.

Any volunteers to review before I choose a victim?
Comment on attachment 8684415 [details]
MozReview Request: Bug 1219442 - Re-write specialpowers as a restartless addon, r=jmaher

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24565/diff/3-4/
Attachment #8684415 - Attachment description: MozReview Request: Bug 1219442 - Rewrite specialpowers as a restartless addon → MozReview Request: Bug 1219442 - Re-write specialpowers as a restartless addon, r=jmaher
Attachment #8684415 - Flags: review?(jmaher)
Comment on attachment 8684415 [details]
MozReview Request: Bug 1219442 - Re-write specialpowers as a restartless addon, r=jmaher

https://reviewboard.mozilla.org/r/24565/#review22979

I assume there are valid reasons for these questions- r=me upon answering.

::: testing/specialpowers/content/SpecialPowersObserver.jsm
(Diff revision 4)
> -  SpecialPowersObserver.prototype._xpcom_categories = [{category: "profile-after-change", service: true }];

why are we removing profile-after-change and xpcom-shutdown?

::: testing/specialpowers/moz.build
(Diff revision 4)
> -    'components/SpecialPowersObserver.js',

why is this removed, we still have a specialpowersobserver.js.
Attachment #8684415 - Flags: review?(jmaher) → review+
(In reply to Joel Maher (:jmaher) from comment #17)
> why are we removing profile-after-change and xpcom-shutdown?

profile-after-change and xpcom-shutdown were the events used by the old addon to initialize and uninitialize everything. These two events have been replaced by 'startup()' and 'shutdown()' in the bootstrap.js script which automatically get called at the proper time.

> why is this removed, we still have a specialpowersobserver.js.

EXTRA_COMPONENTS simply copies the component to $OBJDIR/dist/bin/components. I think this is designed for components that are supposed to be shipped with firefox, and it was a mistake to include SpecialPowersObserver.js here. The SpecialPowersObserver component can't work without the rest of the extension, so I don't think it makes any sense to ship it standalone. I could be wrong, but removing it doesn't seem to have any negative consequences on try.
https://hg.mozilla.org/mozilla-central/rev/cc82d9b8f949
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in before you can comment on or make changes to this bug.