browser_profiler_tree-abstract-01.js & 02.js cause intermittent failures when run in same suite

RESOLVED FIXED in Firefox 46

Status

RESOLVED FIXED
3 years ago
9 months ago

People

(Reporter: jdescottes, Assigned: jdescottes)

Tracking

unspecified
Firefox 46
x86_64
Linux

Firefox Tracking Flags

(firefox46 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

3 years ago
STRs :
- pre : use optimized build
- pre : use Linux 64 bits
- pre : remember to run tests in e10s mode
- run several times ./mach mochitest --e10s devtools/performance/test/browser_profiler_tree-abstract-0*


Actual : intermittent failures in browser_profiler_tree-abstract-02.js :
- "The root node is now focused. - Got null, expected [object XULElement]
"
- "The 'baz' node is now focused. - Got null, expected [object XULElement]
"

See https://treeherder.mozilla.org/#/jobs?repo=try&revision=9fa903f52c0d
(the failure rate on try is ~75%, on a local computer it is much lower : ~10% on my VM)
 
Expected : the tests should always pass.

===========================================

This issue was highlighted after landing Bug 1215955, which introduced a new mochitest. This reshuffled the way tests are split between the dt* test suites. tree-abstract-01.js and tree-abstract-02.js used to be run in two different test suites.

===========================================

Investigation :

After running tree-abstract-01 the browser seems to be unable to move the focus to elements created in tree-abstract-02 right away when only calling "someNewElement.focus()". document.commandDispatcher.focusedElement remains null, no matter how long we wait after calling "focus()".

Waiting for a bit _before_ calling "focus()" fixes the issue.

Triggering a "mousedown" on the new container element created in tree-abstract-02 and waiting for document.commandDispatcher.focusedElement to be this container also fixes the issue.
(illustrated by https://treeherder.mozilla.org/#/jobs?repo=try&revision=dda48223de4c)

Interestingly, when starting the tests in e10s mode, document.commandDispatcher.focusedElement is normally NOT null when the test starts. However, it _may_ be null when tree-abstract-02 starts : in this case the intermittent failure will occur. 
Sadly, we can not rely on this to wait for the test to be ready : in normal (non e10s) mode, document.commandDispatcher.focusedElement can be null when the test starts, without any consequence.

tree-abstract-01 is moving the focus to an item which is removed when when the test ends.
Calling (and waiting for) "blur()" on the focused element before ending tress-abstract-01 also seems to fix the problem locally (try push ongoing at https://treeherder.mozilla.org/#/jobs?repo=try&revision=3cea1af3373d)
(Assignee)

Updated

3 years ago
Blocks: 1215955
(Assignee)

Comment 1

3 years ago
I isolated the failure in a test case that doesn't involve any devtools code, and opened a new bug to track this : Bug 1240509.

In the meantime, I propose to skip tree-abstract-02 on linux opt e10s until 1240509 gets fixed.
(Assignee)

Comment 2

3 years ago
Posted patch bug1240368.diff (obsolete) — Splinter Review
Victor, flagging you for review since you seem to know the test I want to skip (performance/test/browser_profiler_tree-abstract-02.js).

As explained in this bug the issue is actually a conflict between browser_profiler_tree-abstract-01 & browser_profiler_tree-abstract-02 (see 1240509).

But since this might cause unrelated changesets to fail builds and be backed out, I prefer to skip the test until we have a proper fix.

Try push : https://treeherder.mozilla.org/#/jobs?repo=try&revision=e87f61b4d40d
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Attachment #8709028 - Flags: review?(vporof)
Comment on attachment 8709028 [details] [diff] [review]
bug1240368.diff

Review of attachment 8709028 [details] [diff] [review]:
-----------------------------------------------------------------

oh my god why is life so cruel
Attachment #8709028 - Flags: review?(vporof) → review+
Great work Julian!
(Assignee)

Comment 5

3 years ago
Thanks for the review. However, as highlighted by the try push, the next  test also relies on focus() and is now failing. We should also skip it :/
Would it make more sense to disable the test using EventUtils so you don't end up playing test disabling whack-a-mole?
(Assignee)

Comment 7

3 years ago
(In reply to Ryan VanderMeulen [:RyanVM] from comment #6)
> Would it make more sense to disable the test using EventUtils so you don't
> end up playing test disabling whack-a-mole?

Good point :) Try push : https://treeherder.mozilla.org/#/jobs?repo=try&revision=db6dc024207a
(Assignee)

Comment 8

3 years ago
Carry on r+ from previous. Try push successful.
Attachment #8709028 - Attachment is obsolete: true
Attachment #8709203 - Flags: review+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed

Comment 10

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/488a52302100
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox46: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
(In reply to Ryan VanderMeulen [:RyanVM] from comment #6)
> Would it make more sense to disable the test using EventUtils so you don't
> end up playing test disabling whack-a-mole?

I think this is turning into whack-a-mole anyway with browser_profiler_tree-abstract-02.js: https://bugzilla.mozilla.org/show_bug.cgi?id=1030318#c48.  Enabling a couple of new framework/ tests in e10s probably pushed some other test using EventUtils into the same chunk.

Comment 12

3 years ago
[bugday-20160323]

Status: RESOLVED,FIXED -> UNVERIFIED

Comments:
STR: Not clear.
Developer specific testing

Component: 
Name			Firefox
Version			46.0b9
Build ID		20160322075646
Update Channel          beta
User Agent		Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
OS			Windows 7 SP1 x86_64

Expected Results: 
Developer specific testing

Actual Results: 
As expected

Updated

9 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.