Similar to bug 989960, but for browser-chrome tests.

Don't need this for devtools de-chrome-ification after all.

With Promises enabled in the JS Engine (bug 911216), does the status of this bug change?

(In reply to Alex Vincent [:WeirdAl] from comment #3) > With Promises enabled in the JS Engine (bug 911216), does the status of this > bug change? No, it's still needed.

Mike, is this something the platform/frontend-hybrid team people could look at? Dunno if we have better process for radar'ing stuff to them/you - please let me know if so!

This sounds like a Good Next Bug to me, especially since there's a template in bug 989960 to more or less work off of. I don't believe it currently falls under my team's mandate, which (as I've come to understand it) is to do odd jobs (usually in the UI space) for Platform Engineering managers.

This bug should likely be worked on by someone experienced with the code base, because a good part of it is about fixing or whitelisting newly discovered test failures across the tree, which may uncover bugs in various areas of the code.

(In reply to Jarda Snajdr [:jsnajdr] from comment #7) > https://treeherder.mozilla.org/#/jobs?repo=try&revision=183a7a5c200d These try results are 2 months old and started expiring, so I just pushed your patch to try again to see what tests are currently failing: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2035c0d877d61b414942d836fc57fb3afd44e346

Attaching the patch Jarda has pushed to try in comment 7.

Comment on attachment 8852855 [details] [diff] [review] Jarda's patch from comment 7 We'll probably want to reuse the Assert.jsm instance we already have. We'll have to decide what to do for thisTestLeaksUncaughtRejectionsAndShouldBeFixed, which might be something for a follow-up bug. For the rest, this looks generally good. I'll take a closer look to a patch that passes the tryserver build.

I've pushed the patch to try again to have an updated view of the failures: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e9d40635da00e7d9c9ba4adf6fd6d6d17b1d8b1e At least the following tests leak unhandled rejections: browser/components/extensions/test/browser/browser_ext_windows_allowScriptsToClose.js browser/components/sessionstore/test/browser_background_tab_crash.js browser/extensions/pdfjs/test/browser_pdfjs_zoom.js devtools/client/aboutdebugging/test/browser_addons_reload.js devtools/client/debugger/new/test/mochitest/browser_dbg-iframes.js devtools/client/inspector/markup/test/browser_markup_links_06.js devtools/client/netmonitor/test/browser_net_simple-request-data.js devtools/server/tests/browser/browser_webextension_inspected_window.js dom/base/test/browser_timeout_throttling_with_audio_playback.js dom/tests/browser/browser_largeAllocation_non_win32.js dom/tests/browser/browser_largeAllocation_win32.js toolkit/content/tests/browser/browser_block_webAudio.js toolkit/content/tests/browser/browser_mute_webAudio.js The following folders have lots of tests leaking rejections: devtools/client/webaudioeditor/test/* browser/components/extensions/test/browser/test-oop-extensions/* browser/components/extensions/test/browser/* And I'm afraid that in addition to that we'll see that a lot of the test timeouts in the try results are also related to this patch, and may need to be debugged. This test seems related and will definitely need debugging: devtools/client/debugger/test/mochitest/browser_dbg_promises-rejection-stack.js

You could potentially keep using the escape route via the thisTestLeaksUncaughtRejectionsAndShouldBeFixed mechanism, so that at least new errors can be caught.

Comment on attachment 8852855 [details] [diff] [review] Jarda's patch from comment 7 Review of attachment 8852855 [details] [diff] [review]: ----------------------------------------------------------------- ::: testing/mochitest/browser-test.js @@ +220,5 @@ > "webConsoleCommandController", > ]; > > + this.PromiseTestUtils.init(); > + this.PromiseTestUtils.Assert = new this.Assert((err, message, stack) => { Instead of initializing Assert to a new object, we can just place... this.PromiseTestUtils.Assert = this.currentTest.scope.Assert; ...just before this existing line: this.ContentTask.setTestScope(currentScope); @@ +322,5 @@ > this.SimpleTest.waitForFocus(aCallback); > }, > > finish: function Tester_finish(aSkipSummary) { > + this.PromiseTestUtils.assertNoUncaughtRejections(); I guess we have to add this call here too: this.PromiseTestUtils.ensureDOMPromiseRejectionsProcessed(); Judging by the number of existing checks for unhandled rejections in the current suite, we probably check in more places than needed, but it doesn't hurt and it's better than missing places where we should check. @@ +961,5 @@ > }; > > this.thisTestLeaksUncaughtRejectionsAndShouldBeFixed = function(...rejections) { > + // TODO: handle case when rejections.length > 0 > + self.__tester.PromiseTestUtils.thisTestLeaksUncaughtRejectionsAndShouldBeFixed(); We should convert this to direct PromiseTestUtils usage.

Comment on attachment 8852855 [details] [diff] [review] Jarda's patch from comment 7 I've refreshed my old patch with some changes from this one, and started a tryserver build again: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9b69380db8f0a8a93abc2f4f7a7b95ff1c87bc89

First round of whitelisting and simple fixes, based on the previous tryserver build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d10fa9f305309384f8b7325bd5cfd177fcaaaff2 It also seems that we already fixed the majority of unhandled rejections that were whitelisted when this feature was originally introduced for Promise.jsm. The whitelisting mechanism offered by PromiseTestUtils detects when an issue is fixed, so we won't have leftovers like these in the future.

Comment on attachment 8867291 [details] Bug 1242505 - Part 1 - Handle more rejections in code exercised by browser-chrome tests. This adds the whitelistRejectionsGlobally function, mainly used for shutdown issues in devtools and WebExtensions. It also solves the timeouts that were caused by the reporting code tripping on cases where the rejection happened in a document that was unloaded, which didn't happen with xpcshell tests. I'm still working on whitelisting so I'll ask for review later, in the meantime you may want to take a look to the infrastructure changes. I'd like to add a few self-tests later as well.

Comment on attachment 8867291 [details] Bug 1242505 - Part 1 - Handle more rejections in code exercised by browser-chrome tests. https://reviewboard.mozilla.org/r/138826/#review145914 Looks great to me :-) ::: browser/components/extensions/ExtensionPopups.jsm:120 (Diff revision 2) > this.extension.forgetOnClose(this); > > this.destroyed = true; > this.browserLoadedDeferred.reject(new Error("Popup destroyed")); > + // Ignore unhandled rejections if the "attach" method is not called. > + this.browserLoaded.catch(() => {}); This is not test code and may need review from someone who knows that file. ::: toolkit/modules/tests/modules/PromiseTestUtils.jsm:57 (Diff revision 2) > */ > _rejectionIgnoreFns: [], > > /** > + * If any of the functions in this array returns true when called with the > + * rejection details as its only argument, the rejection is ingored. This typo: ingored

Mac OS X tests are lagging behind, but for the rest there is a green tryserver build! https://treeherder.mozilla.org/#/jobs?repo=try&revision=2d6996813a5f72a9234bfce7cbbc4ef33ae1c5a5 I'll separate the patch in various changesets before posting for review. I've also added some additional self-tests that showed that I still had to move some of the PromiseTestUtils calls around in "browser-test.js".

(In reply to :Paolo Amadini from comment #28) > This is not test code and may need review from someone who knows that file. I think the changes in Part 1 are simple enough that they can be handled by a sweeping review, but if you'd like to flag someone in particular feel free to add them in MozReview!

Comment on attachment 8867291 [details] Bug 1242505 - Part 1 - Handle more rejections in code exercised by browser-chrome tests. https://reviewboard.mozilla.org/r/138826/#review146646

Comment on attachment 8871268 [details] Bug 1242505 - Part 2 - Update PromiseTestUtils for use in mochitests. https://reviewboard.mozilla.org/r/142764/#review146650

Comment on attachment 8871269 [details] Bug 1242505 - Detect unhandled rejections of native Promises in browser-chrome tests. https://reviewboard.mozilla.org/r/142766/#review146648

Comment on attachment 8871270 [details] Bug 1242505 - Part 4 - Add self-tests for unhandled rejection handling in browser-chrome tests. https://reviewboard.mozilla.org/r/142768/#review146688

Comment on attachment 8871271 [details] Bug 1242505 - Part 5 - Remove obsolete whitelisting of unhandled rejections. https://reviewboard.mozilla.org/r/142770/#review146690

Pushed by paolo.mozmail@amadzone.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/b59573695517 Part 1 - Handle more rejections in code exercised by browser-chrome tests. r=Mossop https://hg.mozilla.org/integration/mozilla-inbound/rev/22e7144f857c Part 2 - Update PromiseTestUtils for use in mochitests. r=Mossop https://hg.mozilla.org/integration/mozilla-inbound/rev/8d53be05afc5 Part 3 - Detect unhandled rejections of native Promises in browser-chrome tests. r=Mossop https://hg.mozilla.org/integration/mozilla-inbound/rev/c9c01c06908a Part 4 - Add self-tests for unhandled rejection handling in browser-chrome tests. r=Mossop https://hg.mozilla.org/integration/mozilla-inbound/rev/6f0394b523a6 Part 5 - Remove obsolete whitelisting of unhandled rejections. r=Mossop

NOTE TO SHERIFFS ON INTERMITTENT FAILURES ----------------------------------------- The tryserver build is green, however the patch might surface new intermittent failures like this one: https://treeherder.mozilla.org/#/jobs?repo=try&revision=bf70b69e9af94f1d899e82d108e3c16c2abf9627&selectedJob=102554590 These are very likely to be legitimate intermittents that simply we didn't catch before. In the various tryserver builds I ran, they happened very infrequently, so they shouldn't be a reason for backout unless they are very high frequency. We can file new bugs for them, and we can always land a follow-up whitelisting line in the failing tests if it turns out to be necessary.

Backed out bug 1365649 and bug 1242505 for frequently failing devtools/client/netmonitor/test/browser_net_simple-request-data.js: Bug 1365649: https://hg.mozilla.org/integration/mozilla-inbound/rev/398d167b26f90af9953523ef4cbef3e38af5eb4a Bug 1242505: https://hg.mozilla.org/integration/mozilla-inbound/rev/5f54472fcf58d462a31398d15cacbdde5bcad7e1 https://hg.mozilla.org/integration/mozilla-inbound/rev/79db5648dbd2a1397954bc08293082b50a037ebe https://hg.mozilla.org/integration/mozilla-inbound/rev/9dbe428fc209988be73d2a4bbc3c890811310d3d https://hg.mozilla.org/integration/mozilla-inbound/rev/5703711e4ee5c2b00e8f8fc9fcaefbcf6a15fdf5 https://hg.mozilla.org/integration/mozilla-inbound/rev/824f44aad5d7bd26c59aaf301ce60188b5466cef Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=6f0394b523a66dab444b8551deb8f3c6c81d8f31&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=102651239&repo=mozilla-inbound [task 2017-05-28T09:15:04.539439Z] 09:15:04 INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_simple-request-data.js | Item shouldn't have 'odd' class. - [task 2017-05-28T09:15:04.540493Z] 09:15:04 INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_simple-request-data.js | There should be a responseCookies data available. - [task 2017-05-28T09:15:04.541516Z] 09:15:04 INFO - TEST-PASS | devtools/client/netmonitor/test/browser_net_simple-request-data.js | There should be a requestCookies data available. - [task 2017-05-28T09:15:04.542558Z] 09:15:04 INFO - > Network events progress: 13/13, 0/0, got NetMonitor:NetworkEventUpdated:ResponseContent for server1.conn83.child1/netEvent30 [task 2017-05-28T09:15:04.543654Z] 09:15:04 INFO - Console message: [JavaScript Error: "TypeError: requestItem.responseCookies.cookies is undefined" {file: "chrome://mochitests/content/browser/devtools/client/netmonitor/test/browser_net_simple-request-data.js" line: 191}] [task 2017-05-28T09:15:04.544806Z] 09:15:04 INFO - Console message: [JavaScript Error: "TypeError: requestItem.requestCookies.cookies is undefined" {file: "chrome://mochitests/content/browser/devtools/client/netmonitor/test/browser_net_simple-request-data.js" line: 141}] [task 2017-05-28T09:15:04.545904Z] 09:15:04 INFO - Destroying the specified network monitor. [task 2017-05-28T09:15:04.546930Z] 09:15:04 INFO - Removing tab. [task 2017-05-28T09:15:04.547913Z] 09:15:04 INFO - Waiting for event: 'TabClose' on [object XULElement]. [task 2017-05-28T09:15:04.548954Z] 09:15:04 INFO - Got event: 'TabClose' on [object XULElement]. [task 2017-05-28T09:15:04.550095Z] 09:15:04 INFO - Tab removed and finished closing [task 2017-05-28T09:15:04.551137Z] 09:15:04 INFO - finish() was called, cleaning up... [task 2017-05-28T09:15:04.552123Z] 09:15:04 INFO - Buffered messages finished [task 2017-05-28T09:15:04.553397Z] 09:15:04 INFO - TEST-UNEXPECTED-FAIL | devtools/client/netmonitor/test/browser_net_simple-request-data.js | A promise chain failed to handle a rejection: requestItem is undefined - rejection date: Sun May 28 2017 09:15:03 GMT+0000 (UTC) - stack: handler@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/event-emitter.js:138:34 [task 2017-05-28T09:15:04.554381Z] 09:15:04 INFO - emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/event-emitter.js:194:13 [task 2017-05-28T09:15:04.555464Z] 09:15:04 INFO - onResponseContent/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/netmonitor/src/connector/firefox-connector.js:719:7 [task 2017-05-28T09:15:04.556346Z] 09:15:04 INFO - promise callback*onResponseContent@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/netmonitor/src/connector/firefox-connector.js:718:5 [task 2017-05-28T09:15:04.557342Z] 09:15:04 INFO - emitOnObject@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/event/core.js:110:7 [task 2017-05-28T09:15:04.558254Z] 09:15:04 INFO - DevTools RDP*request@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:717:21 [task 2017-05-28T09:15:04.559155Z] 09:15:04 INFO - getResponseContent@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/webconsole/client.js:516:12 [task 2017-05-28T09:15:04.560059Z] 09:15:04 INFO - onNetworkEventUpdate@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/netmonitor/src/connector/firefox-connector.js:614:9 [task 2017-05-28T09:15:04.560935Z] 09:15:04 INFO - emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/event-emitter.js:194:13 [task 2017-05-28T09:15:04.561883Z] 09:15:04 INFO - _onNetworkEventUpdate@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/webconsole/client.js:170:5 [task 2017-05-28T09:15:04.562739Z] 09:15:04 INFO - eventSource/proto.emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:130:9 [task 2017-05-28T09:15:04.564056Z] 09:15:04 INFO - onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/client/main.js:1017:7 [task 2017-05-28T09:15:04.564938Z] 09:15:04 INFO - send/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:570:13 [task 2017-05-28T09:15:04.565854Z] 09:15:04 INFO - exports.makeInfallible/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/ThreadSafeDevToolsUtils.js:101:14 [task 2017-05-28T09:15:04.568580Z] 09:15:04 INFO - DevToolsUtils.executeSoon*exports.executeSoon@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/DevToolsUtils.js:42:19 [task 2017-05-28T09:15:04.569556Z] 09:15:04 INFO - send@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:563:9 [task 2017-05-28T09:15:04.570570Z] 09:15:04 INFO - send@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/server/main.js:1482:5 [task 2017-05-28T09:15:04.571538Z] 09:15:04 INFO - receiveMessage@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/transport/transport.js:761:7 [task 2017-05-28T09:15:04.572449Z] 09:15:04 INFO - - false == true - JS frame :: resource://testing-common/PromiseTestUtils.jsm :: assertNoUncaughtRejections :: line 257 [task 2017-05-28T09:15:04.573331Z] 09:15:04 INFO - Stack trace: [task 2017-05-28T09:15:04.574206Z] 09:15:04 INFO - resource://testing-common/PromiseTestUtils.jsm:assertNoUncaughtRejections:257 [task 2017-05-28T09:15:04.575652Z] 09:15:04 INFO - chrome://mochikit/content/browser-test.js:Tester.prototype.nextTest<:445 [task 2017-05-28T09:15:04.576555Z] 09:15:04 INFO - testScope/test_finish/<@chrome://mochikit/content/browser-test.js:1063:11 [task 2017-05-28T09:15:04.577444Z] 09:15:04 INFO - run@chrome://mochikit/content/browser-test.js:1000:9 [task 2017-05-28T09:15:04.578702Z] 09:15:04 INFO - Console message: [JavaScript Error: "TypeError: this.tabTarget is null" {file: "resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/netmonitor/src/connector/firefox-connector.js" line: 91}]

Pushed by paolo.mozmail@amadzone.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/047e38fdb0c8 Part 1 - Handle more rejections in code exercised by browser-chrome tests. r=Mossop https://hg.mozilla.org/integration/mozilla-inbound/rev/08cf07d18bc4 Part 2 - Update PromiseTestUtils for use in mochitests. r=Mossop https://hg.mozilla.org/integration/mozilla-inbound/rev/0411e687044e Part 3 - Detect unhandled rejections of native Promises in browser-chrome tests. r=Mossop https://hg.mozilla.org/integration/mozilla-inbound/rev/971751d97bb4 Part 4 - Add self-tests for unhandled rejection handling in browser-chrome tests. r=Mossop https://hg.mozilla.org/integration/mozilla-inbound/rev/5809a30a0c48 Part 5 - Remove obsolete whitelisting of unhandled rejections. r=Mossop

(In reply to :Paolo Amadini from comment #43) > https://treeherder.mozilla.org/#/ > jobs?repo=try&revision=bf70b69e9af94f1d899e82d108e3c16c2abf9627&selectedJob=1 > 02554590 This turned out to be high frequency, so I've whitelisted it and re-landed. There isn't much point in a new tryserver build because by the time we have its results we might have introduced new failures, and there isn't enough volume to detect how often the new intermittents will happen anyways. Let's see how it goes!

https://hg.mozilla.org/mozilla-central/rev/047e38fdb0c8 https://hg.mozilla.org/mozilla-central/rev/08cf07d18bc4 https://hg.mozilla.org/mozilla-central/rev/0411e687044e https://hg.mozilla.org/mozilla-central/rev/971751d97bb4 https://hg.mozilla.org/mozilla-central/rev/5809a30a0c48

This was backed out because of the unexplained high-frequency intermittent failure in bug 1368364, I'll try and figure out what is going on and re-land at the beginning of the next cycle.

With Florian we decided to disable the test that caused bug 1368364 on Linux debug builds, and I filed bug 1373549 for the non-trivial work required to re-enable the test in the future. I've started 60 browser-chrome jobs on all platforms, let's see how it goes: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8f448ae3b7542b314f8940f966727b4551733faa https://treeherder.mozilla.org/#/jobs?repo=try&revision=c4795caf730a3995d080ae9db5273fd5f67c1e50 https://treeherder.mozilla.org/#/jobs?repo=try&revision=44fce201d3761a2898c98dcad3e1224d70aa0781 Note that in these days there are a lot of unrelated high-frequency intermittents, all have bugs on file though.

The failure affects other platforms too, just less frequently. It shouldn't happen on e10s though, let's see: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c93a3e4367532b09689596ad5befe9feb8f577e4 https://treeherder.mozilla.org/#/jobs?repo=try&revision=98ec192c5d9344a224cb18a6efc9b702c6447581 https://treeherder.mozilla.org/#/jobs?repo=try&revision=c63c3345e6396abd71a97437736cfe11bef94c11 Florian, I talked to the walrus in browser_BrowserUITelemetry_buckets.js and, if the tryserver builds succeed, he's fine with him and his friends running only on e10s, because the infrastructure they are testing should the same. Do you agree?

(In reply to :Paolo Amadini from comment #53) > Florian, I talked to the walrus in browser_BrowserUITelemetry_buckets.js > and, if the tryserver builds succeed, he's fine with him and his friends > running only on e10s, because the infrastructure they are testing should the > same. Do you agree? I agree as long as you file a bug to get it re-enabled by the relevant team, and include that bug number in a comment on the skip-if line of the test manifest.

Pushed by paolo.mozmail@amadzone.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/3b544d62b767 Detect unhandled rejections of native Promises in browser-chrome tests. r=Mossop