Closed
Bug 1363760
Opened 7 years ago
Closed 7 years ago
Install specialpowers as a non-temporary addon in tests
Categories
(Core :: Security: Process Sandboxing, enhancement)
Core
Security: Process Sandboxing
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: Alex_Gaynor, Assigned: Alex_Gaynor)
References
Details
(Whiteboard: sbmc3)
Attachments
(3 files)
Currently these are performed by the content process, which is causing challenges as we move to further restrict which files are readable. The most proximate challenge is that when running |mochitest| with a packaged build, the src directory where the SpecialPowers JS files are has no relationship to where the running binary is. We should figure out where/why these are being loaded by content, and see if we can't move the file open/read into the parent.
Assignee | ||
Comment 1•7 years ago
|
||
Here's a pair of stack traces for trying to read one of these files: libsystem_kernel.dylib`lstat$INODE64+0xa XUL`nsFileChannel::nsFileChannel(nsIURI*)+0x22f XUL`nsFileProtocolHandler::NewChannel2(nsIURI*, nsILoadInfo*, nsIChannel**)+0x5e XUL`mozilla::net::nsIOService::NewChannelFromURIWithProxyFlagsInternal(nsIURI*, nsIURI*, unsigned int, nsILoadInfo*, nsIChannel**)+0x376 XUL`NS_NewChannelInternal(nsIChannel**, nsIURI*, nsILoadInfo*, nsILoadGroup*, nsIInterfaceRequestor*, unsigned int, nsIIOService*)+0xda XUL`nsChromeProtocolHandler::NewChannel2(nsIURI*, nsILoadInfo*, nsIChannel**)+0x311 XUL`mozilla::net::nsIOService::NewChannelFromURIWithProxyFlagsInternal(nsIURI*, nsIURI*, unsigned int, nsILoadInfo*, nsIChannel**)+0x376 XUL`mozilla::net::nsIOService::NewChannelFromURIWithProxyFlags2(nsIURI*, nsIURI*, unsigned int, nsIDOMNode*, nsIPrincipal*, nsIPrincipal*, unsigned int, unsigned int, nsIChannel**)+0x18c XUL`mozilla::net::nsIOService::NewChannelFromURI2(nsIURI*, nsIDOMNode*, nsIPrincipal*, nsIPrincipal*, unsigned int, unsigned int, nsIChannel**)+0x29 XUL`NS_NewChannelInternal(nsIChannel**, nsIURI*, nsINode*, nsIPrincipal*, nsIPrincipal*, unsigned int, unsigned int, nsILoadGroup*, nsIInterfaceRequestor*, unsigned int, nsIIOService*)+0x148 XUL`NS_NewChannel(nsIChannel**, nsIURI*, nsIPrincipal*, unsigned int, unsigned int, nsILoadGroup*, nsIInterfaceRequestor*, unsigned int, nsIIOService*)+0x30 XUL`nsMessageManagerScriptExecutor::TryCacheLoadAndCompileScript(nsAString const&, bool, bool, JS::MutableHandle<JSScript*>)+0x386 XUL`nsMessageManagerScriptExecutor::LoadScriptInternal(nsAString const&, bool)+0xc4 XUL`non-virtual thunk to mozilla::dom::TabChild::RecvLoadRemoteScript(nsString const&, bool const&)+0x3a XUL`mozilla::dom::PBrowserChild::OnMessageReceived(IPC::Message const&)+0x852c XUL`mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&)+0x4da XUL`mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&)+0x159 XUL`mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&)+0x2c5 XUL`mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&)+0x315 XUL`mozilla::ipc::MessageChannel::MessageTask::Run()+0x7c and libsystem_kernel.dylib`stat$INODE64+0xa XUL`nsFileChannel::MakeFileInputStream(nsIFile*, nsCOMPtr<nsIInputStream>&, nsCString&, bool)+0x40 XUL`nsFileChannel::OpenContentStream(bool, nsIInputStream**, nsIChannel**)+0x530 XUL`nsBaseChannel::Open(nsIInputStream**)+0x69 XUL`nsBaseChannel::Open2(nsIInputStream**)+0x4d XUL`nsMessageManagerScriptExecutor::TryCacheLoadAndCompileScript(nsAString const&, bool, bool, JS::MutableHandle<JSScript*>)+0x3e7 XUL`nsMessageManagerScriptExecutor::LoadScriptInternal(nsAString const&, bool)+0xc4 XUL`non-virtual thunk to mozilla::dom::TabChild::RecvLoadRemoteScript(nsString const&, bool const&)+0x3a XUL`mozilla::dom::PBrowserChild::OnMessageReceived(IPC::Message const&)+0x852c XUL`mozilla::dom::PContentChild::OnMessageReceived(IPC::Message const&)+0x4da XUL`mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&)+0x159 XUL`mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&)+0x2c5 XUL`mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&)+0x315 XUL`mozilla::ipc::MessageChannel::MessageTask::Run()+0x7c XUL`mozilla::SchedulerGroup::Runnable::Run()+0xb5 XUL`nsThread::ProcessNextEvent(bool, bool*)+0x5db XUL`NS_ProcessNextEvent(nsIThread*, bool)+0x50 XUL`mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*)+0x1a3 XUL`MessageLoop::Run()+0x4a XUL`XRE_RunAppShell()+0x105
Comment 2•7 years ago
|
||
Something that might work here: have the script in the parent process create a File object[1], then register a blob URL[2] and send that to the content process. [1] https://developer.mozilla.org/en-US/docs/Extensions/Using_the_DOM_File_API_in_chrome_code [2] https://developer.mozilla.org/en-US/docs/Web/API/URL/createObjectURL — as of bug 1279186 this should broadcast a PBlob to all processes
Assignee | ||
Comment 3•7 years ago
|
||
If I've read the code correctly, the offender is one of: - https://dxr.mozilla.org/mozilla-central/source/testing/specialpowers/content/specialpowers.js#215-217 - https://dxr.mozilla.org/mozilla-central/source/testing/specialpowers/content/SpecialPowersObserver.jsm#91-93 Someone on IRC also suggested that data URIs may be a solution.
Assignee | ||
Comment 4•7 years ago
|
||
ni? Andy for the addons perspective and Joel for the mochitest perspective. Hopefully this message has enough context to explain why. After some more research and discussion with the rest of the sandboxing team, I'm going to attempt to summarize where we're at: When running mochitest (and reftest, it looks like), `specialpowers` is installed as a temporary addon. Then in the parent process we tell the message manager to load a frame script in our tab, with a `chrome://specialpowers/.../MozillaLogger.js` URL (and a few others). Digging into how this is implemented: - When installing a temporary addon, it appears that the code doesn't copy the addon contents anywhere, instead it just wires up some metadata to the existing disk location - Loading a chrome:// URL basically rewrites it as a file:// URL and tries to load it in the content process. As we ratchet up read restrictions for the sandbox, this will break mochitest with packaged builds, but I believe it'll break some (all?) uses of install temporary legacy addons. This leaves us with a few questions: 1) Are the specialpowers and legacy addons issues going to be the same, or have I misread one or both of how they're setup? 2) Is it necessary to continue to support temporary installation of legacy addons, give they're going away entirely in 6 months 3) What's the right way to fix this? For web-extensions we have bug 1334550 to proxy all their loads to the parent process. Is the fix here to install specialpowers to an existing whitelisted directory? To implement something specific to it? To package it as a web-extension? (No idea if this makes sense :-)). Something else? Appreciate any insight into how we can make everybody happy here!
Flags: needinfo?(jmaher)
Flags: needinfo?(amckay)
Updated•7 years ago
|
Flags: needinfo?(amckay) → needinfo?(aswan)
Comment 5•7 years ago
|
||
(In reply to Alex Gaynor [:Alex_Gaynor] from comment #4) > Digging into how this is implemented: > > - When installing a temporary addon, it appears that the code doesn't copy > the addon contents anywhere, instead it just wires up some metadata to the > existing disk location > - Loading a chrome:// URL basically rewrites it as a file:// URL and tries > to load it in the content process. I don't think this is specific to temporary installation, permanent installation works the same way. Are you sure it gets rewritten to file: ? I would expect it to be jar: That doesn't really matter though, jar has the same problem. > As we ratchet up read restrictions for the sandbox, this will break > mochitest with packaged builds, but I believe it'll break some (all?) uses > of install temporary legacy addons. Right, to be precise this will break any legacy extensions that register chrome: resources. > This leaves us with a few questions: > > 1) Are the specialpowers and legacy addons issues going to be the same, or > have I misread one or both of how they're setup? Not sure I understand the question, are you asking if these addons are going to keep working the way they do right now? I think your understanding of how they currently work is correct. And I don't know of any plans to change how they work (other than this bug of course!) > 2) Is it necessary to continue to support temporary installation of legacy > addons, give they're going away entirely in 6 months Legacy extensions signed by AMO won't be usable on release builds but we still use them heavily in automation (I don't have an exhaustive list handy but specialpowers, mochitest, reftest, talos, etc. all rely on legacy extensions that simply can't be written as webextensions) and we'll have some exceptions for things like shield and test pilot. > 3) What's the right way to fix this? I'm not sure. How are other chrome: resources that end up mapping to files on disk handled in the content process?
Flags: needinfo?(aswan)
Comment 6•7 years ago
|
||
(In reply to Andrew Swan [:aswan] from comment #5) > (In reply to Alex Gaynor [:Alex_Gaynor] from comment #4) > > Digging into how this is implemented: > > > > - When installing a temporary addon, it appears that the code doesn't copy > > the addon contents anywhere, instead it just wires up some metadata to the > > existing disk location > > - Loading a chrome:// URL basically rewrites it as a file:// URL and tries > > to load it in the content process. > > I don't think this is specific to temporary installation, permanent > installation works the same way. > Are you sure it gets rewritten to file: ? I would expect it to be jar: > That doesn't really matter though, jar has the same problem. The difference for us is that a temporary installation uses the add-on files (or JAR file) where they are, but a permanent installation of an add-on copies the files to $PROFILE/extensions. What we're trying to rollout is a sandbox that prevents the content process from reading the home directory (where sensitive data is stored) and other locations we know content doesn't need to read. We will have sandbox exception rules in place to allow the content process to access files in $PROFILE/extensions directly (to support legacy add-ons.) Hence add-on files installed in $PROFILE/extensions can be read by content, but a temporary add-on, which can be installed from anywhere on the filesystem (most likely from the home directory), would run into file access failures. > > As we ratchet up read restrictions for the sandbox, this will break > > mochitest with packaged builds, but I believe it'll break some (all?) uses > > of install temporary legacy addons. > > Right, to be precise this will break any legacy extensions that register > chrome: resources. Not as long as we allow content to read from $PROFILE/extensions. Legacy add-ons would be affected if they rely on loading files from the content process outside of $PROFILE/extensions. That's covered on https://blog.mozilla.org/addons/2017/02/16/the-road-to-firefox-57-compatibility-milestones/ We are not expecting that to be a large number of add-ons, but haven't got to testing this heavily yet. We want to address mochitests on packaged builds too, but those use different directories we might be able to exclude for the time being. And, to reiterate, this refers to legacy add-ons. For WebExtensions, we are working on remoting the moz-extension protocol which allows temporary and permanent installs of WebExtensions to work without special sandbox rules. That's bug 1334550.
Comment 7•7 years ago
|
||
(In reply to Haik Aftandilian [:haik] from comment #6) > (In reply to Andrew Swan [:aswan] from comment #5) > > (In reply to Alex Gaynor [:Alex_Gaynor] from comment #4) > > > Digging into how this is implemented: > > > > > > - When installing a temporary addon, it appears that the code doesn't copy > > > the addon contents anywhere, instead it just wires up some metadata to the > > > existing disk location > > > - Loading a chrome:// URL basically rewrites it as a file:// URL and tries > > > to load it in the content process. > > > > I don't think this is specific to temporary installation, permanent > > installation works the same way. > > Are you sure it gets rewritten to file: ? I would expect it to be jar: > > That doesn't really matter though, jar has the same problem. > > The difference for us is that a temporary installation uses the add-on files > (or JAR file) where they are, but a permanent installation of an add-on > copies the files to $PROFILE/extensions. Ah, I had assumed specialpowers got built into an xpi and of course the exception for {profile}/extensions does make a big difference. If that exception is going to live on for legacy extensions, then I think we're actually in good shape here -- when bug 1356826 lands we can switch from temporary to permanent installation and everything should work. I suspect there are a bunch of mochitests that use specialpowers to get at nsIFile/FileUtils/whatever and hit the filesystem which will break, but that's a separate issue.
Comment 8•7 years ago
|
||
1) Are the specialpowers and legacy addons issues going to be the same, or have I misread one or both of how they're setup? you have read this correctly- that is how we setup addons 2) Is it necessary to continue to support temporary installation of legacy addons, give they're going away entirely in 6 months my understanding is there is no work being done to change in-tree test related addons (we have 30+). They are all legacy. For Talos we sign our addons, for reftest/mochitest we install temporarily via marionette. I have asked dozens of times if we need to update our test addons and I am always told it will be fine (even just a couple weeks ago). My understanding is that using the pref "security.turn_off_all_security_so_that_viruses_can_take_over_this_computer" we will be fine. Should we be planning on converting the test addons in all cases to web extensions before Firefox 57 ships? If so we will need additional help to do this. 3) What's the right way to fix this? this I don't know.
Flags: needinfo?(jmaher)
Assignee | ||
Comment 9•7 years ago
|
||
Echoing back what I read: - We'll continue having legacy addons for testing purposes well past 57. (As an aside, I hadn't seen turn_off_all_security_so_that_viruses_can_take_over_this_computer before, and when you mentioned it I thought it was a joke at first :-)) - Once bug 1356826 lands we can change installing specialpowers to just be a permanent addon, copying it to $PROFILE/extensions, and this will fix the current pain point. This is already on the addons team's radar, so yay! :-) - We do have a ton of other testing addons, so there may be others that'll need similar treatment. Thanks for the insights Joel and Andrew, very much appreciated. It seems like we're in a good place!
Assignee | ||
Updated•7 years ago
|
Updated•7 years ago
|
Whiteboard: sbmc3
Assignee | ||
Updated•7 years ago
|
Summary: Move file reads of special-powers javascript to parent process → Install specialpowers as a non-temporary addon in tests
Assignee | ||
Comment 10•7 years ago
|
||
Changed the bug title to accurately reflect what the real solution will be.
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8867757 -
Flags: review?(aswan)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → agaynor
Comment 12•7 years ago
|
||
Is there anything we have to do here to make sure test addons are cleanly uninstalled (after a crash)? If that even matters given that we use temp profiles.
Comment hidden (mozreview-request) |
Comment 14•7 years ago
|
||
mozreview-review |
Comment on attachment 8867757 [details] Bug 1363760 - Part 3 - In tests, install SpecialPowers and mochijar as non-temporary addons https://reviewboard.mozilla.org/r/139302/#review142660 ::: testing/mochitest/runtests.py:2137 (Diff revision 2) > > # install specialpowers and mochikit as temporary addons > addons = Addons(self.marionette) > > if mozinfo.info.get('toolkit') != 'gonk': > - addons.install(os.path.join(here, 'extensions', 'specialpowers'), temp=True) > + addons.install(create_zip( I'm not up on all the details here but I see specialpowers already packaged as an xpi in OBJDIR/dist/xpi-stage/specialpowers@mozilla.org.xpi Can you just copy that over? And do the same for mochijar?
Assignee | ||
Comment 15•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8867757 [details] Bug 1363760 - Part 3 - In tests, install SpecialPowers and mochijar as non-temporary addons https://reviewboard.mozilla.org/r/139302/#review142660 > I'm not up on all the details here but I see specialpowers already packaged as an xpi in OBJDIR/dist/xpi-stage/specialpowers@mozilla.org.xpi > Can you just copy that over? And do the same for mochijar? Will that work for packaged builds? Do we have an `$OBJDIR` with things in it on the machine running tests, or just the source?
Comment 16•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8867757 [details] Bug 1363760 - Part 3 - In tests, install SpecialPowers and mochijar as non-temporary addons https://reviewboard.mozilla.org/r/139302/#review142660 > Will that work for packaged builds? Do we have an `$OBJDIR` with things in it on the machine running tests, or just the source? If `here` in that code is not an OBJDIR, what is it?
Assignee | ||
Comment 17•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8867757 [details] Bug 1363760 - Part 3 - In tests, install SpecialPowers and mochijar as non-temporary addons https://reviewboard.mozilla.org/r/139302/#review142660 > If `here` in that code is not an OBJDIR, what is it? `here = os.path.abspath(os.path.dirname(__file__))`
Assignee | ||
Comment 18•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8867757 [details] Bug 1363760 - Part 3 - In tests, install SpecialPowers and mochijar as non-temporary addons https://reviewboard.mozilla.org/r/139302/#review142660 > `here = os.path.abspath(os.path.dirname(__file__))` Mmmm, no you're right, `here` is inside `$OBJDIR` - I'll try the change you've proposed.
Assignee | ||
Comment 19•7 years ago
|
||
While using the pre-built XPI appears to work well enough (at least locally), I'm working through some test failure: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c3e392cb0a0e&group_state=expanded From debugging, it looks like the problem is that in `uninstallAddon`, the `yield addonListMutation` never fires -- nothing in the `getAddonListWithAddon` or `waitForMutation` looks like it should suffer from one other addon happening to be installed, but I'm still in the process of debugging.
Assignee | ||
Comment 20•7 years ago
|
||
Debugging notes: Using a non-temporary specialpowers with a temporary mochijar does not exhibit the same bug. Upon further investigation the problem appears to be that the DOM queries that are performed to test that things were properly uninstalled do not handle going from 1 => 0 temporary addons, which lots of these tests do once mochijar and specialpowers are not temporary. Specific problems: (1) `waitForMutation(node, { childList: true })` does not consider a mutation to have occurred if `node` is removed from the DOM (teehee). This hits us at https://dxr.mozilla.org/mozilla-central/source/devtools/client/aboutdebugging/test/head.js?q=function%2A+uninstallAddon&redirect_type=single#215-236 (2) Further, given a `node`, if that node is removed from the DOM, `node.querySelectorAll("...")` will happily return some of it's children nodes even though none of it is attached to `document` anymore, whereas if a child of `node` is removed, `node.querySelectorAll("...")` obviously doesn't return in it. This hits us at https://dxr.mozilla.org/mozilla-central/source/devtools/client/aboutdebugging/test/head.js?q=function%2A+uninstallAddon&redirect_type=single#237-241 The root of these is that when we've got >1 temporary addon installed and we remove it, we delete an <li> from the <ul> we're observing. When we're removing the final temporary addon, we delete the whole <ul> (and stick a `<p>Nothing yet.</p>` in its place). There may be other issues, I've only debugged one particular failure, but these seem very likely to be a pattern which recurs, so it wouldn't surprise me if these accounted for all the failures. It's been a while since I DOM'd in earnest, so feedback on this would be appreciated. Issue (1) can be resolved be using `waitForMutation(addonList.parentNode, { childList: true, subtree: true })` which should work no matter how many addons are installed. Issue (2) I'm less certain about, what's the right way to express "children which match this query selector, but only if I'm still attached to document".
Comment hidden (mozreview-request) |
Assignee | ||
Comment 22•7 years ago
|
||
> XPI file /builds/slave/test/build/tests/mochitest/../../../dist/xpi-stage/specialpowers@mozilla.org.xpi does not exist
Seems to indicate that the XPIs aren't around in automation, once we get to running tests.
Comment 23•7 years ago
|
||
mozreview-review |
Comment on attachment 8867757 [details] Bug 1363760 - Part 3 - In tests, install SpecialPowers and mochijar as non-temporary addons https://reviewboard.mozilla.org/r/139302/#review143590 If you want sign-off on the concept of using regular (non-temporary) installation, r=me but a test/build peer should sign off on the actual implementation.
Attachment #8867757 -
Flags: review?(aswan)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 26•7 years ago
|
||
I'm the one who started installing these temporarily in the first place back when addon signing was rolling out, so would be happy to do reviews on the mochitest side of things.
Assignee | ||
Comment 27•7 years ago
|
||
:ahal, awesome, thanks! Just as soon as I've got the try bots happy I'll r? you. (Several tests for about:addons make assumptions about the shape of the DOM)
Assignee | ||
Updated•7 years ago
|
Attachment #8867757 -
Flags: review?(aswan) → review?(ahalberstadt)
Comment 28•7 years ago
|
||
mozreview-review |
Comment on attachment 8867757 [details] Bug 1363760 - Part 3 - In tests, install SpecialPowers and mochijar as non-temporary addons https://reviewboard.mozilla.org/r/139302/#review147636 Thanks for looking into this! I don't really understand the /devtools and /toolkit changes, it might be a good idea to split those into a separate commit and get someone more relevant to review them. As far as the mochitest harness, looks good, nice and simple! I'll change to an r+ with the issue fixed. ::: testing/mochitest/runtests.py:779 (Diff revision 5) > +def _add_to_zip(z, path, zippath): > + """ > + Adds a `path` on disk to the zip `z` at location `zippath` within in. > + """ > + if os.path.isfile(path): > + z.write(path, zippath, zipfile.ZIP_DEFLATED) > + elif os.path.isdir(path): > + if zippath: > + z.write(path, zippath) > + for name in os.listdir(path): > + _add_to_zip(z, os.path.join(path, name), os.path.join(zippath, name)) > + > + > +def create_zip(path): > + """ > + Takes a `path` on disk and creates a zipfile with its contents. Returns a > + path to the location of the temporary zip file. > + """ > + with tempfile.NamedTemporaryFile(delete=False) as f: > + with zipfile.ZipFile(f, "w") as z: > + _add_to_zip(z, path, "") > + return f.name There's a higher level utility for this in the stdlib, under `shutil.make_archive`. I believe you can use it like: shutil.make_archive(<filename>, 'zip', <directory>)
Attachment #8867757 -
Flags: review?(ahalberstadt) → review-
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8872631 -
Flags: review?(jdescottes)
Attachment #8872632 -
Flags: review?(amckay)
Attachment #8867757 -
Flags: review?(ahalberstadt)
Assignee | ||
Updated•7 years ago
|
Attachment #8867757 -
Flags: review?(ahalberstadt)
Updated•7 years ago
|
Attachment #8872632 -
Flags: review?(amckay) → review?(aswan)
Comment 32•7 years ago
|
||
mozreview-review |
Comment on attachment 8872632 [details] Bug 1363760 - Part 2 - Corrected the permission that is reset at the end of a test https://reviewboard.mozilla.org/r/144168/#review147902 This is an old test that was written before `SpecialPowers.pushPrefEnv()` existed, can you just change the call to `Services.prefs.setBoolPref()` to use that instead, then there's no need to clean up.
Attachment #8872632 -
Flags: review?(aswan)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 35•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8872632 [details] Bug 1363760 - Part 2 - Corrected the permission that is reset at the end of a test https://reviewboard.mozilla.org/r/144168/#review147902 Done.
Assignee | ||
Updated•7 years ago
|
Attachment #8872632 -
Flags: review?(amckay)
Attachment #8867757 -
Flags: review?(ahalberstadt)
Assignee | ||
Updated•7 years ago
|
Attachment #8872632 -
Flags: review?(aswan)
Assignee | ||
Updated•7 years ago
|
Attachment #8867757 -
Flags: review?(ahalberstadt)
Assignee | ||
Comment 36•7 years ago
|
||
Not sure what I'm doing wrong, but bz and mozreview seem to be enjoying getting out of sync. I've attempted to make sure they're both in agreement, since I have no idea what actual notifications people get otherwise.
Comment 37•7 years ago
|
||
mozreview-review |
Comment on attachment 8872632 [details] Bug 1363760 - Part 2 - Corrected the permission that is reset at the end of a test https://reviewboard.mozilla.org/r/144168/#review147926 thanks!
Attachment #8872632 -
Flags: review?(aswan) → review+
Comment 38•7 years ago
|
||
mozreview-review |
Comment on attachment 8867757 [details] Bug 1363760 - Part 3 - In tests, install SpecialPowers and mochijar as non-temporary addons https://reviewboard.mozilla.org/r/139302/#review147984 Thanks, lgtm!
Attachment #8867757 -
Flags: review?(ahalberstadt) → review+
Comment 39•7 years ago
|
||
mozreview-review |
Comment on attachment 8872631 [details] Bug 1363760 - Part 1 - Allow the about:debugging tests to pass if there are no temporary addons installed https://reviewboard.mozilla.org/r/144166/#review148704 LGTM, thanks!
Attachment #8872631 -
Flags: review?(jdescottes) → review+
Assignee | ||
Updated•7 years ago
|
Keywords: checkin-needed
Comment 40•7 years ago
|
||
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/65fcedb6c1f8 Part 1 - Allow the about:debugging tests to pass if there are no temporary addons installed r=jdescottes https://hg.mozilla.org/integration/autoland/rev/0205cdd17a67 Part 2 - Corrected the permission that is reset at the end of a test r=aswan https://hg.mozilla.org/integration/autoland/rev/71e0ff5a6c76 Part 3 - In tests, install SpecialPowers and mochijar as non-temporary addons r=ahal
Keywords: checkin-needed
Comment 41•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/65fcedb6c1f8 https://hg.mozilla.org/mozilla-central/rev/0205cdd17a67 https://hg.mozilla.org/mozilla-central/rev/71e0ff5a6c76
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in
before you can comment on or make changes to this bug.
Description
•