1586139 - Fix reader mode browser chrome tests so they pass with fission enabled

Status: RESOLVED FIXED

(Toolkit :: Reader Mode, defect, P1)

Description

[:Gijs (back August 24; he/him)]

It's unclear why these tests fail; reader mode shouldn't really need fission work (I hope?)

Comment 1

[:Gijs (back August 24; he/him)]

This seems to be due to the use of event listening in the content frame; the use of BrowserTestUtils.waitForContentEvent in 5 of the 6 tests is broken in fission, possibly because we process-switch and end up with the listener in the "wrong" process, before that switch occurs.

The other test, browser_readerMode_hidden_nodes.js, manually adds DOMContentLoaded and MozAfterPaint handlers, and other tests in the fission spreadsheet also list having problems with this - potentially due to a similar issue of adding the listeners too late or too early.

Comment 2

[:Gijs (back August 24; he/him)]

This needs a "proper" way to wait for custom/untrusted content events to fix.

Comment 3

[:Gijs (back August 24; he/him)]

Attachment: Bug 1586139 - enable reader mode tests in fission, r?mccr8

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/adfdc1b55325
enable reader mode tests in fission, r=mccr8

Comment 5

[:Gijs (back August 24; he/him)]

(In reply to Pulsebot from comment #4)

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/adfdc1b55325
enable reader mode tests in fission, r=mccr8

Why was this pushed? The trypush has one of the tests perma-orange on windows-10-64-qr ( https://treeherder.mozilla.org/#/jobs?repo=try&revision=e72b0953d8021901b456cc741290b97d504ea081 ). I was intending to look into it next week, I kind of have a lot on my plate at the moment so I didn't get to it...

I expect you probably want to disable browser_readerMode.js on windows+fission for now...

Comment 6

[Raul Gurzau (:RaulG)]

Looked over on the try push and did some more retriggers there on windows-10-64-qr as i see only one failure for browser_readerMode.js. Will wait for them to finish to see if it's just high-frequency or perma failure indeed and if the disable on that test is needed. Thank you.

Comment 7

[:Gijs (back August 24; he/him)]

(In reply to Raul Gurzau (:RaulG) from comment #6)

Looked over on the try push and did some more retriggers there on windows-10-64-qr as i see only one failure for browser_readerMode.js.

It's orange on every one of the bc2 jobs that have finished at this point on https://treeherder.mozilla.org/#/jobs?repo=try&revision=e72b0953d8021901b456cc741290b97d504ea081&selectedJob=276436013 - it's below the failure of browser/components/sessionstore/test/browser_windowStateContainer.js on 4 of the 5 failing jobs.

Comment 8

[Raul Gurzau (:RaulG)]

Attachment: Bug 1586139 - Disabled browser_readerMode.js on windows with fission. r=Gijs

Comment 9

[Raul Gurzau (:RaulG)]

Disabled browser_readerMode.js and another test on linux as on Try it seems to fail: https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&selectedJob=276434280&revision=e72b0953d8021901b456cc741290b97d504ea081\

I retriggered jobs on Autoland on windows-10-64-qr with fission and the failure is not there: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=276642906&resultStatus=testfailed%2Cbusted%2Cexception%2Csuccess%2Cretry%2Cusercancel%2Crunnable&searchStr=windows%2C10%2Cx64%2Cquantumrender%2Copt%2Cmochitests%2Cwith%2Cfission%2Cenabled%2Ctest-windows10-64-qr%2Fopt-mochitest-browser-chrome-fis-e10s-2%2Cm-fis%28bc2%29&tochange=adfdc1b55325d4b7e93381b903d186d73f2cd8f2&fromchange=05f4ac2d6dd6beb54482ce138d36cf3226f2464c

I also looked for this job on previous pushes and it looks like it stopped running on autoland. In m-c i can see it.

Comment 10

[:Gijs (back August 24; he/him)]

(In reply to Raul Gurzau (:RaulG) from comment #9)

Disabled browser_readerMode.js and another test on linux as on Try it seems to fail: https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&selectedJob=276434280&revision=e72b0953d8021901b456cc741290b97d504ea081

As noted on phab, this commit ( https://hg.mozilla.org/integration/autoland/rev/adfdc1b55325 ) only changed reader mode test files. There's no way it's got anything to do with the scrollPositions.js failure.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/16aae9e3e3ea
Disabled browser_readerMode.js on windows with fission. r=Gijs

Comment 12

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/adfdc1b55325
https://hg.mozilla.org/mozilla-central/rev/16aae9e3e3ea

Comment 13

[:Gijs (back August 24; he/him)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=aef6efa211a25c6b9622fe8114a7795974bf2b44

Comment 14

[:Gijs (back August 24; he/him)]

Attachment: Bug 1586139 - fix race condition in reader mode test, r?kmag

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3d2ddf81f801
fix race condition in reader mode test, r=kmag

Comment 16

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/3d2ddf81f801