Intermittent browser_urlbarSearchSingleWordNotification.js | Were expecting to get a notification (and more)

VERIFIED FIXED in Firefox 33

Status

()

defect
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: emorley, Assigned: Gijs)

Tracking

({intermittent-failure})

Trunk
mozilla34
All
Linux
Points:
1
Dependency tree / graph
Bug Flags:
firefox-backlog +
in-testsuite +

Firefox Tracking Flags

(firefox32 unaffected, firefox33 verified, firefox34 verified, firefox-esr24 unaffected, firefox-esr31 unaffected)

Details

Attachments

(1 attachment, 2 obsolete attachments)

This was introduced in the last few days by bug 1042519, but attempting to back that out results in conflicts.

Ubuntu VM 12.04 mozilla-inbound debug test mochitest-browser-chrome-1 on 2014-08-05 23:06:57 PDT for push e9348984ca0e

slave: tst-linux32-spot-611

https://tbpl.mozilla.org/php/getParsedLog.php?id=45309213&tree=Mozilla-Inbound

{
23:21:08     INFO -  10092 INFO TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/general/browser_urlbarSearchSingleWordNotification.js | waitForDocLoadAndStopIt: The expected URL was loaded - waitForDocLoadAndStopIt: The expected URL was loaded
23:21:08     INFO -  10093 INFO TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/general/browser_urlbarSearchSingleWordNotification.js | Pref should have been toggled - Pref should have been toggled
23:21:08     INFO -  10094 INFO waitForDocLoadAndStopIt: Waiting for URL: http://localhost/
23:21:08     INFO -  10095 INFO waitForDocLoadAndStopIt: onStateChange: http://localhost/
23:21:08     INFO -  10096 INFO waitForDocLoadAndStopIt: Document start: http://localhost/
23:21:08     INFO -  10097 INFO TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/general/browser_urlbarSearchSingleWordNotification.js | waitForDocLoadAndStopIt: The expected URL was loaded - waitForDocLoadAndStopIt: The expected URL was loaded
23:21:08     INFO -  10098 INFO Console message: [JavaScript Error: "chrome://browser/content/browser.xul : Unable to run script because scripts are blocked internally."]
23:21:08     INFO -  10099 INFO Console message: [JavaScript Error: "chrome://browser/content/browser.xul : Unable to run script because scripts are blocked internally."]
23:21:08     INFO -  10100 INFO TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/general/browser_urlbarSearchSingleWordNotification.js | We are expecting to not get a notification - We are expecting to not get a notification
23:21:08     INFO -  10101 INFO Leaving test test_navigate_single_host
23:21:08     INFO -  10102 INFO Entering test test_navigate_single_host
23:21:08     INFO -  10103 INFO waitForDocLoadAndStopIt: Waiting for URL: https://www.google.com/search?q=localhost.&ie=utf-8&oe=utf-8&aq=t&rls=org.mozilla:en-US:unofficial&client=firefox-a&channel=fflb
23:21:08     INFO -  10104 INFO waitForDocLoadAndStopIt: onStateChange: https://www.google.com/search?q=localhost.&ie=utf-8&oe=utf-8&aq=t&rls=org.mozilla:en-US:unofficial&client=firefox-a&channel=fflb
23:21:08     INFO -  10105 INFO waitForDocLoadAndStopIt: Document start: https://www.google.com/search?q=localhost.&ie=utf-8&oe=utf-8&aq=t&rls=org.mozilla:en-US:unofficial&client=firefox-a&channel=fflb
23:21:08     INFO -  10106 INFO TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/general/browser_urlbarSearchSingleWordNotification.js | waitForDocLoadAndStopIt: The expected URL was loaded - waitForDocLoadAndStopIt: The expected URL was loaded
23:21:08     INFO -  10107 INFO Console message: [JavaScript Error: "chrome://browser/content/browser.xul : Unable to run script because scripts are blocked internally."]
23:21:08     INFO -  10108 INFO Console message: [JavaScript Error: "chrome://browser/content/browser.xul : Unable to run script because scripts are blocked internally."]
23:21:08     INFO -  10109 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/general/browser_urlbarSearchSingleWordNotification.js | Were expecting to get a notification
23:21:08     INFO -  Stack trace:
23:21:08     INFO -  chrome://mochitests/content/browser/browser/base/content/test/general/head.js:waitForCondition/interval<:67
23:21:08     INFO -  null:null:0 - Were expecting to get a notification
23:21:08     INFO -  Stack trace:
23:21:08     INFO -  chrome://mochitests/content/browser/browser/base/content/test/general/head.js:waitForCondition/interval<:67
23:21:08     INFO -  null:null:0
23:21:08     INFO -  TEST-INFO | expected PASS
23:21:08     INFO -  10110 INFO *************************
23:21:08     INFO -  10111 INFO A coding exception was thrown and uncaught in a Task.
23:21:08     INFO -  10112 INFO Full message: TypeError: notification is null
23:21:08     INFO -  10113 INFO Full stack: test_navigate_single_host@chrome://mochitests/content/browser/browser/base/content/test/general/browser_urlbarSearchSingleWordNotification.js:61:5
23:21:08     INFO -  10114 INFO Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:866:23
23:21:08     INFO -  10115 INFO this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:745:7
23:21:08     INFO -  10116 INFO *************************
23:21:08     INFO -  10117 INFO dumping last 1 message(s)
23:21:08     INFO -  10118 INFO if you need more context, please use SimpleTest.requestCompleteLog() in your test
23:21:08     INFO -  10119 INFO waitForDocLoadAndStopIt: Waiting for URL: http://localhost./
23:21:08     INFO -  10120 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/general/browser_urlbarSearchSingleWordNotification.js | Uncaught exception at chrome://mochitests/content/browser/browser/base/content/test/general/browser_urlbarSearchSingleWordNotification.js:61 - TypeError: notification is null
23:21:08     INFO -  Stack trace:
23:21:08     INFO -      test_navigate_single_host@chrome://mochitests/content/browser/browser/base/content/test/general/browser_urlbarSearchSingleWordNotification.js:61:5
23:21:08     INFO -      Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:866:23
23:21:08     INFO -      this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:745:7
23:21:08     INFO -      Tester_execTest@chrome://mochikit/content/browser-test.js:609:9
23:21:08     INFO -      Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:535:7 - Uncaught exception at chrome://mochitests/content/browser/browser/base/content/test/general/browser_urlbarSearchSingleWordNotification.js:61 - TypeError: notification is null
23:21:08     INFO -  Stack trace:
23:21:08     INFO -      test_navigate_single_host@chrome://mochitests/content/browser/browser/base/content/test/general/browser_urlbarSearchSingleWordNotification.js:61:5
23:21:08     INFO -      Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:866:23
23:21:08     INFO -      this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:745:7
23:21:08     INFO -      Tester_execTest@chrome://mochikit/content/browser-test.js:609:9
23:21:08     INFO -      Tester.prototype.nextTest</<@chrome://mochikit/content/browser-test.js:535:7
23:21:08     INFO -  TEST-INFO | expected PASS
23:21:08     INFO -  10121 INFO ++DOCSHELL 0x737fb000 == 57 [pid = 1762] [id = 964]
23:21:08     INFO -  10122 INFO ++DOMWINDOW == 135 (0x782a6380) [pid = 1762] [serial = 2497] [outer = (nil)]
23:21:08     INFO -  10123 INFO ++DOMWINDOW == 136 (0x782a6d30) [pid = 1762] [serial = 2498] [outer = 0x782a6380]
23:21:08     INFO -  10124 INFO [1762] WARNING: NS_ENSURE_TRUE(mMutable) failed: file /builds/slave/m-in-lx-d-00000000000000000000/build/netwerk/base/src/nsSimpleURI.cpp, line 265
23:21:08     INFO -  10125 INFO [1762] WARNING: NS_ENSURE_TRUE(resultIndex >= 0) failed: file /builds/slave/m-in-lx-d-00000000000000000000/build/toolkit/components/autocomplete/nsAutoCompleteController.cpp, line 1497
23:21:08     INFO -  10126 INFO [1762] WARNING: malformed hostname: file /builds/slave/m-in-lx-d-00000000000000000000/build/netwerk/base/src/nsURLParsers.cpp, line 562
23:21:08     INFO -  10127 INFO dumping last 5 message(s)
23:21:08     INFO -  10128 INFO if you need more context, please use SimpleTest.requestCompleteLog() in your test
23:21:08     INFO -  10129 INFO Leaving test test_navigate_single_host
23:21:08     INFO -  10130 INFO Entering test test_navigate_invalid_url
23:21:08     INFO -  10131 INFO waitForDocLoadAndStopIt: Waiting for URL: https://www.google.com/search?q=mozilla+is+awesome&ie=utf-8&oe=utf-8&aq=t&rls=org.mozilla:en-US:unofficial&client=firefox-a&channel=fflb
23:21:08     INFO -  10132 INFO waitForDocLoadAndStopIt: onStateChange: https://www.google.com/search?q=mozilla+is+awesome&ie=utf-8&oe=utf-8&aq=t&rls=org.mozilla:en-US:unofficial&client=firefox-a&channel=fflb
23:21:08     INFO -  10133 INFO waitForDocLoadAndStopIt: Document start: https://www.google.com/search?q=mozilla+is+awesome&ie=utf-8&oe=utf-8&aq=t&rls=org.mozilla:en-US:unofficial&client=firefox-a&channel=fflb
23:21:08     INFO -  10134 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/general/browser_urlbarSearchSingleWordNotification.js | waitForDocLoadAndStopIt: The expected URL was loaded - Got https://www.google.com/search?q=mozilla+is+awesome&ie=utf-8&oe=utf-8&aq=t&rls=org.mozilla:en-US:unofficial&client=firefox-a&channel=fflb, expected http://localhost./
23:21:08     INFO -  Stack trace:
23:21:08     INFO -      chrome://mochikit/content/browser-test.js:test_is:771
23:21:08     INFO -      chrome://mochitests/content/browser/browser/base/content/test/general/head.js:waitForDocLoadAndStopIt/progressListener.onStateChange:350
23:21:08     INFO -      chrome://browser/content/tabbrowser.xml:_callProgressListeners/<:481
23:21:08     INFO -      self-hosted:forEach:252
23:21:08     INFO -      chrome://browser/content/tabbrowser.xml:_callProgressListeners:478
23:21:08     INFO -      chrome://browser/content/tabbrowser.xml:mTabProgressListener/<._callProgressListeners:541
23:21:08     INFO -      chrome://browser/content/tabbrowser.xml:mTabProgressListener/<.onStateChange:693
23:21:08     INFO -      chrome://global/content/bindings/browser.xml:loadURIWithFlags:148
23:21:08     INFO -      chrome://browser/content/tabbrowser.xml:loadURIWithFlags:2761
23:21:08     INFO -      chrome://browser/content/utilityOverlay.js:openLinkIn:323
23:21:08     INFO -      chrome://browser/content/utilityOverlay.js:openUILinkIn:200
23:21:08     INFO -      chrome://browser/content/urlbarBindings.xml:loadCurrent:313
23:21:08     INFO -      chrome://browser/content/urlbarBindings.xml:continueOperation:359
23:21:08     INFO -      chrome://browser/content/urlbarBindings.xml:handleCommand/<:295
23:21:08     INFO -      chrome://browser/content/urlbarBindings.xml:_canonizeURL/<:424
23:21:08     INFO -      chrome://browser/content/browser.js:getShortcutOrURIAndPostData:11677
23:21:08     INFO -      chrome://browser/content/urlbarBindings.xml:_canonizeURL:423
23:21:08     INFO -      chrome://browser/content/urlbarBindings.xml:handleCommand:290
23:21:08     INFO -      chrome://global/content/bindings/autocomplete.xml line 417 > Function:anonymous:1
23:21:08     INFO -      chrome://global/content/bindings/autocomplete.xml:onTextEntered:224
23:21:08     INFO -      chrome://global/content/bindings/autocomplete.xml:onKeyPress:479
23:21:08     INFO -      chrome://global/content/bindings/autocomplete.xml:onxblkeypress:563
23:21:08     INFO -      chrome://mochikit/content/tests/SimpleTest/specialpowersAPI.js:doApply:150
23:21:08     INFO -      chrome://mochikit/content/tests/SimpleTest/specialpowersAPI.js:wrapPrivileged/callTrap:174
23:21:08     INFO -      chrome://mochikit/content/tests/SimpleTest/EventUtils.js:synthesizeKey:597
23:21:08     INFO -      chrome://mochitests/content/browser/browser/base/content/test/general/browser_urlbarSearchSingleWordNotification.js:runURLBarSearchTest:38
23:21:08     INFO -      chrome://mochitests/content/browser/browser/base/content/test/general/browser_urlbarSearchSingleWordNotification.js:test_navigate_invalid_url:82
23:21:08     INFO -      Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:869:21
23:21:08     INFO -      this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:745:7 - waitForDocLoadAndStopIt: The expected URL was loaded - Got https://www.google.com/search?q=mozilla+is+awesome&ie=utf-8&oe=utf-8&aq=t&rls=org.mozilla:en-US:unofficial&client=firefox-a&channel=fflb, expected http://localhost./
23:21:08     INFO -  Stack trace:
23:21:08     INFO -      chrome://mochikit/content/browser-test.js:test_is:771
23:21:08     INFO -      chrome://mochitests/content/browser/browser/base/content/test/general/head.js:waitForDocLoadAndStopIt/progressListener.onStateChange:350
23:21:08     INFO -      chrome://browser/content/tabbrowser.xml:_callProgressListeners/<:481
23:21:08     INFO -      self-hosted:forEach:252
23:21:08     INFO -      chrome://browser/content/tabbrowser.xml:_callProgressListeners:478
23:21:08     INFO -      chrome://browser/content/tabbrowser.xml:mTabProgressListener/<._callProgressListeners:541
23:21:08     INFO -      chrome://browser/content/tabbrowser.xml:mTabProgressListener/<.onStateChange:693
23:21:08     INFO -      chrome://global/content/bindings/browser.xml:loadURIWithFlags:148
23:21:08     INFO -      chrome://browser/content/tabbrowser.xml:loadURIWithFlags:2761
23:21:08     INFO -      chrome://browser/content/utilityOverlay.js:openLinkIn:323
23:21:08     INFO -      chrome://browser/content/utilityOverlay.js:openUILinkIn:200
23:21:08     INFO -      chrome://browser/content/urlbarBindings.xml:loadCurrent:313
23:21:08     INFO -      chrome://browser/content/urlbarBindings.xml:continueOperation:359
23:21:08     INFO -      chrome://browser/content/urlbarBindings.xml:handleCommand/<:295
23:21:08     INFO -      chrome://browser/content/urlbarBindings.xml:_canonizeURL/<:424
23:21:08     INFO -      chrome://browser/content/browser.js:getShortcutOrURIAndPostData:11677
23:21:08     INFO -      chrome://browser/content/urlbarBindings.xml:_canonizeURL:423
23:21:08     INFO -      chrome://browser/content/urlbarBindings.xml:handleCommand:290
23:21:08     INFO -      chrome://global/content/bindings/autocomplete.xml line 417 > Function:anonymous:1
23:21:08     INFO -      chrome://global/content/bindings/autocomplete.xml:onTextEntered:224
23:21:08     INFO -      chrome://global/content/bindings/autocomplete.xml:onKeyPress:479
23:21:08     INFO -      chrome://global/content/bindings/autocomplete.xml:onxblkeypress:563
23:21:08     INFO -      chrome://mochikit/content/tests/SimpleTest/specialpowersAPI.js:doApply:150
23:21:08     INFO -      chrome://mochikit/content/tests/SimpleTest/specialpowersAPI.js:wrapPrivileged/callTrap:174
23:21:08     INFO -      chrome://mochikit/content/tests/SimpleTest/EventUtils.js:synthesizeKey:597
23:21:08     INFO -      chrome://mochitests/content/browser/browser/base/content/test/general/browser_urlbarSearchSingleWordNotification.js:runURLBarSearchTest:38
23:21:08     INFO -      chrome://mochitests/content/browser/browser/base/content/test/general/browser_urlbarSearchSingleWordNotification.js:test_navigate_invalid_url:82
23:21:08     INFO -      Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:869:21
23:21:08     INFO -      this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:745:7
23:21:08     INFO -  TEST-INFO | expected PASS
23:21:09     INFO -  10135 INFO [1762] WARNING: NS_ENSURE_TRUE(mMutable) failed: file /builds/slave/m-in-lx-d-00000000000000000000/build/netwerk/base/src/nsSimpleURI.cpp, line 265
23:21:09     INFO -  10136 INFO TEST-OK | chrome://mochitests/content/browser/browser/base/content/test/general/browser_urlbarSearchSingleWordNotification.js | took 11265ms
23:21:09     INFO -  10137 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/general/browser_urlbarSearchSingleWordNotification.js | Found an unexpected tab at the end of test run: about:blank - Found an unexpected tab at the end of test run: about:blank
}
Flags: needinfo?(gijskruitbosch+bugs)
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Flags: needinfo?(gijskruitbosch+bugs)
This switches away from a timeout (because these are debug failures, I suspect the notification bar is just not appearing quickly enough because the asyncResolve doesn't come in quickly enough) to a mutation observer instead, and seems to work fine on my local machine. Because if the test fails, notification is null and that wreaks havoc on the rest of the test, I've adapted it to try to not break the thing quite so spectacularly in that case.
Attachment #8468493 - Flags: review?(mak77)
Marco, can you add this to the spreadsheet for this iteration?
Iteration: --- → 34.2
Points: --- → 1
QA Whiteboard: [qa-]
Flags: needinfo?(mmucci)
Hi Gijs, we aren't adding new work to the iteration until existing commitments are at least in review.  You currently have 7 points worth of work assigned. 

(In reply to :Gijs Kruitbosch from comment #5)
> Marco, can you add this to the spreadsheet for this iteration?
Status: ASSIGNED → NEW
Iteration: 34.2 → ---
Flags: needinfo?(mmucci)
(In reply to Marco Mucci [:MarcoM] from comment #6)
> Hi Gijs, we aren't adding new work to the iteration until existing
> commitments are at least in review.  You currently have 7 points worth of
> work assigned. 

This is an intermittent failure believed introduced in the last 24 hours by bug 1042519 - we can reopen that bug and dupe this over there is that meets the requirements of the spreadsheet better? ;-)
Flags: needinfo?(mmucci)
(In reply to Marco Mucci [:MarcoM] from comment #6)
> Hi Gijs, we aren't adding new work to the iteration until existing
> commitments are at least in review.  You currently have 7 points worth of
> work assigned. 
> 

Yes, and the 5-pointer is waiting on a needinfo request from someone else, and I'm simul-working on the other one of the two bugs (2 computers are useful...) by bisecting a regression range manually (because, for reasons that don't fit in this comment, our automated tools won't work) which is taking time. In the meantime, it took me maybe 20 minutes to write this patch, and as Ed said, it's a recently introduced regression which is causing random automated test failures.

Or, to put it differently: I'm not picking this up at a cost to my currently assigned work (which, in any case, is a disproportionately small fraction of the total cap) or because it's fun, but because it needs doing.

In fact, looking at the spreadsheet, there are "-17" points remaining until the cap, so even if I unassigned all my current work and sat around doing nothing, I couldn't pick up new work because we'd still be 10 points over.

This doesn't seem like a sensible situation.
Status: NEW → ASSIGNED
(In reply to Marco Mucci [:MarcoM] from comment #6)
> Hi Gijs, we aren't adding new work to the iteration until existing
> commitments are at least in review.

Also, this bug is now already "in review"... doesn't that mean it gets exempted from that cap, too?
Marco: we can add this bug to the current iteration - my fault for not making the exception policy clear.
Flags: needinfo?(mmucci)
The total number of points for the iteration is larger than the cap itself as the total represents carry over, new and additional work all at different stages of completion.  The point cap, itself, is a tool used only at the planning meeting and not used at all throughout the iteration.  It is there to control the amount of work selected from the planning meeting and represents the team's commitment to the iteration.  What is important when the iteration begins, following the planning meeting, is tracking the changes in status for the bugs.  That way we know when someone is occupied with work in progress for their committed work selection or they have room to pick up additional work.  Our aim is to complete all the committed work that comes out of the planning meeting.  Anything beyond that is bonus.  Having room to select work beyond the iteration commitment means having all the committed work at least in review.   

(In reply to :Gijs Kruitbosch from comment #8)
> (In reply to Marco Mucci [:MarcoM] from comment #6)
> > Hi Gijs, we aren't adding new work to the iteration until existing
> > commitments are at least in review.  You currently have 7 points worth of
> > work assigned. 
> > 
> 
> Yes, and the 5-pointer is waiting on a needinfo request from someone else,
> and I'm simul-working on the other one of the two bugs (2 computers are
> useful...) by bisecting a regression range manually (because, for reasons
> that don't fit in this comment, our automated tools won't work) which is
> taking time. In the meantime, it took me maybe 20 minutes to write this
> patch, and as Ed said, it's a recently introduced regression which is
> causing random automated test failures.
> 
> Or, to put it differently: I'm not picking this up at a cost to my currently
> assigned work (which, in any case, is a disproportionately small fraction of
> the total cap) or because it's fun, but because it needs doing.
> 
> In fact, looking at the spreadsheet, there are "-17" points remaining until
> the cap, so even if I unassigned all my current work and sat around doing
> nothing, I couldn't pick up new work because we'd still be 10 points over.
> 
> This doesn't seem like a sensible situation.
Iteration: --- → 34.2
Comment on attachment 8468493 [details] [diff] [review]
use a mutation observer instead of a timeout to wait for the notification,

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

::: browser/base/content/test/general/browser_urlbarSearchSingleWordNotification.js
@@ +17,5 @@
> +        deferred.resolve();
> +      }
> +    }
> +    let observer = new MutationObserver(checkForNotification);
> +    observer.observe(notificationBox, {childList: true});

I don't understand, if the test fails here we just timeout cause we are yielding and the deffered won't be resolved.. so how is your later null check on notification helping with the output?

It would have helped with the previous code that was exiting after a timeout... Here you should basically add a timeout to resolve the deferred if you don't get the mutation in a meaningful time... but then again you are reimplementing waitForCondition with a slightly better check.
My opinion is that it would be fine to change waitForCondition to wait longer. It is currently set to 3s, I think we might go up to 10s, it might also help with other intermittents.

@@ +74,4 @@
>  
> +      yield docLoadPromise;
> +    } else {
> +      Services.prefs.setBoolPref(pref, true);

a comment here would be welcome, something like "// If the test failed reset state to a clean value for the next tests." (even if probably each test should ensure the end is properly setup when it runs rather then relying on the previous one...)
Attachment #8468493 - Flags: review?(mak77)
As noted on IRC, I'd like to avoid setTimeout, because it's so unreliable. I've removed the nullcheck.
Attachment #8468780 - Flags: review?(mak77)
Attachment #8468493 - Attachment is obsolete: true
Comment on attachment 8468780 [details] [diff] [review]
use a mutation observer instead of a timeout to wait for the notification,

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

waitForCondition is not a plain setTimeout, it's a polling strategy with an explicit timeout (that I think is too short, regardless). It's a much better approach than a plain "wait N seconds and hope something happened".

The disadvantage here is mostly that timing out will take 30s instead of whatever we define in waitForCondition, nothing major so it might be fine.

But it doesn't have a timeout handler and...

::: browser/base/content/test/general/browser_urlbarSearchSingleWordNotification.js
@@ +12,5 @@
>    let notificationBox = gBrowser.getNotificationBox(tab.linkedBrowser);
>    if (expected) {
> +    let checkForNotification = function() {
> +      if (notificationBox.getNotificationWithValue(value)) {
> +        observer.disconnect();

...if this should timeout, all of the next tests will have an active MutationObserver since nobody disconnected it. what happens if they will use the same notification?
You're right that we should clean it up. Regarding the sequence of setTimeouts for waitCondition: yes, I know that - but it's still relying on the timeout firing reliably, which isn't happening if we look at the logs where the timeout follows the yielding of the previous promise within less than a second (in all of these cases, interestingly...).
Attachment #8468882 - Flags: review?(mak77)
Attachment #8468780 - Attachment is obsolete: true
Attachment #8468780 - Flags: review?(mak77)
Comment on attachment 8468882 [details] [diff] [review]
use a mutation observer instead of a timeout to wait for the notification,

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

ok let's try.
Attachment #8468882 - Flags: review?(mak77) → review+
(In reply to :Gijs Kruitbosch from comment #15)
> but it's still relying on
> the timeout firing reliably, which isn't happening if we look at the logs
> where the timeout follows the yielding of the previous promise within less
> than a second (in all of these cases, interestingly...).

ah yeah that sounds like a bug, looks like it's timing out after the first 100ms... Sounds like the waitForCondition code, or the condition we pass to it, is bogus?
I wonder if the problem are the 100ms, so if moving to, let's say, 10 * 500ms intervals, would work better and help with other intermittent failures.  Fwiw the code works properly in Scratchpad.
(In reply to Marco Bonardo [:mak] (Away 15-31 Aug) from comment #18)
> Comment on attachment 8468882 [details] [diff] [review]
> use a mutation observer instead of a timeout to wait for the notification,
> 
> Review of attachment 8468882 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ok let's try.

https://hg.mozilla.org/integration/fx-team/rev/01e2fbcd6455

Filed bug 1050183 for the waitForCondition issue.
Flags: firefox-backlog+
https://hg.mozilla.org/mozilla-central/rev/01e2fbcd6455
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Marking verified per qa- - but 33 is still affected, so we should probably uplift this...
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.