Closed Bug 448676 Opened 12 years ago Closed 10 years ago

Enable UniversalXPConnect in tinderbox crashtest and/or reftest profiles

Categories

(Testing :: Reftest, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: graememcc, Unassigned)

References

Details

Attachments

(1 file, 1 obsolete file)

There are several layout reftests (bug 263683, bug 407959, bug 401361) currently disabled that need to request UniversalXPConnect in order to work. 

When creating a profile for the test run, the unit test boxes should have the capability preferences set to automatically accept these requests. Without this, any time a reftest is written asking for the privileges, the security manager dialog is displayed, causing multiple non-obvious reftest failures, and potentially long cycle times.
Attached patch Patch (obsolete) — Splinter Review
Unsure who would review this.
Hmm... which tests would this profile affect?
The tests need to be runnable by developers; changing tinderbox alone is both insufficient and harmful.
Blocks: 448987
No longer blocks: 373371
Blocks: 449653
Ping?  Any progress being made here?
Can this be handled in ReadManifest (layout/tools/reftest/reftest.js) so that UniversalXPConnect can be enabled for reftests marked with a special tag in the manifest?
Blocks: 455598
No longer blocks: 455598
Blocks: 456273
Attached patch Like so?Splinter Review
This enables "UniversalXPConnect" for all tests.
I have a half-baked patch that adds syntax to the manifest to choose
any privilege for each individual test too... if this is the right
approach at all...
Attachment #331860 - Attachment is obsolete: true
That patch:

1)  Nukes existing capability prefs if there are any
2)  Doesn't work unless the profile already has the "don't do the new file://
    security stuff" pref set, unless I'm missing something.
I'm not even sure we should do this for reftests; it encourages people to write tests that are usable only by Mozilla.  It would be nice if reftests were generally usable by other browsers.

(Note that we do now have the ability to do reftest-equivalent inside mochitest.)
(In reply to comment #7)
> 1)  Nukes existing capability prefs if there are any

The 'prefHasUserValue' was intended to avoid existing prefs and I actually
tested that existing "capability.principal.codebase.pN..." in prefs.js
survived untouched.  Maybe I'm missing something, please elaborate.

> 2)  Doesn't work unless the profile already has the "don't do the new file://
>     security stuff" pref set, unless I'm missing something.

Sorry, I don't know what pref set you're referring to.  Would it work if
I also add that pref set unless it already exists?
(In reply to comment #8)
I agree, but I think the primary reason for needing privileges is for
crash tests.  In my experience, some crash tests does not crash
reliably under mochitest.  (I just saw that today actually, in bug 456273
the test crashes reliably when loading from file: but only occasionally
when run as a mochitest.)

> (Note that we do now have the ability to do reftest-equivalent inside
> mochitest.)

Ok, I didn't know that. Then we should use that for the reftests that need
privileges. (BTW, is it documented how to use it somewhere?)

So what do you suggest for crash tests that need privileges?
Ah, gCapabilityPrefBranch doesn't include other capability prefs.  I misread that part, sorry.

> Sorry, I don't know what pref set you're referring to.

"security.fileuri.strict_origin_policy".  Setting that to false would probably make this patch work, yes...

> So what do you suggest for crash tests that need privileges?

Making file:// mochitests work might be the answer, since we need those anyway.
Blocks: 421839
This might be better served by fixing bug 468913.
Component: Testing → Reftest
Product: Core → Testing
QA Contact: testing → reftest
Version: Trunk → unspecified
We could easily set whatever prefs we want in runreftest.py now, although I don't know how dbaron feels about that. Another option would be to add a third reftest pass, like "reftest-privileged", which would create a profile with the right prefs, and run tests from a different reftest.list so that these tests could be sequestered from the other tests.
I'm ok with enabling more privileges in the crashtests than for the reftests.  I think privileged reftests should be done using the reftest-in-mochitest, in the rare cases that we really need them.
Blocks: 497875
Blocks: 414740
Summary: Enable UniversalXPConnect in tinderbox reftest profiles → Enable UniversalXPConnect in tinderbox crashtest and/or reftest profiles
Version: unspecified → Trunk
I should have returned here to comment long ago. 

Many bugs later, but before it was explicitly mentioned in the comments here, I learned of mochitest's window snapshot and comparison capabilities. These proved sufficient for the test I was trying to write at the time of filing this bug.

I'm happy for this to either be closed, or morphed to cover crashtests.
enablePrivilege is going away.  See bug 546848 and its dependency tree.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → WONTFIX
When we get the work in bug 462483 done, it should be trivial to make it available to reftests as well, if we want to go that route.
You need to log in before you can comment on or make changes to this bug.