1855869 - Unexpected passes in two M-xorig tests on Android Fission

Status: RESOLVED FIXED

(GeckoView Graveyard :: Sandboxing, defect, P2)

Description

[[:owlish] 🦉 PST]

The test A note:toolkit/components/extensions/test/mochitest/test_ext_contentscript_cache.html` fails with a manifest condition:

[task 2023-09-26T02:06:52.786Z] 02:06:52     INFO -  TEST-PASS | toolkit/components/extensions/test/mochitest/test_ext_contentscript_cache.html | Should have no cached scripts
[task 2023-09-26T02:06:52.786Z] 02:06:52     INFO -  TEST-PASS | toolkit/components/extensions/test/mochitest/test_ext_contentscript_cache.html | Should have one cached script
[task 2023-09-26T02:06:52.786Z] 02:06:52     INFO -  TEST-PASS | toolkit/components/extensions/test/mochitest/test_ext_contentscript_cache.html | undefined assertion name
[task 2023-09-26T02:06:52.786Z] 02:06:52     INFO -  TEST-PASS | toolkit/components/extensions/test/mochitest/test_ext_contentscript_cache.html | Should have no cached scripts after heap-minimize
[task 2023-09-26T02:06:52.786Z] 02:06:52     INFO -  add_task | Leaving test_contentscript_cache
[task 2023-09-26T02:06:52.786Z] 02:06:52     INFO -  Buffered messages finished
[task 2023-09-26T02:06:52.786Z] 02:06:52  WARNING -  TEST-UNEXPECTED-PASS | toolkit/components/extensions/test/mochitest/test_ext_contentscript_cache.html | fail-if condition in manifest - We expected at least one failure
[task 2023-09-26T02:06:52.786Z] 02:06:52     INFO -  TEST-INFO | expected FAIL

Fails exclusively on Android builds with isolateHighValue isolation strategy (fission.webContentIsolationStrategy set to 2). This might end up being a very simple failure where we would need to adjust an assertion or the expectation in the manifest, but it does seem like it needs a closer look, so I am filing a bug for this one.

Comment 1

[[:owlish] 🦉 PST]

Hey kmag, it looks like you might be familiar with the code - do you have ideas what might be going on here? Is it simply a matter of adjusting expectations in the ini or an actual bug?

Comment 2

[[:owlish] 🦉 PST]

I am adding the layout/base/tests/test_bug603550.html to this bug as well.
For context on this one: https://bugzilla.mozilla.org/show_bug.cgi?id=1677543

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

Redirect a needinfo that is pending on an inactive user to the triage owner.
:kaya, since the bug has recent activity, could you have a look please?

For more information, please visit BugBot documentation.

Comment 4

[[:owlish] 🦉 PST]

Hey Nika, I think you are the most familiar with the area. Do you have ideas what might be going on here? Is it simply a matter of adjusting expectations in the ini or an actual bug?

Comment 5

[[:owlish] 🦉 PST]

layout/base/tests/test_bug603550.html is related to OOP iframes, which I am not sure work on Android (and generally, iframes are not fully Fission-compatible IIRC)

Comment 6

[Nika Layzell [:nika] (ni? for response)]

Looks like toolkit/components/extensions/test/mochitest/test_ext_contentscript_cache.html was marked as fail-if = xorigin in the original patches which introduced xorigin (bug 1602668). Looking at the code, it appears that it's behaving differently depending on whether you have extensions loaded in the parent process or in an extension process. Is there a chance that you're running xorigin tests with parent process extensions? I imagine that could be what is causing the difference in behaviour there leading to the unexpected pass.

I find it interesting that it passes with isolateHighValue = true only though - that's a bit odd.

---

(In reply to [:owlish] 🦉 PST from comment #5)

layout/base/tests/test_bug603550.html is related to OOP iframes, which I am not sure work on Android (and generally, iframes are not fully Fission-compatible IIRC)

bug 603550 is much earlier than oop iframes were introduced (oop iframes ~= fission), so I don't think that's correct. All xorigin tests are run in oop iframes when run with fission and isolateEverything enabled (that's what makes them xorigin iframes).

This one being marked as fail-if = xorigin && fission is probably passing with isolateHighValue just because it's not actually being isolated like expected in that case.

---

It might be reasonable for us to only run xorigin tests with the isolateEverything configuration, and not with isolateHighValue to keep things simpler, as the tests don't give us much value in the isolateHighValue case as the tests themselves won't actually be isolated.

Comment 7

[[:owlish] 🦉 PST]

Sounds good, I'll make sure we don't run M-xorig with isolateHighValue

Comment 8

[[:owlish] 🦉 PST]

I checked - at the moment we run only plain mochitests with isolateHighValue. I won't be adding this variant for M-xorig.

As these tests pass on isolateEverything, which is what we are running at the moment, I will enable them.

Comment 9

[[:owlish] 🦉 PST]

Attachment: Bug 1855869 - Re-enable two mochitests that fail with isolateHighValue because we will not be running M-xorig with isolateHighValue

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f9694adb42c9
Re-enable two mochitests that fail with isolateHighValue because we will not be running M-xorig with isolateHighValue r=dholbert

Comment 11

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/f9694adb42c9