1654437 - [meta] Fix failing password manager mochitests with cross-origin and Fission enabled

Status: RESOLVED FIXED

(Toolkit :: Password Manager, defect, P2)

Description

[Neha Kochar [:neha]]

These tests are marked as skipped/failed for cross-origin and Fission, and need to be fixed
toolkit/components/passwordmgr/test/mochitest/test_autocomplete_highlight_non_login.html
toolkit/components/passwordmgr/test/mochitest/test_autocomplete_https_downgrade.html
toolkit/components/passwordmgr/test/mochitest/test_autocomplete_https_upgrade.html
toolkit/components/passwordmgr/test/mochitest/test_autocomplete_password_generation_confirm.html
toolkit/components/passwordmgr/test/mochitest/test_autofill_different_formActionOrigin.html
toolkit/components/passwordmgr/test/mochitest/test_autofill_different_subdomain.html
toolkit/components/passwordmgr/test/mochitest/test_autofill_highlight.html
toolkit/components/passwordmgr/test/mochitest/test_autofill_highlight_empty_username.html
toolkit/components/passwordmgr/test/mochitest/test_autofill_https_downgrade.html
toolkit/components/passwordmgr/test/mochitest/test_autofill_https_upgrade.html
toolkit/components/passwordmgr/test/mochitest/test_autofill_password-only.html
toolkit/components/passwordmgr/test/mochitest/test_basic_form.html
toolkit/components/passwordmgr/test/mochitest/test_basic_form_0pw.html
toolkit/components/passwordmgr/test/mochitest/test_basic_form_1pw.html
toolkit/components/passwordmgr/test/mochitest/test_basic_form_1pw_2.html
toolkit/components/passwordmgr/test/mochitest/test_basic_form_2pw_1.html
toolkit/components/passwordmgr/test/mochitest/test_basic_form_2pw_2.html
toolkit/components/passwordmgr/test/mochitest/test_basic_form_3pw_1.html
toolkit/components/passwordmgr/test/mochitest/test_basic_form_html5.html
toolkit/components/passwordmgr/test/mochitest/test_basic_form_pwevent.html
toolkit/components/passwordmgr/test/mochitest/test_basic_form_pwonly.html
toolkit/components/passwordmgr/test/mochitest/test_form_action_1.html
toolkit/components/passwordmgr/test/mochitest/test_form_action_2.html
toolkit/components/passwordmgr/test/mochitest/test_form_action_javascript.html
toolkit/components/passwordmgr/test/mochitest/test_formless_autofill.html
toolkit/components/passwordmgr/test/mochitest/test_formless_submit.html
toolkit/components/passwordmgr/test/mochitest/test_formless_submit_navigation.html
toolkit/components/passwordmgr/test/mochitest/test_maxlength.html
toolkit/components/passwordmgr/test/mochitest/test_munged_values.html
toolkit/components/passwordmgr/test/mochitest/test_password_field_autocomplete.html
toolkit/components/passwordmgr/test/mochitest/test_password_length.html
toolkit/components/passwordmgr/test/mochitest/test_prompt.html
toolkit/components/passwordmgr/test/mochitest/test_submit_without_field_modifications.html
toolkit/components/passwordmgr/test/mochitest/test_username_focus.html
toolkit/components/passwordmgr/test/mochitest/test_autocomplete_sandboxed.html

https://docs.google.com/spreadsheets/d/16G5AZhHWWow3rBgim4QBHzWXMIIJiky2SzXYgDMTTKY/edit#gid=1354562828&range=84:119

More info: https://wiki.mozilla.org/Project_Fission/Enabling_Tests_with_Fission#Cross-Origin_Mochitests

Comment 1

[Neha Kochar [:neha]]

Matt, could you please investigate these failures to find out if these failures are because of broken features (M6b - Fx82) or broken tests (M7 - Fx83-85) ?

Comment 2

[Neil Deakin]

Most of the tests here are failing because the observer notification that notifies that the password fill-in is complete is being sent from the wrong process at https://searchfox.org/mozilla-central/rev/ab81b8552f4aa9696a2524f97fdfeb59d4dc31c1/toolkit/components/passwordmgr/LoginManagerChild.jsm#662

While this isn't in test code, this notification is only used to notify tests so that they can start testing. I confirmed visually for some tests that the password fill-in does work correctly.

Some of the autocomplete tests are broken for a different reason (the ones that start with test_auto* as well as a few others, including some tests not listed in comment 0). The '--enable-xorigin-tests' flag seems to cause the test iframe to load within example.org instead of example.com
Some of the autocomplete tests rely on the page being loaded from 'example.com' as they add login details for example.com and expect to fill them into a page loaded from there.

As an example, I changed the test test_case_differences.html to add logins for 'example.org' instead and it then works fine. But, of course, it no longer works without the '--enable-xorigin-tests' flag.

The best approach here would be to go through each of the tests and use the page's domain, or some other generated name, rather than hardcoding 'example.com' and 'example.org' for the login details.

Comment 3

[Matthew N. [:MattN]]

https://searchfox.org/mozilla-central/rev/8df04ff073d0fa70f692935c5dcddc0e23cb0117/testing/mochitest/runtests.py#1135-1140 seems fishy to me and doesn't seem to honour the scheme = https option.

Comment 4

[Neil Deakin]

I verified that all the tests except two are caused by the issues described above in comment 2 or some variation.

The two remaining tests are test_formless_submit_navigation.html and test_formless_submit_navigation_negative.html. These fail calling history.pushState(). However, I assume that this is caused by 1649131.

Comment 5

[Neha Kochar [:neha]]

We still have 2 tests remaining:
toolkit/components/passwordmgr/test/browser/browser_doorhanger_generated_password.js
toolkit/components/passwordmgr/test/browser/browser_doorhanger_toggles.js

See https://docs.google.com/spreadsheets/d/16G5AZhHWWow3rBgim4QBHzWXMIIJiky2SzXYgDMTTKY/edit#gid=1354562828&range=153:156

Comment 6

[Neil Deakin]

Looks like they were disabled by 1668809 because of session history. Maybe Olli can explain what the issue with these tests are.

They are disabled in non-cross origin mode as well.

Comment 7

[Neha Kochar [:neha]]

Anny, can you check these failures and find more info on why these fail with SHIP?

Comment 8

[Anny Gakhokidze [:annyG]]

Looking! I think I got a fix for these two, will upload a patch once I'm back to work.

Comment 9

[Neha Kochar [:neha]]

Assigning to Anny as she's got a patch for these. :)

Comment 10

[Anny Gakhokidze [:annyG]]

Attachment: Bug 1654437 - Fix failing password manager tests for Fission, r=kmag!

The tests submit a form, which leads to a URL change, and then assert correct
location.pathname, and all of this takes place inside of SpecialPowers.spawn.
The test fails becase the SpecialPowers JS actor gets destroyed after the form
gets submitted, while there is still pending code to execute and this throws an
error. We need to submit a form, wait for the browser to load, and then check
location.pathname.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ee4beb10381b
Fix failing password manager tests for Fission, r=NeilDeakin

Comment 12

[Narcis Beleuzu [:NarcisB]]

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