Closed
Bug 1219442
Opened 9 years ago
Closed 9 years ago
Rewrite SpecialPowers as a restartless addon
Categories
(Testing :: General, defect)
Testing
General
Tracking
(firefox45 fixed)
RESOLVED
FIXED
mozilla45
Tracking | Status | |
---|---|---|
firefox45 | --- | fixed |
People
(Reporter: jgriffin, Assigned: ahal)
References
Details
Attachments
(1 file)
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 | ||
Updated•9 years ago
|
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•9 years ago
|
||
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.
Assignee | ||
Comment 2•9 years ago
|
||
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.
Assignee | ||
Comment 3•9 years ago
|
||
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.
Assignee | ||
Comment 4•9 years ago
|
||
Bug 1219442 - Rewrite specialpowers as a restartless addon
Assignee | ||
Comment 5•9 years ago
|
||
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.
Assignee | ||
Comment 6•9 years ago
|
||
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/
Assignee | ||
Comment 7•9 years ago
|
||
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.
Assignee | ||
Comment 8•9 years ago
|
||
(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.
Assignee | ||
Comment 9•9 years ago
|
||
Filed bug 1224294 to track mochitest, and noticed that someone already made the reftest extension restartless.. sweet!
Assignee | ||
Comment 10•9 years ago
|
||
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.
Assignee | ||
Comment 11•9 years ago
|
||
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.
Reporter | ||
Comment 12•9 years ago
|
||
Since sp won't be signed, I think this should be totally acceptable.
Assignee | ||
Comment 13•9 years ago
|
||
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/
Assignee | ||
Comment 14•9 years ago
|
||
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?
Assignee | ||
Comment 15•9 years ago
|
||
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)
Assignee | ||
Comment 16•9 years ago
|
||
Here's a sequence of try runs which show green on everything between them:
https://treeherder.mozilla.org/#/jobs?repo=try&author=ahalberstadt@mozilla.com&fromchange=8a9e3f684c25&tochange=f5f4fb4156f0
Comment 17•9 years ago
|
||
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+
Assignee | ||
Comment 18•9 years ago
|
||
(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.
Comment 19•9 years ago
|
||
Comment 20•9 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox45:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in
before you can comment on or make changes to this bug.
Description
•