Closed Bug 1576742 Opened 1 year ago Closed 1 year ago

Remove Fission-incompatible test of UniversalXPConnect gunk

Categories

(Core :: XPConnect, task, P2)

task

Tracking

()

RESOLVED FIXED
mozilla71
Fission Milestone M4
Tracking Status
firefox71 --- fixed

People

(Reporter: kmag, Assigned: kmag)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

We don't support UniversalXPConnect outside of tests anymore, and even in tests we want to get rid of it, so let's just remove this test rather than updating it.

I'm not sure I'm OK with this until we actually stop using UniversalXPConnect...

Should we just disable this test when Fission is enabled for the moment? And if we don't plan to stop using UniversalXPConnect before Fission ships, we probably do want to fix this test....

Flags: needinfo?(kmaglione+bmo)

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #2)

I'm not sure I'm OK with this until we actually stop using UniversalXPConnect...

Should we just disable this test when Fission is enabled for the moment? And if we don't plan to stop using UniversalXPConnect before Fission ships, we probably do want to fix this test....

We could just change it to use a cross-origin, same-process frame, but I'd honestly rather just delete it. The only things that still rely on UniversalXPConnect are a few scattered tests, and I don't think we need to ensure it preserves its current exact semantics strictly for their sake.

That said, I also don't think this test makes any kind of sense anymore. It calls enablePrivilege in a function scope, and assumes that the privilege only lasts to the end of that scope. But that only made sense when enablePrivilege worked by examining the stack for signed code which had added privileges. What remains of it now just works at the compartment level. It doesn't care about stacks, and it certainly doesn't go away at the end of the scope that it was called in.

Flags: needinfo?(kmaglione+bmo)

The only things that still rely on UniversalXPConnect are a few scattered tests

I thought all our Talos tests had a fundamental dependency on UniversalXPConnect to work properly. In fact, that's the one real blocker for removing UniversalXPConnect altogether that I know of.

Will look into the test details tomorrow.

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #4)

The only things that still rely on UniversalXPConnect are a few scattered tests

I thought all our Talos tests had a fundamental dependency on UniversalXPConnect to work properly. In fact, that's the one real blocker for removing UniversalXPConnect altogether that I know of.

There are a few scattered parts of talos that still use it (mainly the profiler code and the a11y tests), but I wouldn't call it fundamental. In any case, none of them should be using it to reach into cross-origin documents (and from what I can tell, none of them do). If they do, they'll be broken by Fission long before they're broken by any changes to UniversalXPConnect.

Priority: -- → P2
Fission Milestone: --- → M4
Pushed by maglione.k@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/ddaafba7326f
Remove test for UniversalXPConnect gunk. r=bzbarsky
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
You need to log in before you can comment on or make changes to this bug.