Bug 1188405 is about to split gDevTools.jsm into modules. This is going to help reloading them once the loader reloads. gDevTools.jsm will stay to keep add-ons working, but we should remove usage of it in our codebase. Instead, we should require the relevant module: - devtools/client/framework/devtools for gDevTools - devtools/client/framework/devtools-browser for gDevToolsBrowser

Main patch. Convert all Cu.import/require to just require(devtools) or require(devtools-browser). I removed some lazy getter here and there as devtools is loaded anyway. I included here only the more obvious replacements and kept more complex scenario for next patches.

Here is the complex scenarios, similar to the browser toolbox issue I hit in previous bug.

While working on this, I couldn't resist cleaning up some require/import...

toolbox.js was using Cu.import(gDevTools) without second argument and ended up injecting gDevTools into all next modules being loaded. Hopefully, fonts.js seems to be the only one relying on this!

Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=00c06dcd5628 Green except for bug 1246692's patch included in this run.

Comment on attachment 8717374 [details] [diff] [review] Replace usages of gDevTools.jsm by module imports - v1 Review of attachment 8717374 [details] [diff] [review]: ----------------------------------------------------------------- ::: devtools/client/debugger/debugger-commands.js @@ +7,5 @@ > "use strict"; > > const { Cc, Ci, Cu } = require("chrome"); > const l10n = require("gcli/l10n"); > +const { gDevTools } = require("devtools/client/framework/devtools"); Should it be lazy? ::: devtools/client/framework/toolbox-options.js @@ +8,5 @@ > const Services = require("Services"); > const promise = require("promise"); > Cu.import("resource://gre/modules/XPCOMUtils.jsm"); > Cu.import("resource://gre/modules/Task.jsm"); > +const {gDevTools} = require("devtools/client/framework/devtools"); Should it be lazy? ::: devtools/client/inspector/rules/rules.js @@ +23,5 @@ > const {RuleEditor} = > require("devtools/client/inspector/rules/views/rule-editor"); > const {createChild, promiseWarn} = > require("devtools/client/inspector/shared/utils"); > +const {gDevTools} = require("devtools/client/framework/devtools"); Should it be lazy? ::: devtools/client/shared/autocomplete-popup.js @@ +8,5 @@ > const {Cc, Ci, Cu} = require("chrome"); > const XUL_NS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"; > > loader.lazyImporter(this, "Services", "resource://gre/modules/Services.jsm"); > +const {gDevTools} = require("devtools/client/framework/devtools"); Should it be lazy? ::: devtools/client/shared/theme.js @@ +11,5 @@ > > const { Ci, Cu } = require("chrome"); > const { NetUtil } = Cu.import("resource://gre/modules/NetUtil.jsm", {}); > loader.lazyRequireGetter(this, "Services"); > +const { gDevTools } = require("devtools/client/framework/devtools"); Should it be lazy? ::: devtools/client/shared/view-source.js @@ +8,3 @@ > loader.lazyImporter(this, "Task", "resource://gre/modules/Task.jsm"); > > +var {gDevTools} = require("devtools/client/framework/devtools"); Should it be lazy? ::: devtools/server/actors/csscoverage.js @@ +11,5 @@ > > const events = require("sdk/event/core"); > const protocol = require("devtools/server/protocol"); > const { method, custom, RetVal, Arg } = protocol; > +const {gDevTools} = require("devtools/client/framework/devtools"); Should it be lazy? ::: devtools/shared/gcli/commands/calllog.js @@ +6,5 @@ > > const { Cc, Ci, Cu } = require("chrome"); > const l10n = require("gcli/l10n"); > const gcli = require("gcli/index"); > +const { gDevTools } = require("devtools/client/framework/devtools"); Should it be lazy? ::: devtools/shared/gcli/commands/csscoverage.js @@ +5,5 @@ > "use strict"; > > const { Cc, Ci } = require("chrome"); > > +const { gDevTools } = require("devtools/client/framework/devtools"); Should it be lazy?

Comment on attachment 8717376 [details] [diff] [review] Various require/import cleanups - v1 Review of attachment 8717376 [details] [diff] [review]: ----------------------------------------------------------------- ::: devtools/client/responsivedesign/responsivedesign.jsm @@ -11,5 @@ > -Cu.import("resource://gre/modules/XPCOMUtils.jsm"); > -Cu.import("resource://devtools/client/framework/gDevTools.jsm"); > -Cu.import("resource://devtools/shared/event-emitter.js"); > -Cu.import("resource://devtools/client/shared/widgets/ViewHelpers.jsm"); > -XPCOMUtils.defineLazyModuleGetter(this, "SystemAppProxy", This is still used in `buildPhoneUI` ::: devtools/client/webconsole/hudservice.js @@ +20,4 @@ > loader.lazyRequireGetter(this, "gDevTools", "devtools/client/framework/devtools", true); > loader.lazyRequireGetter(this, "DebuggerServer", "devtools/server/main", true); > loader.lazyRequireGetter(this, "DebuggerClient", "devtools/shared/client/main", true); > +loader.lazyRequireGetter(this, "showDoorhanger", "devtools/client/shared/doorhanger"); Needs `true` arg as well?

Comment on attachment 8717375 [details] [diff] [review] Non naive replacements of gDevTools.jsm - v1 Review of attachment 8717375 [details] [diff] [review]: ----------------------------------------------------------------- ::: devtools/client/devtools-clhandler.js @@ +54,5 @@ > + // Ensure loading main devtools module that hooks up into browser UI > + // and initialize all devtools machinery. > + // browser.xul or main top-level document used to load this module, > + // but this code may be called without/before it. > + require("devtools/client/framework/devtools-browser"); Mention bug 1247203 here as well.

Comment on attachment 8717374 [details] [diff] [review] Replace usages of gDevTools.jsm by module imports - v1 Agreed on IRC laziness is overall, since the module should generally be loaded in every case anyway.

Good catch! What do you think about merging all these patches before landing?

Comment on attachment 8717977 [details] [diff] [review] Various require/import cleanups - v2 Review of attachment 8717977 [details] [diff] [review]: ----------------------------------------------------------------- I am fine with landing small commits personally, but do what makes sense to you.

https://hg.mozilla.org/integration/fx-team/rev/6889447e1634ec55711e5ffec38fe8c74068fef2 Bug 1245462 - Replace usages of gDevTools.jsm by module imports. r=jryans https://hg.mozilla.org/integration/fx-team/rev/03788fdd62056997891df07af06766a7ec3f1f52 Bug 1245462 - Cleanup various require/import in devtools. r=jryans https://hg.mozilla.org/integration/fx-team/rev/94a25d092e44055eba9c830913d43b8a4f91d898 Bug 1245462 - DevTools is not longer exposed by gDevTools.jsm. r=jryans https://hg.mozilla.org/integration/fx-team/rev/96951f405d9e7abb431aaeb72ffa25498a0b6ffb Bug 1245462 - Explicitely import gDevTools r=jryans

https://hg.mozilla.org/mozilla-central/rev/6889447e1634 https://hg.mozilla.org/mozilla-central/rev/03788fdd6205 https://hg.mozilla.org/mozilla-central/rev/94a25d092e44 https://hg.mozilla.org/mozilla-central/rev/96951f405d9e