Closed Bug 966424 Opened 11 years ago Closed 11 years ago

Intermittent browser_bug765105_background_image_tooltip.js | Test timed out or Tooltip contains an image - Got 0, expected 1

Categories

(DevTools :: Inspector, defect)

Product:
DevTools
Component:
Inspector
Platform:
x86_64
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox29 wontfix, firefox30 fixed, firefox31 fixed, firefox-esr24 unaffected)

Status:
RESOLVED FIXED
Milestone:
Firefox 31
Tracking Flags:
Tracking Status
firefox29 --- wontfix
firefox30 --- fixed
firefox31 --- fixed
firefox-esr24 --- unaffected

People

(Reporter: RyanVM, Assigned: pbro)

References

Details

(Keywords: intermittent-failure)

Attachments

(2 files, 3 obsolete files)

bug966424-intermittent-browser_bug765105_background_image_tooltip.js v2.patch
47.37 KB, patch
pbro
: review+
lsblakk
: approval-mozilla-aurora+
Details | Diff | Splinter Review
bug966424-intermittent-Removed setTimeout during image download for tests.patch
4.28 KB, patch
pbro
: review+
Details | Diff | Splinter Review
https://tbpl.mozilla.org/php/getParsedLog.php?id=33883475&tree=Fx-Team Ubuntu VM 12.04 x64 fx-team opt test mochitest-browser-chrome on 2014-01-31 07:22:37 PST for push 671618886656 slave: tst-linux64-spot-066 08:06:16 INFO - TEST-START | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js 08:06:16 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js | Console message: [JavaScript Error: "The character encoding of the HTML document was not declared. The document will render with garbled text in some browser configurations if the document contains characters from outside the US-ASCII range. The character encoding of the page must be declared in the document or in the transfer protocol." {file: "data:text/html,rule%20view%20tooltip%20test" line: 0}] 08:06:17 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js | Console message: [JavaScript Warning: "XUL box for hbox element contained an inline span child, forcing all its children to be wrapped in a block." {file: "resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/highlighter.js" line: 351}] 08:06:17 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js | Console message: [JavaScript Warning: "Expected color but found 'physical'. Error in parsing value for 'color'. Declaration dropped." {file: "resource://gre-resources/hiddenWindow.html" line: 0 column: 0 source: "physical"}] 08:06:17 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js | Console message: [JavaScript Warning: "Expected color but found 'physical'. Error in parsing value for 'color'. Declaration dropped." {file: "resource://gre-resources/hiddenWindow.html" line: 0 column: 0 source: "physical"}] 08:06:17 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js | Console message: [JavaScript Warning: "Expected color but found 'physical'. Error in parsing value for 'color'. Declaration dropped." {file: "resource://gre-resources/hiddenWindow.html" line: 0 column: 0 source: "physical"}] 08:06:17 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js | Console message: [JavaScript Warning: "Expected color but found 'physical'. Error in parsing value for 'color'. Declaration dropped." {file: "resource://gre-resources/hiddenWindow.html" line: 0 column: 0 source: "physical"}] 08:06:17 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js | Console message: [JavaScript Warning: "Expected color but found 'physical'. Error in parsing value for 'color'. Declaration dropped." {file: "resource://gre-resources/hiddenWindow.html" line: 0 column: 0 source: "physical"}] 08:06:17 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js | Console message: [JavaScript Warning: "Expected color but found 'physical'. Error in parsing value for 'color'. Declaration dropped." {file: "resource://gre-resources/hiddenWindow.html" line: 0 column: 0 source: "physical"}] 08:06:17 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js | Console message: [JavaScript Warning: "Expected color but found 'physical'. Error in parsing value for 'color'. Declaration dropped." {file: "resource://gre-resources/hiddenWindow.html" line: 0 column: 0 source: "physical"}] 08:06:17 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js | Console message: [JavaScript Warning: "Expected color but found 'physical'. Error in parsing value for 'color'. Declaration dropped." {file: "resource://gre-resources/hiddenWindow.html" line: 0 column: 0 source: "physical"}] 08:06:17 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js | Console message: [JavaScript Warning: "Expected color but found 'repeat-y'. Error in parsing value for 'color'. Declaration dropped." {file: "resource://gre-resources/hiddenWindow.html" line: 0 column: 0 source: "repeat-y"}] 08:06:17 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js | Console message: [JavaScript Warning: "Expected color but found 'repeat-y'. Error in parsing value for 'color'. Declaration dropped." {file: "resource://gre-resources/hiddenWindow.html" line: 0 column: 0 source: "repeat-y"}] 08:06:17 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js | Console message: [JavaScript Warning: "Expected color but found '-y'. Error in parsing value for 'color'. Declaration dropped." {file: "resource://gre-resources/hiddenWindow.html" line: 0 column: 0 source: "-y"}] 08:06:17 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js | Console message: [JavaScript Warning: "Expected color but found '-y'. Error in parsing value for 'color'. Declaration dropped." {file: "resource://gre-resources/hiddenWindow.html" line: 0 column: 0 source: "-y"}] 08:06:17 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js | Console message: [JavaScript Warning: "Expected color but found 'y'. Error in parsing value for 'color'. Declaration dropped." {file: "resource://gre-resources/hiddenWindow.html" line: 0 column: 0 source: "y"}] 08:06:17 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js | Console message: [JavaScript Warning: "Expected color but found 'y'. Error in parsing value for 'color'. Declaration dropped." {file: "resource://gre-resources/hiddenWindow.html" line: 0 column: 0 source: "y"}] 08:06:17 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js | Console message: [JavaScript Warning: "Expected color but found 'right'. Error in parsing value for 'color'. Declaration dropped." {file: "resource://gre-resources/hiddenWindow.html" line: 0 column: 0 source: "right"}] 08:06:17 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js | Console message: [JavaScript Warning: "Expected color but found 'right'. Error in parsing value for 'color'. Declaration dropped." {file: "resource://gre-resources/hiddenWindow.html" line: 0 column: 0 source: "right"}] 08:06:17 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js | Console message: [JavaScript Warning: "Expected color but found 'top'. Error in parsing value for 'color'. Declaration dropped." {file: "resource://gre-resources/hiddenWindow.html" line: 0 column: 0 source: "top"}] 08:06:17 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js | Console message: [JavaScript Warning: "Expected color but found 'top'. Error in parsing value for 'color'. Declaration dropped." {file: "resource://gre-resources/hiddenWindow.html" line: 0 column: 0 source: "top"}] 08:06:17 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js | Console message: [JavaScript Warning: "Error in parsing value for 'top'. Declaration dropped." {file: "chrome://browser/content/devtools/spectrum-frame.xhtml" line: 0 column: 0 source: "NaNpx"}] 08:06:17 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js | Console message: [JavaScript Warning: "Error in parsing value for 'left'. Declaration dropped." {file: "chrome://browser/content/devtools/spectrum-frame.xhtml" line: 0 column: 0 source: "NaNpx"}] 08:06:17 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js | Console message: [JavaScript Warning: "Error in parsing value for 'top'. Declaration dropped." {file: "chrome://browser/content/devtools/spectrum-frame.xhtml" line: 0 column: 0 source: "NaNpx"}] 08:06:17 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js | Console message: [JavaScript Warning: "Error in parsing value for 'left'. Declaration dropped." {file: "chrome://browser/content/devtools/spectrum-frame.xhtml" line: 0 column: 0 source: "NaNpx"}] 08:06:17 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js | Console message: [JavaScript Warning: "Expected color but found 'repeat-y'. Error in parsing value for 'color'. Declaration dropped." {file: "resource://gre-resources/hiddenWindow.html" line: 0 column: 0 source: "repeat-y"}] 08:06:17 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js | Console message: [JavaScript Warning: "Expected color but found 'repeat-y'. Error in parsing value for 'color'. Declaration dropped." {file: "resource://gre-resources/hiddenWindow.html" line: 0 column: 0 source: "repeat-y"}] 08:06:17 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js | Console message: [JavaScript Warning: "Expected color but found '-y'. Error in parsing value for 'color'. Declaration dropped." {file: "resource://gre-resources/hiddenWindow.html" line: 0 column: 0 source: "-y"}] 08:06:17 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js | Console message: [JavaScript Warning: "Expected color but found '-y'. Error in parsing value for 'color'. Declaration dropped." {file: "resource://gre-resources/hiddenWindow.html" line: 0 column: 0 source: "-y"}] 08:06:17 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js | Console message: [JavaScript Warning: "Expected color but found 'y'. Error in parsing value for 'color'. Declaration dropped." {file: "resource://gre-resources/hiddenWindow.html" line: 0 column: 0 source: "y"}] 08:06:17 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js | Console message: [JavaScript Warning: "Expected color but found 'y'. Error in parsing value for 'color'. Declaration dropped." {file: "resource://gre-resources/hiddenWindow.html" line: 0 column: 0 source: "y"}] 08:06:46 INFO - Xlib: extension "RANDR" missing on display ":0". 08:06:48 WARNING - TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js | Test timed out 08:06:48 INFO - JavaScript strict warning: resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/markupview/markup-view.js, line 1575: reference to undefined property this._onMouseOver 08:06:48 INFO - TEST-INFO | MEMORY STAT vsize after test: 1597276160 08:06:48 INFO - TEST-INFO | MEMORY STAT residentFast after test: 479866880 08:06:48 INFO - TEST-INFO | MEMORY STAT heapAllocated after test: 181999936 08:06:48 INFO - INFO TEST-END | chrome://mochitests/content/browser/browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js | finished in 30391ms
Starring for this bug seems to have moved to bug 973556 after February 15, though I don't see the difference between the failures.
Summary: Intermittent browser_bug765105_background_image_tooltip.js | Test timed out → Intermittent browser_bug765105_background_image_tooltip.js | Test timed out or Tooltip contains an image - Got 0, expected 1
Patrick, can you please take a look at this frequent orange? Thanks :)
Flags: needinfo?(pbrosset)
Started to refactor a little bit the way the tooltip works so that the API consistently returns a promise when the content is ready. I can now change tests that deal with tooltips.
Assignee: nobody → pbrosset
Flags: needinfo?(pbrosset)
Blocks: 979292
One thing I'm going to do now is not show tooltip panels during tests. Most tests are concerned about making sure a given tooltip type is associated to a given anchor and validating the tooltip's content. There's no need to actually show the tooltip to achieve this. Bugs related to panels not actually appearing or hiding, or being placed at incorrect positions are very often XUL bugs, so let's reduce the chances of intermittent failure but not showing panels when possible.
Here's a first attempt at fixing this intermittent, and generally making the "showOnHover" type tooltips more predictable. Ongoing try build https://tbpl.mozilla.org/?tree=Try&rev=4c915d5668bf I'll first relaunch a few try builds to see if the intermittent goes away and ask for review later.
I'm unfortunately off on PTO for a week, so won't be able to land this fix right now, but the try build looks quite promising so far: https://tbpl.mozilla.org/?tree=Try&rev=4c915d5668bf I think the patch is worth reviewing and landing.
Attachment #8386652 - Flags: review?(jwalker)
Same patch as before, just flagged as "patch" so it can be reviewed.
Attachment #8386652 - Attachment is obsolete: true
Attachment #8386652 - Flags: review?(jwalker)
Attachment #8387012 - Flags: review?(jwalker)
Joe, this review has been in your queue for almost a week now. Can we please get a priority bump for this frequent orange? :)
Flags: needinfo?(jwalker)
Comment on attachment 8387012 [details] [diff] [review] bug966424-intermittent-browser_bug765105_background_image_tooltip.js.patch Review of attachment 8387012 [details] [diff] [review]: ----------------------------------------------------------------- Generally speaking I think we need to be careful about what happens to exceptions ::: browser/devtools/styleinspector/test/browser_bug765105_background_image_tooltip.js @@ +125,2 @@ > brace.click(); > + return def.promise; Generally speaking I think we need to worry about exceptions more; there's a test timeout waiting to happen here. I'm not sure this patch is the place to fix it though.
Attachment #8387012 - Flags: review?(jwalker) → review+
Sorry for the delay.
Flags: needinfo?(jwalker)
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/20e22a0b38f6 Thanks :)
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
Test failures were due to a new test that has been added after I attached my patch but before it landed. This new test uses a function that my patch removed. Going to attach a new patch soon.
This new version of the patch just corrects the test failures as explained in my previous comment. New try build: https://tbpl.mozilla.org/?tree=Try&rev=0f835669f069
Attachment #8387012 - Attachment is obsolete: true
Attachment #8392810 - Flags: review+
There's a bunch of new test timeouts in the try build which weren't there at the time I uploaded my first patch. These timeouts aren't due to my changes (if anything, my changes actually speed up some of the tests since they avoid waiting for tooltips to be shown), and I suspect they are due to the new box-model highlighter which landed recently and which cleaned up a lot of tests in a way that they now wait for the right highlighter events. So we need to work on splitting tests up (there's a test refactoring/clean up bug that I've started working on which could be used for that: bug 968727).
Fixed in fx-team https://hg.mozilla.org/integration/fx-team/rev/6fc38f43ea3d
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/6fc38f43ea3d
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 31
Please request Aurora approval for this (it applies cleanly). Also, if possible, can we get this on beta too (doesn't apply so cleanly :P)?
status-firefox29: --- → affected
status-firefox30: --- → affected
status-firefox31: --- → fixed
status-firefox-esr24: --- → unaffected
Flags: needinfo?(pbrosset)
Comment on attachment 8392810 [details] [diff] [review] bug966424-intermittent-browser_bug765105_background_image_tooltip.js v2.patch [Approval Request Comment] Bug caused by (feature/regressing bug #): This bug was caused by mochitest browser_bug765105_background_image_tooltip.js which intermittently failed due to the way it tested the appearance of a XUL panel in the devtools. User impact if declined: No user impact if declined, this is only a fix that avoids intermittent test failures. Testing completed (on m-c, etc.): Fix has been on fx-team for severa days and on m-c for 1 day and seems to be getting rid of the issue so far. The last tbpl report in comment 127 is actually not the same problem, it's a test timeout which we've been having a lot more these days in the devtools. But that's a different bug. Risk to taking this patch (and alternatives if risky): The devtools tooltip module was refactored a little bit to fix this bug, so any mistake hidden in there would result in regressions in the image tooltips displayed in the devtools' inspector. Having said this, all tests passed, and the fixed has been on fx-team for several days, the risk is very minimal. String or IDL/UUID changes made by this patch: None
Attachment #8392810 - Flags: approval-mozilla-aurora?
Flags: needinfo?(pbrosset)
(In reply to TBPL Robot from comment #131) :(
:( indeed ... I see 2 problems still: - the test timeouts if there's an error somewhere: this should be solved by adding an error handler on the various tasks of this test to make sure it fails as soon as there's an error (in this case the error is that no image was found and so, trying to access images[0] fails). - the fact that, sometimes, no image is found seems to come from the fact that the test machine, being under heavy stress, doesn't manage to download the image within the 500ms timeout. Indeed, image previewed within tooltips are downloaded by the devtools inspector actor, and there's a timeout on the download to avoid waiting for ever when an image can't be downloaded. That timeout is 500ms, and although the image is local (chrome://....) downloading it seems to take more time.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
The failure in comment 139 is again a different one. This time the test fails directly from the start when opening the inspector. During the inpsector's open sequence, sidebar panels are being created by appending iframes. The test we use to know if one of these iframes is already loaded isn't correct because it may return true before the iframe has loaded. Having said that, I don't think this part has ever failed before. Why is it that test run so slowly these days?
New tentative fix for this intermittently failing test. Here is a list of things I did: - Commented out the protocol logs. This isn't directly related but could slow down even more the test run, and we talked about it in the team. - In test mode (i.e. when gDevTools.testing is true), the inspector actor will not use a timeout in its getImageDataFromURL method, this is what was failing most often. - Finally, I added a little fix to styleinspector/test/head.js as, in slow environment, the openComputedView and openRuleView were failing. Ongoing try build: https://tbpl.mozilla.org/?tree=Try&rev=8670da6fbf14
Attachment #8395582 - Flags: review?(mratcliffe)
Triage drive-by: waiting for more info on the second patch from comment 141 before approving anything for uplift.
Comment on attachment 8395582 [details] [diff] [review] bug966424-intermittent-browser_bug765105_background_image_tooltip.js-NEW-FIX.patch Review of attachment 8395582 [details] [diff] [review]: ----------------------------------------------------------------- Nice! I suppose it makes sense to use gDevTools.testing wherever we use setTimeout as otherwise the same issue will crop up again. r++
Attachment #8395582 - Flags: review?(mratcliffe) → review+
Thanks Mike. There are xpcshell test failures in the try build though. I'll get to them in a minute.
Same patch that Mike R+'d before except that it gets rid of the xpcshell test failures that appeared in the lat try build. New try build: https://tbpl.mozilla.org/?tree=Try&rev=259b3cbc65b6
Attachment #8395582 - Attachment is obsolete: true
Attachment #8398546 - Flags: review+
Comment on attachment 8392810 [details] [diff] [review] bug966424-intermittent-browser_bug765105_background_image_tooltip.js v2.patch worth a shot - i trust we'll notice if there are, in fact, regressions here to tooltip.
Attachment #8392810 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
The second patch (the one that removes the setTimeout) is now in fx-team: https://hg.mozilla.org/integration/fx-team/rev/c77ff7b59bd4
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/c77ff7b59bd4
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Comment 171 seems to show that even with my 2 patches, there are still times when this test fails. It now seems to fail a little bit differently, in the styleinspector/test/head.js in the function that opens up the inspector's rule-view. It seems pointless to try and fix that function as is knowing that I'm working on bug 988313 in which I re-wrote that function from scratch.
Seems unlikely we're going to get this backported to beta29. But I think we should still try to get this bug (and with any luck, bug 988313) onto Aurora30. Sound reasonable, Patrick?
status-firefox29: affected → wontfix
Depends on: 988313
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: