Closed Bug 855763 Opened 12 years ago Closed 12 years ago

[Responsive Mode] Intermittent browser_responsiveuiaddcustompreset.js | Test timed out, | Found a tab after previous test timed out: data:text/html,foo, browser_Services.js | Services.prompt is an nsIPromptService

Categories

(DevTools :: General, defect)

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

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
Firefox 23

People

(Reporter: philor, Assigned: msucan)

References

Details

(Keywords: intermittent-failure)

Attachments

(2 files, 1 obsolete file)

updated patch
5.54 KB, patch
jwalker
: review+
Details | Diff | Splinter Review
another patch
1.27 KB, patch
Details | Diff | Splinter Review
+++ This bug was initially created as a clone of Bug #798775 +++ https://bugzilla.mozilla.org/show_bug.cgi?id=798775#c26 philor https://tbpl.mozilla.org/php/getParsedLog.php?id=21188537&tree=Firefox Rev3 WINNT 5.1 mozilla-central opt test mochitest-browser-chrome on 2013-03-27 20:33:14 slave: talos-r3-xp-038 20:51:05 WARNING - TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/responsivedesign/test/browser_responsiveuiaddcustompreset.js | Test timed out 20:51:06 WARNING - TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/responsivedesign/test/browser_responsiveuiaddcustompreset.js | Found a tab after previous test timed out: data:text/html,foo 20:56:29 WARNING - TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/content/tests/browser/browser_Services.js | Services.prompt is an nsIPromptService https://tbpl.mozilla.org/php/getParsedLog.php?id=21205732&tree=Firefox WINNT 6.2 mozilla-central pgo test mochitest-browser-chrome on 2013-03-28 06:41:11 PDT for push 962f5293f87f slave: t-w864-ix-024 06:55:36 INFO - TEST-PASS | chrome://mochitests/content/browser/browser/devtools/responsivedesign/test/browser_responsiveuiaddcustompreset.js | number of presets didn't change after add preset and cancel 06:55:44 INFO - 1364478944916 Services.DataReporting.Policy DEBUG Next data submission is scheduled in the future: Fri Mar 29 2013 06:48:44 GMT-0700 (Pacific Standard Time) 06:56:05 WARNING - TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/responsivedesign/test/browser_responsiveuiaddcustompreset.js | Test timed out (screenshot) 06:56:06 INFO - INFO TEST-END | chrome://mochitests/content/browser/browser/devtools/responsivedesign/test/browser_responsiveuiaddcustompreset.js | finished in 29989ms 06:56:06 WARNING - TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/responsivedesign/test/browser_responsiveuiaddcustompreset.js | Found a tab after previous test timed out: data:text/html,foo 06:56:06 INFO - ReferenceError: info is not defined: restart@chrome://mochitests/content/browser/browser/devtools/responsivedesign/test/browser_responsiveuiaddcustompreset.js:80 06:56:06 INFO - @resource:///modules/devtools/EventEmitter.jsm:56 06:56:06 INFO - EventEmitter_emit@resource:///modules/devtools/EventEmitter.jsm:100 06:56:06 INFO - RUI_unload@resource://app/modules/devtools/responsivedesign.jsm:240 06:56:06 INFO - @resource://app/modules/devtools/responsivedesign.jsm:264 06:56:06 INFO - _beginRemoveTab@chrome://browser/content/tabbrowser.xml:1668 06:56:06 INFO - removeTab@chrome://browser/content/tabbrowser.xml:1563 06:56:06 INFO - Tester_waitForWindowsState@chrome://mochikit/content/browser-test.js:138 06:56:06 INFO - Tester_nextTest@chrome://mochikit/content/browser-test.js:344 06:56:06 INFO - @chrome://mochikit/content/browser-test.js:435 06:56:06 INFO - TEST-INFO | checking window state 06:56:06 INFO - TEST-INFO | chrome://mochitests/content/browser/browser/devtools/responsivedesign/test/browser_responsiveuiaddcustompreset.js | Console message: [JavaScript Error: "ReferenceError: info is not defined: restart@chrome://mochitests/content/browser/browser/devtools/responsivedesign/test/browser_responsiveuiaddcustompreset.js:80 06:56:06 INFO - @resource:///modules/devtools/EventEmitter.jsm:56 06:56:06 INFO - EventEmitter_emit@resource:///modules/devtools/EventEmitter.jsm:100 06:56:06 INFO - RUI_unload@resource://app/modules/devtools/responsivedesign.jsm:240 06:56:06 INFO - @resource://app/modules/devtools/responsivedesign.jsm:264 06:56:06 INFO - _beginRemoveTab@chrome://browser/content/tabbrowser.xml:1668 06:56:06 INFO - removeTab@chrome://browser/content/tabbrowser.xml:1563 06:56:06 INFO - Tester_waitForWindowsState@chrome://mochikit/content/browser-test.js:138 06:56:06 INFO - Tester_nextTest@chrome://mochikit/content/browser-test.js:344 06:56:06 INFO - @chrome://mochikit/content/browser-test.js:435 06:56:06 INFO - " {file: "resource:///modules/devtools/EventEmitter.jsm" line: 105}] 07:00:15 WARNING - TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/toolkit/content/tests/browser/browser_Services.js | Services.prompt is an nsIPromptService
Mihai, can you look into this please?
Flags: needinfo?(mihai.sucan)
(In reply to Ryan VanderMeulen [:RyanVM] from comment #40) > Mihai, can you look into this please? I will look into this as soon as possible. I'll try this week or next week.
Assignee: nobody → mihai.sucan
Status: NEW → ASSIGNED
Flags: needinfo?(mihai.sucan)
Attached patch proposed fix (obsolete) — — Splinter Review
I was able to reproduce intermittent failures on my system which meant it's easier for me to fix this. I made a number of changes: - the test was slightly too eager to do things after responsive mode enable/disable. I added some executeSoons. - there was at least one missing listener for responsive mode enable/disable. - the key synthesizer keeps failing intermittently for me. I just switched that to doCommand(). This test is not about checking if xul:key elements work, or if the synthesizeKey() function works. We can assume that if the key.doCommand() method works the actual keyboard shortcut also works. No more failures on my system. I'll land this in inbound once I get r+. Please let me know if I broke anything. Thanks!
Attachment #739099 - Flags: review?(jwalker)
Comment on attachment 739099 [details] [diff] [review] proposed fix Review of attachment 739099 [details] [diff] [review]: ----------------------------------------------------------------- I think we should be more careful about adding executeSoon, it's a trap waiting to happen. Do you know what we're waiting for for each of the added executeSoons or is it just a 'maybe this will help'?
Attachment #739099 - Flags: review?(jwalker)
Attached patch updated patch — — Splinter Review
(In reply to Joe Walker [:jwalker] from comment #47) > Comment on attachment 739099 [details] [diff] [review] > proposed fix > > Review of attachment 739099 [details] [diff] [review]: > ----------------------------------------------------------------- > > I think we should be more careful about adding executeSoon, it's a trap > waiting to happen. Agreed. The attempt here was not to use executeSoon() as a way to delay things for an undefined amount of time. It looked to me that layout wasn't flushed, or that responsive mode still had other things to do after emitting "on" and "off" events. > Do you know what we're waiting for for each of the added > executeSoons or is it just a 'maybe this will help'? I updated the patch and picked more judiciously which executeSoons I add. I add one executeSoon before each responsive mode toggle. Each executeSoon is now commented. Also, do note the test had two executeSoon calls to get out of the current event loop, but it is not consistent about it. It also lacked on/off event listeners in a couple of cases. This patch only makes init/destroy consistent through out the test. I looked in the responsive mode code and I did not see any other events we can rely on. Given that responsive mode code does DOM changes during init/destroy we could only rely on DOM events (which is flimsy), mozRequestAnimationFrame or executeSoon. Am I missing anything? Please let me know if you have any suggestions. Thank you!
Attachment #739099 - Attachment is obsolete: true
Attachment #739522 - Flags: review?(jwalker)
Comment on attachment 739522 [details] [diff] [review] updated patch Review of attachment 739522 [details] [diff] [review]: ----------------------------------------------------------------- Thanks!
Attachment #739522 - Flags: review?(jwalker) → review+
Thank you for the review. Landed in inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/474291134a4a
https://hg.mozilla.org/mozilla-central/rev/474291134a4a
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 23
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch another patch — — Splinter Review
This test is still failing. Here's a patch that tries to fix the intermittent failure again. Approach: - on my system I can no longer reproduce the intermittent failures. - to reproduce them I removed all the executeSoon() calls and tried to find a way to fix the failures. - waitForFocus(), wait for DOM events, mozRequestAnimationFrame() did not work. - what worked is in this patch: force a document reflow by reading document.height. - this patch is minimal: I did not remove the executeSoon() calls, I only added the line which forces the reflow - to avoid other surprises. Not much point in asking for review, it's a one-liner. Going to land this in inbound.
https://hg.mozilla.org/mozilla-central/rev/576ff91b64d7
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
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: