Nightly: 48.0a1 (2016-04-02) gecko-dev repository: 1bb567e https://github.com/mozilla/gecko-dev/commit/1bb567e688ffbcb0939974b6717bd97dbecee319 Step to reproduce: 1. Load DevTools Reload's install.rdf 2. Hit F12 to open devtool, and hit Ctrl+Alt+R Actual result: Nothing happened. My Browser Console says an error thrown (please see the attached file).

Thanks for the ping - I will reroute to the developers working on it which will take good care of it :-)

Thanks for the report, I can repro. Something regressed recently.

Work in progress. It looks like JSM have a very different behavior when used in packaged builds. It seems to have changed. May be the addon manager flush JSM/chrome caches more agressively. From bootstrap.js, we no longer get access to the previous Loader.jsm instance and always get access to the new one.

My patchs depend on bug 1258546, which itself depends on bug 1258496. If the platform patch from bug 1258496 doesn't move forward quickly, I'll try to reverse this dependency.

A followup from a previous cleanup. bug 1247203 comment 30. It looks like main.js is more disturbing then really helpful. It is especially disturbing in Loader.jsm. It is mostly used to defined what is exported *for compatiblity reasons* out of Loader.jsm. It is less disturbing for the SDK loader but still I think it is more related to addon concept than our usage. We couldn't really reload these main modules as-is. There is a lot of stuff around them that needs to be set/unset. What do you think? If you don't get a good feeling about this patch I'll just drop it and focus on what is actually really necessary. Here the way to unload/reload the previously loaded module loader is what fixes the original issue behind this bug report. (devtools-unload event)

This is another important fix after this regression. If we try to use Loader.jsm in bootstrap.js, it will actually load a brand new instance with the fresh sources reference by chrome.manifest of the addon. It wasn't doing that when I introduced the addon... So we shouldn't depend on any devtools module before doing the reload!

The browser console also needs to be closed and reloaded differently. I think it would be better if HUDService closes it automatically when the modules are unloaded, but it doesn't do that for now. Anyway, I think it is still bootstrap.js responsibility to reopen it.

Comment on attachment 8739215 [details] [diff] [review] fix browser console reload via the addon - v1 Review of attachment 8739215 [details] [diff] [review]: ----------------------------------------------------------------- ::: devtools/bootstrap.js @@ +168,5 @@ > > + // Browser console document can't just be reloaded. > + // HUDService is going to close it on unload. > + // Instead we have to manually toggle it. > + if (reopenBrowserConsole) { Shouldn't this patch be merged with the previous one? `reopenBrowserConsole` is introduced in the previous patch, and is not used anywhere there...

Comment on attachment 8739212 [details] [diff] [review] Remove unecessary Loader.jsm main in favor of require() - v1 Review of attachment 8739212 [details] [diff] [review]: ----------------------------------------------------------------- I think this looks good overall, but who is using the "reload" data? ::: devtools/bootstrap.js @@ +99,5 @@ > }, false); > > + // As we can't get a reference to existing Loader.jsm instances, we send them > + // an observer service notification to unload them. > + Services.obs.notifyObservers(null, "devtools-unload", "reload"); Who is using this "reload" data? ::: devtools/server/content-server.jsm @@ +19,5 @@ > // in the same process. > let devtools = new DevToolsLoader(); > devtools.invisibleToDebugger = true; > + let main = devtools.require("devtools/server/main"); > + let { DebuggerServer, ActorPool } = main; Might as well combine these lines? ::: devtools/shared/Loader.jsm @@ +130,5 @@ > var gNextLoaderID = 0; > > /** > * The main devtools API. > + * The standard instance of this loader is Nit: rewrap the remaining lines below

Merged the two patches.

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #10) > Comment on attachment 8739212 [details] [diff] [review] > Remove unecessary Loader.jsm main in favor of require() - v1 > > Review of attachment 8739212 [details] [diff] [review]: > ----------------------------------------------------------------- > > I think this looks good overall, but who is using the "reload" data? > > ::: devtools/bootstrap.js > @@ +99,5 @@ > > }, false); > > > > + // As we can't get a reference to existing Loader.jsm instances, we send them > > + // an observer service notification to unload them. > > + Services.obs.notifyObservers(null, "devtools-unload", "reload"); > > Who is using this "reload" data? Loader.jsm: DevToolsLoader.prototype.observe which passes it to `provider.unload(data)`. It is then passed to unload listeners, but they don't care about this argument. (devtools.js and devtools-browser.js at end of files: `unload(function () {... })`); So it is more to respect SDK module loader convention than being really useful.

Comment on attachment 8740932 [details] [diff] [review] Remove unecessary Loader.jsm main in favor of require() - v2 Review of attachment 8740932 [details] [diff] [review]: ----------------------------------------------------------------- ::: devtools/shared/Loader.jsm @@ +309,5 @@ > this.devtools = this.loader = new DevToolsLoader(); > > this.require = this.devtools.require.bind(this.devtools); > + > +// For compatiblity reasons, exposes these symbols on "devtools": Nit: compatibility, expose

Fix comment.

https://hg.mozilla.org/integration/fx-team/rev/cd9c7f0216c4b19d79e607ca7ff04302c6375add Bug 1261665 - Remove unecessary Loader.jsm main in favor of require(). r=jryans https://hg.mozilla.org/integration/fx-team/rev/ef461673c13f81ff4bbb162f739227ab75844941 Bug 1261665 - Do not use devtools module in devtools addon before reloading. r=jryans

https://hg.mozilla.org/mozilla-central/rev/cd9c7f0216c4 https://hg.mozilla.org/mozilla-central/rev/ef461673c13f