Closed Bug 1173196 Opened 9 years ago Closed 9 years ago

Intermittent browser_toolbox_minimize.js | Test timed out

Categories

(DevTools :: Framework, defect)

Product:
DevTools
Component:
Framework
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

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

Status:
RESOLVED FIXED
Milestone:
Firefox 41
Tracking Flags:
Tracking Status
firefox39 --- unaffected
firefox40 --- unaffected
firefox41 --- fixed
firefox-esr31 --- unaffected
firefox-esr38 --- unaffected

People

(Reporter: KWierso, Assigned: sjakthol)

Details

(Keywords: intermittent-failure)

Attachments

(2 files)

bug-1173196-minimize-intermittent.patch
2.10 KB, patch
pbro
: review+
sjakthol
: checkin+
Details | Diff | Splinter Review
bug-1173196-minimize-intermittent-try-2.patch
1.98 KB, patch
pbro
: review+
Details | Diff | Splinter Review 
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
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
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 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+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/c6eda9b9f296
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox41: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Is this something that would be useful to backport?
status-firefox39: --- → ?
status-firefox40: --- → ?
status-firefox-esr31: --- → unaffected
status-firefox-esr38: --- → unaffected
Flags: needinfo?(sjakthol)
I think this just landed so it's only in Nightly.
status-firefox39: ? → unaffected
status-firefox40: ? → unaffected
Flags: needinfo?(sjakthol)
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 → ---
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)
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+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/7a7b4d0dc565
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
status-firefox41: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: