A bunch of tests don't like waiting for focus/activate events once we remove the initial about:blank load

RESOLVED FIXED in Firefox 65

Status

()

enhancement
P1
normal
RESOLVED FIXED
6 months ago
6 months ago

People

(Reporter: Gijs, Assigned: Gijs)

Tracking

(Blocks 1 bug)

Trunk
Firefox 65
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox65 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 months ago
I tested the fix in bug 1500818 against the status quo, and I tested locally whether it fixed orange I saw in browser_contextmenu_linkopen.js with the initial about:blank doc removed.

Unfortunately, before landing I neglected to check what the result of the combination of that patch and disabling the initial about:blank doc was on try, against all the other tests - I just assumed it'd be beneficial everywhere. Here's an actual trypush:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2a1416e0ea05d318211db0e3ffd0d3951d0eae4e&selectedJob=207070966

Now, the good news is most of the failures are very straightforward. Tests wait for the new window, and then wait for the initial browser to load - but they now time out on Linux, because the focus/activate events arrive after the initial browser's loaded, so then they wait forever for the document in question to load because it already did.

I'm a bit on the fence about whether to fix all these tests, or back out bug 1500818 and then fix just browser_contextmenu_linkopen.js to explicitly wait for focus/activate stuff itself. On the one hand, fixing these things will, I believe, make the tests more resilient, and the focus/activate stuff is a 'gotcha' that we should try to address structurally. On the other, we're replacing one hard-to-figure-out cause of issues (spurious focus/activate events after window opening/closing interfering with popups) with another (focus/activate events making it harder to 'catch' other events early in the lifetime of the new window).

I've got a local set of patches for these failures so I'll put that up and then you can decide, Dão, or we can discuss in more detail if you've got other ideas about how to address this.
(Assignee)

Comment 3

6 months ago
I'm updating the patch to omit changes to browser/components/sessionstore/test/browser_354894_perwindowpb.js (which wasn't fixed by the changes I pushed earlier, cf.  https://treeherder.mozilla.org/#/jobs?repo=try&revision=10ac608f414670045bbe7380e26a770c902b718b&selectedJob=207103114 ) and browser/base/content/test/plugins/browser_private_clicktoplay.js , which went orange on windows both with and without the about:blank changes. I'll have to look into those in more depth - which kind of makes sense, they were the least straightforward updates.

I'll note that when running trypushes for the changes in bug 1496360 I saw similar failures in browser_private_clicktoplay.js, even without the changes from bug 1500818, so something will have to be fixed there regardless ( https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=204820344&revision=43d2d9c9b132e6a52fbaa518c46e0240f8683f9c ).

Both before and after the initial about:blank doc no longer being loaded, the remaining changes to the tests seem to be fine.

Comment 4

6 months ago
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f8615bd92bc6
fix various tests to not race with window focus/activate, r=dao

Comment 5

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f8615bd92bc6
Status: ASSIGNED → RESOLVED
Last Resolved: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
You need to log in before you can comment on or make changes to this bug.