1572895 - Update some XPConnect mochitests to work under Fission

Status: RESOLVED FIXED

(Core :: XPConnect, task)

Comment 1

[Kris Maglione [:kmag]]

Attachment: Bug 1572895 - Update some XPConnect mochitests to work under Fission. r=mccr8

Comment 2

[Andrew McCreight [:mccr8]]

Boris, test_bug505915.html checks if cross origin access causes a specific kind of security error. With Fission, this particular access just isn't allowed at all, so Kris's change to make the test work when Fission is testing something else entirely (it just confirms that you can't access the thing cross origin at all). Do you think that's okay? Maybe we can assume that non-Fission tests are always going to be run?

Comment 3

[Kris Maglione [:kmag]]

(In reply to Andrew McCreight [:mccr8] from comment #2)

Boris, test_bug505915.html checks if cross origin access causes a specific kind of security error. With Fission, this particular access just isn't allowed at all, so Kris's change to make the test work when Fission is testing something else entirely (it just confirms that you can't access the thing cross origin at all). Do you think that's okay? Maybe we can assume that non-Fission tests are always going to be run?

To be clear, it tests the same thing with Fission is disabled. When Fission is enabled, it just tests that we don't have access to the cross-origin document at all, which has the same effect of not allowing us to create a tree walker for it.

The alternative is to just skip the test under Fission, but I think it's nice to continue testing that the same thing will never be possible under the same circumstances of the original bug.

Comment 4

[Andrew McCreight [:mccr8]]

(In reply to Kris Maglione [:kmag] from comment #3)

To be clear, it tests the same thing with Fission is disabled. When Fission is enabled, it just tests that we don't have access to the cross-origin document at all, which has the same effect of not allowing us to create a tree walker for it.
Well, it tests that the access fails, but the original bug was about making sure that it actually failed due to NS_ERROR_XPC_SECURITY_MANAGER_VETO and not something else, AFAICT.

The alternative is to just skip the test under Fission, but I think it's nice to continue testing that the same thing will never be possible under the same circumstances of the original bug.

That's true. The alternative I was thinking of was adding a chrome Mochitest for this behavior, but maybe that's overkill.

Comment 5

[Kris Maglione [:kmag]]

(In reply to Andrew McCreight [:mccr8] from comment #4)

The alternative is to just skip the test under Fission, but I think it's nice to continue testing that the same thing will never be possible under the same circumstances of the original bug.

That's true. The alternative I was thinking of was adding a chrome Mochitest for this behavior, but maybe that's overkill.

Well, if we created a chrome mochitest, it would have to be a chrome mochitest which creates a content window, which creates another content window, and does all of the actual work.

That's kind of not the point of chrome mochitests :)

Comment 6

[Kris Maglione [:kmag]]

(That said, at this point it could pretty easily be converted to an xpcshell test, which would be much more efficient)

Comment 7

[Boris Zbarsky [:bzbarsky]]

The simplest fix to the test would be if we could use a URL for the iframe which is same-site with the mochitest hostname (and hence same-process) but not same-origin. Maybe something on //test1.mochi.test:8888 or something?

Comment 8

[Kris Maglione [:kmag]]

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

The simplest fix to the test would be if we could use a URL for the iframe which is same-site with the mochitest hostname (and hence same-process) but not same-origin. Maybe something on //test1.mochi.test:8888 or something?

Even if that works now (and I don't think it does), it will stop working soon. We use the entire origin string to choose the process to put a frame in. The only exception right now is that we accidentally ignore origin attributes, since we construct a principal from just the URI to get the origin string from, but that's going to change.

If we want to continue testing the cross-origin case, I guess I'll just migrate it to an xpcshell test with a windowless browser.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Even if that works now (and I don't think it does), it will stop working soon. We use the entire origin string to choose the process to put a frame in

Uh... how can that possibly work? The test is running on http://mochi.test:8888, right? If the subframe is on http://test1.mochi.test:8888, then they can synchronously access each other if they both set document.domain to mochi.test, so you can't put them in separate processes. If we do put such things in separate processes with Fission, that is a major bug we need to fix.

Comment 10

[Kris Maglione [:kmag]]

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

Even if that works now (and I don't think it does), it will stop working soon. We use the entire origin string to choose the process to put a frame in

Uh... how can that possibly work? The test is running on http://mochi.test:8888, right? If the subframe is on http://test1.mochi.test:8888, then they can synchronously access each other if they both set document.domain to mochi.test, so you can't put them in separate processes. If we do put such things in separate processes with Fission, that is a major bug we need to fix.

Oh, sorry, I misunderstood what you were suggesting. Yeah, I suppose that might work. We construct the webisolated origin string using siteOrigin, so those would always wind up in the same process.

Comment 11

[Boris Zbarsky [:bzbarsky]]

OK. I'd prefer it if we did the test1.mochi.test thing and didn't change the rest of the test.

Comment 12

[Kris Maglione [:kmag]]

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

OK. I'd prefer it if we did the test1.mochi.test thing and didn't change the rest of the test.

Already done :)

Comment 13

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4477d849b72a0cf15159b1f99a8530a250575365
Bug 1572895 - Update some XPConnect mochitests to work under Fission. r=mccr8

Comment 14

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/4477d849b72a