Closed Bug 1240368 Opened 5 years ago Closed 5 years ago

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


(DevTools :: Performance Tools (Profiler/Timeline), defect)

Not set


(firefox46 fixed)

Firefox 46
Tracking Status
firefox46 --- fixed


(Reporter: jdescottes, Assigned: jdescottes)




(1 file, 1 obsolete file)

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]

(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

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
Blocks: 1215955
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.
Attached 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 :
Assignee: nobody → jdescottes
Attachment #8709028 - Flags: review?(vporof)
Comment on attachment 8709028 [details] [diff] [review]

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

oh my god why is life so cruel
Attachment #8709028 - Flags: review?(vporof) → review+
Great work Julian!
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?
(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 :
Carry on r+ from previous. Try push successful.
Attachment #8709028 - Attachment is obsolete: true
Attachment #8709203 - Flags: review+
Keywords: checkin-needed
Closed: 5 years ago
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:  Enabling a couple of new framework/ tests in e10s probably pushed some other test using EventUtils into the same chunk.


STR: Not clear.
Developer specific testing

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
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.