Closed Bug 1001821 Opened 6 years ago Closed 5 years ago

Intermittent - browser_eyedropper_cmd.js leaks a window when run as a standalone directory

Categories

(DevTools :: Inspector, defect)

defect
Not set

Tracking

(firefox36 fixed, firefox37 fixed, firefox38 fixed, firefox-esr31 unaffected)

RESOLVED FIXED
Firefox 38
Tracking Status
firefox36 --- fixed
firefox37 --- fixed
firefox38 --- fixed
firefox-esr31 --- unaffected

People

(Reporter: jmaher, Assigned: sjakthol)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

this is a new test (bug 997163) and while working on bug 992911, I see this leaking windows:

here is a link to a log:
https://tbpl.mozilla.org/php/getParsedLog.php?id=38523292&tree=Try
this doesn't seem to be a problem anymore, we now run devtools with --run-by-dir
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → WORKSFORME
now that we run --run-by-dir live, I guess this is intermittent more than I saw on my retriggers on try.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Summary: new test browser_eyedropper_cmd.js leaks a window on linux debug when run as a standalone directory → Intermittent - browser_eyedropper_cmd.js leaks a window on linux debug when run as a standalone directory
Looks like this is happening with pretty good frequency across all platforms since mochitest-dt run-by-dir landed.
Flags: needinfo?(fayearthur)
OS: Linux → All
Summary: Intermittent - browser_eyedropper_cmd.js leaks a window on linux debug when run as a standalone directory → Intermittent - browser_eyedropper_cmd.js leaks a window when run as a standalone directory
When the test simulates a click eyedropper copies a color to clipboard and test starts to tear down the toolbar and tab. Eyedropper destroys itself once the color is copied BUT it's done after a timeout. Thus in some cases the test finishes before eyedropper is destroyed causing the frame to leak.

This patch makes the test to wait for eyedropper to destroy itself before it starts to tear down the test.

Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=496acb78ca3a
Assignee: nobody → sjakthol
Status: REOPENED → ASSIGNED
Attachment #8550432 - Flags: review?(mratcliffe)
Can we maybe find a different reviewer for this? This is a top orange on trunk and we're coming up on a week without activity now.
Flags: needinfo?(sjakthol)
Comment on attachment 8550432 [details] [diff] [review]
eyedropper-intermittent-leak.patch

Patrick reviewed the initial landing of eyedropper so he should be able to review this. Patrick?
Flags: needinfo?(sjakthol)
Attachment #8550432 - Flags: review?(mratcliffe) → review?(pbrosset)
Comment on attachment 8550432 [details] [diff] [review]
eyedropper-intermittent-leak.patch

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

Yep, that looks very good. Your analysis of the problem seems correct and the fix also. Try is green too, let's land this.

::: browser/devtools/eyedropper/test/browser_eyedropper_cmd.js
@@ +32,5 @@
>  
>  function inspectAndWaitForCopy() {
> +  let copied = waitForClipboard(() => {}, DIV_COLOR);
> +  let ready = inspectPage(); // resolves once eyedropper is destroyed
> +  

nit: trailing whitespaces here.
Attachment #8550432 - Flags: review?(pbrosset) → review+
https://hg.mozilla.org/mozilla-central/rev/3ca92178aac7
Status: ASSIGNED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 38
Flags: needinfo?(fayearthur)
Flags: qe-verify-
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.