Removing dependency on Promise.jsm from devtools/shared/defer appears to be more challenging than the other modules. It makes two tests fail because of changes to the stack behavior when Promises are involved: TEST-UNEXPECTED-FAIL | devtools/server/tests/unit/test_protocol_stack.js | run_test/onConnect/< - [run_test/onConnect/< : 86] Incomplete stack - false == true TEST-UNEXPECTED-FAIL | devtools/shared/tests/unit/test_executeSoon.js | - Incomplete stack - false == true For test_protocol_stack.js, it is simple, there is just a change is the way the stacks are stringified. For test_executeSoon.js it is another story as it seems to highlight a loss of information in the stacks. http://searchfox.org/mozilla-central/source/devtools/shared/tests/unit/test_executeSoon.js#28-40 yield waitForTick(); let stack = Components.stack; ... We expect to find the call to waitForTick in this stack. Here the the stack with current tip: @/mnt/desktop/gecko/obj-firefox-artifact/_tests/xpcshell/devtools/shared/tests/unit/test_executeSoon.js:29:17 process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:922:23 walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:806:7 Promise*scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:739:11 schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:770:7 completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:707:7 DevToolsUtils.executeSoon*exports.executeSoon@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/DevToolsUtils.js:57:19 waitForTick@/mnt/desktop/gecko/obj-firefox-artifact/_tests/xpcshell/devtools/shared/tests/unit/test_executeSoon.js:47:3 @/mnt/desktop/gecko/obj-firefox-artifact/_tests/xpcshell/devtools/shared/tests/unit/test_executeSoon.js:28:9 TaskImpl_run@resource://gre/modules/Task.jsm:331:42 TaskImpl@resource://gre/modules/Task.jsm:280:3 asyncFunction@resource://gre/modules/Task.jsm:252:14 Task_spawn@resource://gre/modules/Task.jsm:166:12 _run_next_test@/mnt/desktop/gecko/testing/xpcshell/head.js:1488:9 run@/mnt/desktop/gecko/testing/xpcshell/head.js:701:9 _do_main@/mnt/desktop/gecko/testing/xpcshell/head.js:221:3 _execute_test@/mnt/desktop/gecko/testing/xpcshell/head.js:544:5 @-e:1:1 You see everything, the executeSoon, waitForTick and Promise.jsm internals! But here is what you get if you make defer use DOM Promise: @/mnt/desktop/gecko/obj-firefox-artifact/_tests/xpcshell/devtools/shared/tests/unit/test_executeSoon.js:29:17 _do_main@/mnt/desktop/gecko/testing/xpcshell/head.js:221:3 _execute_test@/mnt/desktop/gecko/testing/xpcshell/head.js:544:5 @-e:1:1 It looks like async stack don't work at all across DOM Promises... It is unclear what is the expected behavior here regarding DOM Promise, but it seems to clearly differ from Promise.jsm. * It looks like stack are correct when throwing and looking at exception stacks: new Promise(function foo(done) {throw new Error("err")}).then(null, function bar(e) {console.log(e.stack);}); - foo@debugger eval code:1:39 - @debugger eval code:1:1 The important point here is that we see "foo" function in the stack! * But the is a loss of information when looking at current stack from promise handlers: new Promise(function foo() {throw new Error("err")}).then(null,function bar(e) {console.log(new Error().stack);}); - bar@debugger eval code:1:93 - promise callback*@debugger eval code:1:1 new Promise(function foo(done) {done()}).then(function bar(e) {console.log(new Error().stack);}); - bar@debugger eval code:1:76 - promise callback*@debugger eval code:1:1 There is this strange "promise callback" item, but we don't see "foo" in the stack. I ran all these tests with javascript.options.asyncstack set to true. I tried to look at Promise.cpp and it is unclear how async stack are supported, if they are. It looks like it correctly register instanciation, and settlement for PromiseDebugging: http://searchfox.org/mozilla-central/source/js/src/builtin/Promise.cpp#3178-3185 http://searchfox.org/mozilla-central/source/js/src/builtin/Promise.cpp#1290-1298 But at first look, I don't see anything to help supporting async stack. It looks like it should involve "JS::AutoSetAsyncStackForNewCalls" in order to help supporting that, but I can't see any usage of this. Or is it using AsyncFunction.cpp, which uses AutoSetAsyncStackForNewCalls...??

This change also results in this mochitest failure: devtools/client/framework/test/browser_target_events.js | A promise chain failed to handle a rejection: unsafe CPOW usage forbidden - stack: handler@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/event-emitter.js:83:7 emit@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/event-emitter.js:137:11 onLocationChange@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/client/framework/target.js:789:9 This may relate to bug 1386299. Before event-emitter was in a JSM, it may have still be able to use CPOW, whereas it is clear that DevTools modules can't since bug 1355994 landed. This exception comes from this line: http://searchfox.org/mozilla-central/source/devtools/client/framework/target.js#789 which looks very outdated, broken and useless: 1) Navigate should come with an object as first argument, not a window object. Like this one: http://searchfox.org/mozilla-central/source/devtools/client/framework/target.js#534-538 2) It looks like there is no code using navRequest/navPayload: http://searchfox.org/mozilla-central/source/devtools/client/framework/target.js#552-558 3) It looks completely redundant with the tabNavigated event that the server should send: http://searchfox.org/mozilla-central/source/devtools/client/framework/target.js#561 It looks like setupListeners could possibly be completely redundant with setupRemoteListeners... It may be worth to also fix that in a dedicated bug.

This may be a dupe of bug 1438121, since stacks captured by Error also use SavedFrame. See bug 1438121 comment 16 for a summary of the cause.

I think that all the roadblocks have been adressed. Stacks have been fixed by Jim on bug 1438121 and EventEmitter have been fixed in bug 1386299. Let's see what try says today.

Try is green, we should be able to proceed with this patch, finally! https://treeherder.mozilla.org/#/jobs?repo=try&revision=1a600b75a946ec86cb7a42ca9de3cda0589a0f5f

Comment on attachment 8986433 [details] Bug 1388054 - Stop using Promise.jsm from devtools/shared/defer. https://reviewboard.mozilla.org/r/251794/#review258244 Great, thanks to everyone who helped here! :)

Pushed by apoirot@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0605328631fd Stop using Promise.jsm from devtools/shared/defer. r=jryans

Backed out changeset 0605328631fd (bug 1388054) for failing in devtools/client/shadereditor/test/browser_se_aaa_run_first_leaktest.js on a CLOSED TREE Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=0605328631fda3d73bf749db3b3f2d3cc652a1ae&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=pending&filter-resultStatus=running&filter-resultStatus=success&filter-searchStr=dt7&selectedJob=184032026 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=184032026&repo=autoland&lineNumber=2096 Backout: https://hg.mozilla.org/integration/autoland/rev/30ef0ff424054f15fb9dee7203cd73eac30f9c9e

Ah. Right. Shader editor may be really run only on osx because it needs webgl to work on CI. Now, it is unclear why it makes this test to fail except just being a race. It should not be related to panel's document promises as defer is using a system Promise...