1527922 - Perma DevEdition browser_toolbarKeyNav.js | Test timed out when Gecko 67 merges to Beta on 2019-03-11

Status: VERIFIED FIXED

(Firefox :: Keyboard Navigation, defect, P2)

Description

[Oana Pop-Rus]

[Tracking Requested - why for this release]:

Central as beta simulation : https://treeherder.mozilla.org/#/jobs?repo=try&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel%2Crunnable&revision=0d9e35367944d97278d6d8b925e733e1f1c245be&selectedJob=228396527

Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=228396527&repo=try&lineNumber=1442

06:07:15 INFO - TEST-INFO | started process screencapture
06:07:16 INFO - TEST-INFO | screencapture: exit 0
06:07:16 INFO - Buffered messages logged at 06:06:30
06:07:16 INFO - Entering test bound setPref
06:07:16 INFO - Leaving test bound setPref
06:07:16 INFO - Entering test bound testTabStopsNoPage
06:07:16 INFO - Buffered messages logged at 06:06:31
06:07:16 INFO - TEST-PASS | browser/base/content/test/keyboard/browser_toolbarKeyNav.js | URL bar focused for start of test -
06:07:16 INFO - Console message: [JavaScript Error: "getScreenshot(https://example.com/) failed: TypeError: NetworkError when attempting to fetch resource." {file: "resource://activity-stream/lib/Screenshots.jsm" line: 45}]
06:07:16 INFO - getScreenshotForURL@resource://activity-stream/lib/Screenshots.jsm:45:10
06:07:16 INFO -
06:07:16 INFO - Console message: [JavaScript Error: "Unknown collection "main/tippytop"" {file: "resource://services-settings/RemoteSettingsClient.jsm" line: 259}]
06:07:16 INFO - sync@resource://services-settings/RemoteSettingsClient.jsm:259:13
06:07:16 INFO -
06:07:16 INFO - Buffered messages finished
06:07:16 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/keyboard/browser_toolbarKeyNav.js | Test timed out -
06:07:16 INFO - GECKO(926) | MEMORY STAT | vsize 4413MB | residentFast 315MB | heapAllocated 94MB
06:07:16 INFO - TEST-OK | browser/base/content/test/keyboard/browser_toolbarKeyNav.js | took 45018ms
06:07:16 INFO - Not taking screenshot here: see the one that was previously logged
06:07:16 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/keyboard/browser_toolbarKeyNav.js | Found a tab after previous test timed out: about:blank -
06:07:16 INFO - checking window state
06:07:17 INFO - GECKO(926) | Completed ShutdownLeaks collections in process 930
06:07:17 INFO - GECKO(926) | Completed ShutdownLeaks collections in process 935
06:07:17 INFO - GECKO(926) | Completed ShutdownLeaks collections in process 932
06:07:17 INFO - GECKO(926) | Completed ShutdownLeaks collections in process 931
06:07:17 INFO - GECKO(926) | Completed ShutdownLeaks collections in process 934
06:07:17 INFO - GECKO(926) | Completed ShutdownLeaks collections in process 933
06:07:17 INFO - GECKO(926) | Completed ShutdownLeaks collections in process 936
06:07:17 INFO - GECKO(926) | Completed ShutdownLeaks collections in process 927
06:07:17 INFO - GECKO(926) | Completed ShutdownLeaks collections in process 928
06:07:17 INFO - GECKO(926) | Completed ShutdownLeaks collections in process 926
06:07:17 INFO - TEST-START | Shutdown

Comment 1

[Dão Gottwald [:dao]]

Jamie, can you look into this?

Comment 2

[Intermittent Failures Robot]

4 failures in 2602 pushes (0.002 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• try: 4

Platform breakdown:

• macosx64-devedition: 2
• linux64-devedition: 1
• windows10-64-devedition: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1527922&startday=2019-02-18&endday=2019-02-24&tree=all

Comment 3

[James Teh [:Jamie]]

I'm trying to reproduce this locally, but I can't get it to build. I applied the patch from:
https://hg.mozilla.org/try/rev/0d9e35367944d97278d6d8b925e733e1f1c245be
to central. However, I get build failures in JS:

js/src
In file included from c:/Users/jamie/src/gecko/js/src/builtin/RegExp.cpp:22:
In file included from c:/Users/jamie/src/gecko/js/src\vm/EnvironmentObject-inl.h:10:
In file included from c:/Users/jamie/src/gecko/js/src\vm/EnvironmentObject.h:10:
In file included from c:/Users/jamie/src/gecko/js/src\builtin/ModuleObject.h:15:
In file included from c:/Users/jamie/src/gecko/js/src\frontend/EitherParser.h:24:
In file included from c:/Users/jamie/src/gecko/js/src\frontend/BCEParserHandle.h:12:
c:/Users/jamie/src/gecko/js/src\frontend/Parser.h(272,29):  error: out-of-line declaration of 'TraceBinParser' does not match any declaration in namespace 'js::frontend'; did you mean 'TraceParser'?
  friend void js::frontend::TraceBinParser(JSTracer* trc,
                            ^~~~~~~~~~~~~~
                            TraceParser
c:/Users/jamie/src/gecko/js/src\frontend/BytecodeCompiler.h(118,6):  note: 'TraceParser' declared here
void TraceParser(JSTracer* trc, JS::AutoGCRooter* parser);
     ^
1 error generated.

Is there some other patch I need to apply?

Comment 4

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Hi Jamie, please apply the patches from https://treeherder.mozilla.org/#/jobs?repo=try&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel%2Crunnable&revision=84d36bfb378e090bcdf060490d21a6b62b4abe6e to get working builds. That's tested with https://hg.mozilla.org/mozilla-central/rev/d97cc5b9eeae57f6fb5d4b595ca68a569afa7bd2 as base.

Comment 5

[James Teh [:Jamie]]

I can't reproduce this test failure applying those patches locally. I wonder if this is somehow related to having the official branding, which I'm guessing you only get if you build on CI.

I think the problem might be whether the Reload button is enabled or disabled when no page (about:blank) is loaded. Testing manually, this seems to be somewhat quirky. The first time I load about:blank in a new window, the Reload button is disabled. If I close that tab and open a new one with about:blank, the Reload button is enabled. If I press Reload, the Reload button gets disabled and the Back button gets enabled. What is the rule supposed to be regarding about:blank and the Reload button? The key nav tests depend on this being consistent.

Comment 6

[Dão Gottwald [:dao]]

(In reply to James Teh [:Jamie] from comment #5)

What is the rule supposed to be regarding about:blank and the Reload button?

https://searchfox.org/mozilla-central/rev/2a6f3dde00801374d3b2a704232de54a132af389/browser/base/content/browser.js#4818-4824

Comment 7

[James Teh [:Jamie]]

I saw that code, but that doesn't really explain this:

The first time I load about:blank in a new window, the Reload button is disabled. If I close that tab and open a new one with about:blank, the Reload button is enabled. If I press Reload, the Reload button gets disabled and the Back button gets enabled.

Surely this inconsistency isn't "intentional"? Is there some way I can make it consistent at least for the purposes of this test?

Comment 8

[Dão Gottwald [:dao]]

No, it isn't intentional, it would be a bug in that code. Short of fixing that bug, you could probably also use a custom test page (even just using data:) instead of about:blank.

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Please be aware that the merges from central to beta start next Monday and this test will permafail. Thank you.

Comment 10

[James Teh [:Jamie]]

Attachment: Bug 1527922: Ensure the Reload button is disabled when testing against blank tabs in the browser toolbar key nav tests.

For a blank tab, the Reload button should be disabled.
These tests depend on this.
This seems to be true when setting the new tab page to blank in Firefox Options.
However, when we open about:blank with BrowserTestUtils.withNewTab, this is unreliable.
That is, sometimes the Reload button is enabled, sometimes it isn't.
I don't understand why this happens.
For the purposes of these tests, just force the Reload button to be disabled for new, blank tabs so we get consistent results.

Comment 11

[James Teh [:Jamie]]

(In reply to Dão Gottwald [::dao] from comment #8)

No, it isn't intentional, it would be a bug in that code. Short of fixing that bug, you could probably also use a custom test page (even just using data:) instead of about:blank.

These tests are explicitly testing the behaviour when no page is loaded; i.e. the page actions buttons and Back, Forward and Reload buttons should be disabled. So, I can't use a custom page. Hopefully, the hack I've implemented in the tests to work around this will suffice.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c26d4a8d43c4
Ensure the Reload button is disabled when testing against blank tabs in the browser toolbar key nav tests. r=Gijs

Comment 13

[Stefan Hindli [:stefan_hindli]]

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

Comment 14

[Natalia Csoregi [:nataliaCs]]

This issue is still reoccuring: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3f07e637209c105feec8e582c3b55b69f178de4f&searchStr=browser-chrome&group_state=expanded&selectedJob=232764689

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=232764689&repo=try&lineNumber=2625

:jamie any chance you could take a look?

Comment 15

[Natalia Csoregi [:nataliaCs]]

[Tracking Requested - why for this release]:

Comment 16

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Bug 1527922 - Remove DevTools button in browser_toolbarKeyNav.js to prevent branch-specific focus in toolbar

Comment 17

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

This fails because the DevEdition has the DevTools button in the toolbar which gets focused instead of the home button as the screenshot artifact ("mozilla-test-fail-screenshot_irul1s.png") of the log shows: https://taskcluster-artifacts.net/Ymkd2aPNQkON6PjN9adaHw/0/public/test_info/mozilla-test-fail-screenshot_irul1s.png

Questions:
Is it expected that the test reports "Test timed out" or should the button mismatch be shown as a human-readable string("Expected X, got Y")?
Should the previous changes in this bug stay in the tree?

Patch is untested, will get added to 2019-03-11 beta simulation.

Comment 18

[Intermittent Failures Robot]

4 failures in 2818 pushes (0.001 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• try: 4

Platform breakdown:

• linux32-devedition: 1
• linux64-devedition: 1
• macosx64-devedition: 2

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1527922&startday=2019-03-04&endday=2019-03-10&tree=all

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f21b07ac7fb7
Remove DevTools button in browser_toolbarKeyNav.js to prevent branch-specific focus in toolbar r=Gijs

Comment 20

[:Gijs (he/him)]

(In reply to James Teh [:Jamie] from comment #11)

(In reply to Dão Gottwald [::dao] from comment #8)

No, it isn't intentional, it would be a bug in that code. Short of fixing that bug, you could probably also use a custom test page (even just using data:) instead of about:blank.

These tests are explicitly testing the behaviour when no page is loaded; i.e. the page actions buttons and Back, Forward and Reload buttons should be disabled. So, I can't use a custom page. Hopefully, the hack I've implemented in the tests to work around this will suffice.

Jamie, can you check if the button in question that was sometimes disabled and sometimes wasn't is really the reload/stop button (see Aryx's commit about another button interfering on devedition) and if so, file a follow-up bug for that behavior, ideally with concrete steps to reproduce)? Thanks.

Comment 21

[:Gijs (he/him)]

(In reply to :Gijs (he/him) from comment #20)

(In reply to James Teh [:Jamie] from comment #11)

(In reply to Dão Gottwald [::dao] from comment #8)

No, it isn't intentional, it would be a bug in that code. Short of fixing that bug, you could probably also use a custom test page (even just using data:) instead of about:blank.

These tests are explicitly testing the behaviour when no page is loaded; i.e. the page actions buttons and Back, Forward and Reload buttons should be disabled. So, I can't use a custom page. Hopefully, the hack I've implemented in the tests to work around this will suffice.

Jamie, can you check if the button in question that was sometimes disabled and sometimes wasn't is really the reload/stop button (see Aryx's commit about another button interfering on devedition) and if so, file a follow-up bug for that behavior, ideally with concrete steps to reproduce)? Thanks.

Oh, it seems this already happened and is bug 1533633.

Comment 22

[Raul Gurzau (:RaulG)]

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

Comment 23

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

It's still failing: https://treeherder.mozilla.org/#/jobs?repo=try&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel%2Crunnable&revision=932e020b1987edc382cb5401e391214ea93d8ab5&selectedJob=233388342

Comment 24

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

The developer tools button is still present for the failure on beta: https://treeherder.mozilla.org/logviewer.html#?job_id=233916718&repo=mozilla-beta / https://taskcluster-artifacts.net/d8XlIq4eSoeeon0Zv3xfyQ/0/public/test_info//mozilla-test-fail-screenshot_gPgsvA.png

Gijs, can you take a look at the code for the removal of the button and the toolbar reset later, please?

Comment 25

[:Gijs (he/him)]

(In reply to Sebastian Hengst [:aryx] (needinfo on intermittent or backout) from comment #24)

The developer tools button is still present for the failure on beta: https://treeherder.mozilla.org/logviewer.html#?job_id=233916718&repo=mozilla-beta / https://taskcluster-artifacts.net/d8XlIq4eSoeeon0Zv3xfyQ/0/public/test_info//mozilla-test-fail-screenshot_gPgsvA.png

Gijs, can you take a look at the code for the removal of the button and the toolbar reset later, please?

The issue is that there's a CUI.reset() in the test already and that re-adds the button. Looking at how best to fix.

Comment 26

[:Gijs (he/him)]

Attachment: Bug 1527922 - fix test for keyboard navigation for devedition, take #3, r?Aryx

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8c0fb16248ee
fix test for keyboard navigation for devedition, take #3, r=aryx

Comment 28

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/8c0fb16248ee

Comment 29

[Intermittent Failures Robot]

27 failures in 2771 pushes (0.01 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• try: 13
• mozilla-beta: 14

Platform breakdown:

• windows10-64-devedition: 6
• macosx64-devedition: 5
• linux64-devedition: 4
• windows7-32-devedition: 7
• linux32-devedition: 5

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1527922&startday=2019-03-11&endday=2019-03-17&tree=all

Comment 30

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Verified fixed with today's beta simulation: https://treeherder.mozilla.org/#/jobs?repo=try&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel%2Crunnable&revision=88ffa9306065a962a3acd1d567aa97e03ac1eb5b

Comment 31

[James Teh [:Jamie]]

Thanks Gijs and :aryx for fixing this. I'm sorry I was so slow to respond; I was away at a conference last week.