Closed Bug 1415379 Opened 2 years ago Closed 2 years ago

Expand mochitest content process sandbox to include content-task.js

Categories

(Testing :: Mochitest, defect, P1, major)

Unspecified
Windows
defect

Tracking

(firefox59 fixed)

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: MattN, Assigned: MattN)

References

(Blocks 2 open bugs)

Details

Attachments

(1 file)

My mochitest profile only has
> obj-fx-dbg\_tests\testing\mochitest\
on the security.sandbox.content.read_path_whitelist whitelist but resource://testing-common/content-task.js resolves to
> obj-fx-dbg\_tests\modules\content-task.js
which is used by many mochitests. This is causing an NS_ERROR_FILE_ACCESS_DENIED[1] when using ContentTask.jsm which is made for use in mochitest-bc. This is causing many test failure that are getting filed as intermittents.

[1] https://dxr.mozilla.org/mozilla-central/rev/2535bad09d720e71a982f3f70dd6925f66ab8ec7/dom/base/nsFrameMessageManager.cpp#1628
There is probably a cleaner way to get the _tests directory. Is there a reason why the sandbox whitelist was more strict in the first place?

I would also like to understand why this wasn't a permafailure? Maybe this patch is paving over the real problem? Or maybe this patch only fixes the perma-problem I have locally and isn't related to the intermittent test failures in automation?

Try push confirming it fixes a dependent intermittent failure: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3d6f629acb63cdbbb2fa0b7f062d3956b2a33bfd
Comment on attachment 8926261 [details]
Bug 1415379 - Add the entire _tests directory to the sandbox whitelist.

https://reviewboard.mozilla.org/r/197522/#review203006

Lgtm! Please also wait for Alex_Gaynor's review though.

::: testing/mochitest/runtests.py:1836
(Diff revision 1)
> +        _tests_dir = os.path.dirname(os.path.dirname(SCRIPT_DIR))
> +        sandbox_whitelist_paths = [_tests_dir] + options.sandboxReadWhitelist

nit: don't underscore prefix `_tests_dir`, that's typically only used to prevent instance variables from becoming public facing APIs.
Attachment #8926261 - Flags: review?(ahalberstadt) → review+
Comment on attachment 8926261 [details]
Bug 1415379 - Add the entire _tests directory to the sandbox whitelist.

https://reviewboard.mozilla.org/r/197522/#review203344

Substance of the patch looks ok (modulo :ahal's nit), but do you understand why you hit this issue and no one else seems to? I'd like to understand what's different about your env that leads to this before it lands.
Attachment #8926261 - Flags: review?(agaynor) → review+
(In reply to Alex Gaynor [:Alex_Gaynor] from comment #4)
> Substance of the patch looks ok (modulo :ahal's nit), but do you understand
> why you hit this issue and no one else seems to? I'd like to understand
> what's different about your env that leads to this before it lands.

Did anyone else try to run the affected tests locally on Windows? I'm thinking that maybe this issue doesn't affect automation for some reason but it probably affects a local builds.
I don't know, does this affect all mochitests or just a few? I definitely run plenty of mochitests locally on macOS, I assume windows devs do likewise.
It affects any that use ContentTask.jsm. :jaws said this test works fine on his machine so I'm wondering if it's because this was on Windows 8?
can we expect this will land today/tomorrow?  we have a very high failure rate on bug 1411118 and I want to resolve it today if possible.
I'm still awfully confused -- I don't understand why this wouldn't be a permafailure -- but the patch itself looks fine.
(In reply to Joel Maher ( :jmaher) (UTC-5) from comment #8)
> can we expect this will land today/tomorrow?  we have a very high failure
> rate on bug 1411118 and I want to resolve it today if possible.

Joel, it's no longer clear to me if fixing this bug will fix bug 1411118 but it will unblock me from locally debugging the issue. I have be unable to get local builds working on any of my other Windows environments to try and debug this more.

Since it sounds like Alex agrees that the whitelist needs to be expanded to include the "modules" directory, I think I'll land this as it doesn't seem like it will hurt anything. We can file a separate bug to investigate what is going on.
Pushed by mozilla@noorenberghe.ca:
https://hg.mozilla.org/integration/autoland/rev/d953d54b2a20
Add the entire _tests directory to the sandbox whitelist. r=ahal,Alex_Gaynor
I was able to reproduce this issue on my Windows 10 machine using MozillaBuild 3.1 and Visual Studio 2017 in a debug build with nothing else in my mozconfig so this wasn't isolated to one computer.
https://hg.mozilla.org/mozilla-central/rev/d953d54b2a20
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
You need to log in before you can comment on or make changes to this bug.