1596855 - Enable browser_permmgr_sync.js with Fission

Status: RESOLVED FIXED

(Core :: Permission Manager, task, P3)

Description

[Andrew McCreight [:mccr8]]

I think bug 1596834 is needed to make this test Fission-compatible, but my patch in that bug does not actually change the results with Fission. The test creates a new tab with about:blank (in a new process), runs some tests that pass, then loads example.com in an iframe by doing "content.document.body.appendChild(iframe)", then runs some tests. Then it sets some more permissions in the parent, then runs some more tests. Then it loads a subdomain of example.com with https, then runs some more tests.

The tests that fail seem to all be tests involving permissions for example.com that are expected to be ALLOW_ACTION.

My guess would be that the load of example.com creates a new process separate from the process about:blank is loaded in, but all of the scripts run in the about:blank process, and it looks like permissions for a domain only work in a process where the domain has been loaded, so these fail.

Comment 1

[Neha Kochar [:neha]]

Ethan, could you please find an assignee for this?

Comment 2

[(inactive) Nihanth Subramanya [:nhnt11]]

Paul, are you able to help with this one? Not sure if your previous work with permissions stuff is relevant for fixing this test.

Comment 3

[Emma Zühlcke [:emz]]

I'm afraid I'm not an expert at this. Maybe :ehsan can provide some input?

Comment 4

[(no longer active)]

Not sure what question needs my input on. I'd appreciate if you could be more specific please. Thanks!

Comment 5

[Ethan Tseng [:ethan]]

Thomas will look into this bug.

Comment 6

[Thomas Nguyen (:tnguyen)]

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

My guess would be that the load of example.com creates a new process separate from the process about:blank is loaded in, but all of the scripts run in the about:blank process, and it looks like permissions for a domain only work in a process where the domain has been loaded, so these fail.

That's true, I could see the permissions model: a child process (A) starts without the knowledge about any permissions. When we start visit a site, asking parent to send out corresponding permission to A.
The problem is, we are loading a site from cross-process example.com iframe (B), then B asks parent permission about permissions of example.com, then store it in B process. We run script checking permission on A, A doesn't know anything about permission in B process, then the test fails.
The problem seems to be hard to me in the case we do another navigate in B, then B has to notify its permissions to all ancestor parents. I don't know if it's worth to try, it would be a pain to walking up to the ancestor chain in fission to do that.
:ehsan. In permission model, is it allowed for A to look at permission in B process in the above example? Or do you have any idea/suggestion?
If it is not allowed, we just move the check inside iframe's process to make the test pass

Comment 7

[Andrew McCreight [:mccr8]]

I don't know much about permissions, but in general I'd think that we wouldn't want A to ever know about the permissions that B has. Maybe what you want is to change things so that the test script asking about B's permissions runs in B's process? And you'd also like to check, if Fission is enabled, that B has no permissions in A's process, even after B loads (of course, with non-Fission this will not be true).

Comment 8

[(no longer active)]

(In reply to Thomas Nguyen (:tnguyen) from comment #6)

The problem is, we are loading a site from cross-process example.com iframe (B), then B asks parent permission about permissions of example.com, then store it in B process. We run script checking permission on A, A doesn't know anything about permission in B process, then the test fails.
The problem seems to be hard to me in the case we do another navigate in B, then B has to notify its permissions to all ancestor parents. I don't know if it's worth to try, it would be a pain to walking up to the ancestor chain in fission to do that.
:ehsan. In permission model, is it allowed for A to look at permission in B process in the above example? Or do you have any idea/suggestion?
If it is not allowed, we just move the check inside iframe's process to make the test pass

I don't think we want B to notify its parent frame process about the permissions that it has. Quite the opposite in fact, we want to rewrite this test in order to make sure that it checks the permissions in the correct content process, IOW the code that checks the permissions should run in the content process for B. In fact the test should probably check if we're in Fission mode, and if so verify that the content process for about:blank never receives any of these permissions.

The basic idea behind the way the permission manager notifies the content processes is that each content process should only ever be told about the permissions it would need, and not about unrelated permissions that wouldn't make any difference to the way it would render web content.

Comment 9

[Thomas Nguyen (:tnguyen)]

Agree, that makes sense to me. Thanks

Comment 10

[Chris Peterson [:cpeterson]]

Temporarily reassigning these DOM Security Fission bugs to ckerschb for re-triage.

Comment 11

[Chris Peterson [:cpeterson]]

This bug is a blocker for Fission's current milestone (M4.1 aka "fix all the mochitests"), but it's currently unassigned. The Fission team is hoping teams will fix their mochitests for Fission before the end of Q1 (75 or 76 Nightly).

If you don't think this mochitest failure should block shipping Fission, just let me know.

Comment 12

[Ethan Tseng [:ethan]]

Johann, will you be able to work on this bug?

Comment 13

[Johann Hofmann [:johannh]]

Yup, I can take a look.

Comment 14

[Chris Peterson [:cpeterson]]

Johann, any updates on this test bug? It's still disabled for Fission. Fixing and re-enabling mochitests blocks enabling Fission in Nightly.

Comment 15

[Chris Peterson [:cpeterson]]

The Fission team hopes to roll out Fission to some Nightly users in Q3, so it would be good if all tests are passing and enabled for Fission by the end of Q2.

Comment 16

[Johann Hofmann [:johannh]]

Attachment: Bug 1596855 - Update browser_permmgr_sync.js for Fission. r=pbz

The test was updated to ensure that under Fission we're not syncing
all permission information to a "parent" content process, and instead
just to the specific iframe that needs it.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7c209c5db026
Update browser_permmgr_sync.js for Fission. r=pbz

Comment 18

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/7c209c5db026