Intermittent browser_toolbox_minimize.js | Test timed out

RESOLVED FIXED in Firefox 41

Status

()

Firefox
Developer Tools: Framework
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: KWierso, Assigned: Sami Jaktholm)

Tracking

({intermittent-failure})

unspecified
Firefox 41
intermittent-failure
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox39 unaffected, firefox40 unaffected, firefox41 fixed, firefox-esr31 unaffected, firefox-esr38 unaffected)

Details

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
12:24:10 INFO - MEMORY STAT heapAllocated after test: 80936820
12:24:10 INFO - 847 INFO TEST-OK | browser/devtools/framework/test/browser_toolbox_hosts_size.js | took 3832ms
12:24:10 INFO - 848 INFO TEST-START | browser/devtools/framework/test/browser_toolbox_minimize.js
12:25:40 INFO - Xlib: extension "RANDR" missing on display ":0".
12:25:42 INFO - TEST-INFO | screentopng: exit 0
12:25:42 INFO - 849 INFO checking window state
12:25:42 INFO - 850 INFO Entering test
12:25:42 INFO - 851 INFO Create a test tab and open the toolbox
12:25:42 INFO - 852 INFO Adding a new tab with URL: 'data:text/html;charset=utf8,test page'
12:25:42 INFO - 853 INFO URL 'data:text/html;charset=utf8,test page' loading complete
12:25:42 INFO - 854 INFO TEST-PASS | browser/devtools/framework/test/browser_toolbox_minimize.js | The minimize button exists in the default bottom host
12:25:42 INFO - 855 INFO Try to minimize the toolbox
12:25:42 INFO - 856 INFO TEST-PASS | browser/devtools/framework/test/browser_toolbox_minimize.js | The toolbox host has been hidden away with a negative-margin
12:25:42 INFO - 857 INFO Try to maximize again the toolbox
12:25:42 INFO - 858 INFO Console message: 1433877880841 Toolkit.GMP WARN GMPInstallManager.parseResponseXML got node name: html, expected: updates
12:25:42 INFO - 859 INFO Console message: [JavaScript Error: "1433877880842 Toolkit.GMP ERROR GMPInstallManager.simpleCheckAndInstall Could not check for addons: {"target":{},"message":"got node name: html, expected: updates"}" {file: "resource://gre/modules/Log.jsm" line: 749}]
12:25:42 INFO - 860 INFO Console message: 1433877880921 Services.HealthReport.HealthReporter WARN Saved state file does not exist.
12:25:42 INFO - 861 INFO TEST-UNEXPECTED-FAIL | browser/devtools/framework/test/browser_toolbox_minimize.js | Test timed out - expected PASS
12:25:42 INFO - MEMORY STAT vsize after test: 638693376
12:25:42 INFO - MEMORY STAT residentFast after test: 196718592
12:25:42 INFO - MEMORY STAT heapAllocated after test: 83923480
12:25:42 INFO - 862 INFO TEST-OK | browser/devtools/framework/test/browser_toolbox_minimize.js | took 90162ms
12:25:42 INFO - 863 INFO TEST-START | browser/devtools/framework/test/browser_toolbox_options.js
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
I don't understand what's happening exactly but it seems like the ontransitionend event listener used in toolbox-host.js to detect when the host becomes maximized again doesn't work.
I've tried just adding the equivalent of an executeSoon after the minimized and maximized events were received, in the test, and that seems to work: 
https://treeherder.mozilla.org/#/jobs?repo=try&revision=666f4f6c73e2
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 116

3 years ago
Created attachment 8621948 [details] [diff] [review]
bug-1173196-minimize-intermittent.patch

Adding the listener before setting the margin seems to fix this.

I think that when the margin is set the browser triggers computation (i.e. GC or something) that exceeds the time it takes to do the transition. This causes 'transitionend' event to come before the script execution continues and the handler is added.

Waiting for a tick gives the browser a chance to do the computation earlier so it won't block the script execution when the margin is set.

As a more general thought, it's always a good idea to attach a listener for an event before you execute the code that triggers the event.

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=98b02dcc5098
Assignee: nobody → sjakthol
Status: NEW → ASSIGNED
Attachment #8621948 - Flags: review?(pbrosset)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment on attachment 8621948 [details] [diff] [review]
bug-1173196-minimize-intermittent.patch

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

Oh, great catch, of course that's the problem. Thanks for finding that out.
Attachment #8621948 - Flags: review?(pbrosset) → review+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
https://hg.mozilla.org/mozilla-central/rev/c6eda9b9f296
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox41: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Is this something that would be useful to backport?
status-firefox39: --- → ?
status-firefox40: --- → ?
status-firefox-esr31: --- → unaffected
status-firefox-esr38: --- → unaffected
Flags: needinfo?(sjakthol)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 183

3 years ago
I think this just landed so it's only in Nightly.
status-firefox39: ? → unaffected
status-firefox40: ? → unaffected
Flags: needinfo?(sjakthol)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 187

3 years ago
Well, that didn't help much. This was the easy fix, now it starts to get more interesting. However, I don't really have time to look at this during the next few days so lets hope that the patch here helped even a little.
Status: RESOLVED → REOPENED
status-firefox41: fixed → ---
Resolution: FIXED → ---
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Comment 214

3 years ago
Created attachment 8623652 [details] [diff] [review]
bug-1173196-minimize-intermittent-try-2.patch

Another idea I had that was that the code accepts a transitionend event for all properties - not just 'margin-bottom' it triggers. So I did a try run with a debugging patch and it revealed that indeed, the first 'minimize' event fired too soon due to a transitionend event for 'backgroud-color' [1]. This apparently caused something to go wrong when the test continued to maximize the toolbox again.

This patch makes the code to ignore transitionend events for properties other than margin-bottom.

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

[1] http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/sjakthol@outlook.com-43814757d8e0/try-win32/try_win7-ix_test-mochitest-devtools-chrome-bm112-tests1-windows-build843.txt.gz
Attachment #8623652 - Flags: review?(pbrosset)
(Assignee)

Updated

3 years ago
Attachment #8621948 - Flags: checkin+
Comment on attachment 8623652 [details] [diff] [review]
bug-1173196-minimize-intermittent-try-2.patch

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

Cool. Thanks Sami.
Attachment #8623652 - Flags: review?(pbrosset) → review+
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
https://hg.mozilla.org/mozilla-central/rev/7a7b4d0dc565
Status: REOPENED → RESOLVED
Last Resolved: 3 years ago3 years ago
status-firefox41: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
You need to log in before you can comment on or make changes to this bug.