As bug 1190452 with dbg-server.jsm, we need to import DebuggerClient via require instead of Cu.import, that, to be able to reload devtools codebase.

This patch is just to help reviewing the second one, in order to highlight the modification I made in order to be able to load this file as a jetpack module.

A first try with some failure, but running on various platform (esp b2g to check scope issues) https://treeherder.mozilla.org/#/jobs?repo=try&revision=92e39a0bf827 And a new one with additional fixes that should be green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2d4278c2855e I cleaned up client.js header with all the imports. We no longer need to bind exported symbols on `this` (only jsm scope is broken on b2g, not commonjs modules). I no longer import transport from client.js, but only from dbg-client.jsm as there is no usage of DebuggerTransport from client.js. Also I no longer load transport.js via subscriptloader, but as a regular module. And finally, I want to highlight that we used to do Cu.import(deprecated-sync-thenables) without the second parameter that is often `{}`. That happen to mess up with all sandboxes global scope :x I'll try to get rid of all Cu.import usages without a second argument. That can bring very unexpected behavior. Here it overloaded `Promise` symbol for all sandboxes :/

Comment on attachment 8646433 [details] [diff] [review] convert to commonjs module - v1 Review of attachment 8646433 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/devtools/client/client.js @@ +33,3 @@ > } > > function dumpv(msg) { It looks like `dumpn` and `dumpv` aren't used in this file and should be removed? I believe they were only here to support `transport.js` which used to get them here when loaded via `loadSubScript`, but it now uses `DevToolsUtils` for them, so I believe they are not used by anyone now. This does cause the transport's logging prefix to always be "DBG-SERVER" even when logging for the client instead of "DBG-CLIENT", which has made me sad for a while... anyway it's not caused by your changes. :) Some day we'll use a nice logging system. ::: toolkit/devtools/client/moz.build @@ +8,5 @@ > 'dbg-client.jsm', > ] > > EXTRA_JS_MODULES.devtools.client += [ > + 'client.js', Does it make sense to call the file "main.js" to match "server/main"? Just an idea, whichever sounds best to you is fine.

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #3) > Comment on attachment 8646433 [details] [diff] [review] > convert to commonjs module - v1 > > Review of attachment 8646433 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: toolkit/devtools/client/client.js > @@ +33,3 @@ > > } > > > > function dumpv(msg) { > > It looks like `dumpn` and `dumpv` aren't used in this file and should be > removed? Removed! > ::: toolkit/devtools/client/moz.build > @@ +8,5 @@ > > 'dbg-client.jsm', > > ] > > > > EXTRA_JS_MODULES.devtools.client += [ > > + 'client.js', > > Does it make sense to call the file "main.js" to match "server/main"? Just > an idea, whichever sounds best to you is fine. Yes. I had the same idea while writing this patch. So it is now named client/main.js.

Comment on attachment 8646919 [details] [diff] [review] convert to commonjs module - v2 https://treeherder.mozilla.org/#/jobs?repo=try&revision=cc83332f9b89

And here is the patch to actually stop using the JSM in favor of the new commonjs module. As for dbg-server.jsm, I'm not dropping dbg-client.jsm as it is used by too many addons.

Finally a green try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=03366f0e1891 (Previously got a somewhat multiplatform green one before some last fixes https://treeherder.mozilla.org/#/jobs?repo=try&revision=c71a1466b714 )

https://hg.mozilla.org/integration/fx-team/rev/30649d4fc1cf https://hg.mozilla.org/integration/fx-team/rev/13b07c406765

https://hg.mozilla.org/mozilla-central/rev/30649d4fc1cf https://hg.mozilla.org/mozilla-central/rev/13b07c406765

Commit pushed to master at https://github.com/mozilla/addon-sdk https://github.com/mozilla/addon-sdk/commit/2c69d5073b311c4c678153cd6e6472d2891e2bfc Bug 1192863 - Use client/main.js instead of dbg-server.jsm. r=jryans