about:debugging is registered statically in docshell/base/nsAboutRedirector.cpp and points to devtools/client/aboutdebugging/aboutdebugging.xhtml We should keep the entry point valid even if DevTools are not installed. When they are not installed, the page should tell the user that DevTools are now an addon and can be installed (TBD in more details once we made some progress with the UI shim). And if DevTools are installed, the original about:debugging UI should be loaded.

Note that if that can help, you may register about:debugging in JS/dynamically, like about:devtools-toolbox: http://searchfox.org/mozilla-central/source/devtools/client/framework/about-devtools-toolbox.js (or like about:flyweb http://searchfox.org/mozilla-central/source/mobile/android/extensions/flyweb/bootstrap.js#25 or like about:pocket http://searchfox.org/mozilla-central/source/browser/extensions/pocket/content/AboutPocket.jsm#24)

(In reply to Alexandre Poirot [:ochameau] from comment #1) > Note that if that can help, you may register about:debugging in > JS/dynamically, like about:devtools-toolbox: > http://searchfox.org/mozilla-central/source/devtools/client/framework/about- > devtools-toolbox.js > > (or like about:flyweb > http://searchfox.org/mozilla-central/source/mobile/android/extensions/ > flyweb/bootstrap.js#25 > or like about:pocket > http://searchfox.org/mozilla-central/source/browser/extensions/pocket/ > content/AboutPocket.jsm#24) Thanks! I actually did that in my first implementation on GitHub [1]. But I was thinking here I would keep a static registration since I still want a UI to be displayed when DevTools are not installed. That's the approach I am trying for now, feel free to have a look at the patches! I guess the other option would be to unregister the about:debugging shim when loading the devtools addon and register the real about:debugging, but I imagine the end result would be more complex than what I have right now. Might be wrong though! [1] https://github.com/juliandescottes/ff-dt/commit/de3c796c7afae2c343c6d01450e031e54a9cbdec

Comment on attachment 8865852 [details] Bug 1363327 - simplify aboutdebugging landing page; https://reviewboard.mozilla.org/r/137460/#review140630 ::: devtools/shim/aboutdebugging/aboutdebugging-shim.js:28 (Diff revision 2) > + loadAboutDebugging() { > + const { BrowserLoader } = Components.utils.import("resource://devtools/client/shared/browser-loader.js", {}); > + const { require } = BrowserLoader({ > + baseURI: "resource://devtools/client/aboutdebugging/", > + window > + }); > + > + this.aboutDebugging = require("./initializer"); > + this.aboutDebugging.init(); > + }, I would not involve any devtools code from m-c. Anything but the Shim. So, instead of calling loadAboutDebugging, I would call DevToolsShim.loadAboutDebugging(window); But that's if we keep this solution overall. I'm wondering if there is better opportunities with a JS implementation of about:debugging. 1) Implement it in JS like what you did here https://github.com/juliandescottes/ff-dt/commit/de3c796c7afae2c343c6d01450e031e54a9cbdec#diff-1cb1ff33d172c8d7d313264b82274bfdR27 2) But instead of doing it from the addon, do it from from m-c. Then from that newChannel function: newChannel: function (aURI, aLoadInfo) { let uri = DevToolsShim.isInstalled() ? "chrome://devtools/content/aboutdebugging/aboutdebugging.xhtml" : "chrome://devtools-shim/content/aboutdebugging/aboutdebugging.xhtml"; let chan = Services.io.newChannelFromURIWithLoadInfo(uri, aLoadInfo); 3) It would be even better to retrieve the chrome://devtools/ url via the shim, from the addon.

Thanks for the feedback. I was not sure about the lifecycle of the registration. If we can simply check for the addon in newChannel, then I agree it will be easier to implement this way. I'll prepare another patch following your suggestions. I'm not sure how we are supposed to generate the uuids used in the registration code (I copied the one on github from the about:devtools-toolbox registration code). But we can clarify that later on.

./mach uuid if you want new id and I imagine you do if you switch from c++ to js implementation. This is used to differenciate two xpcom. Sometimes you may have two implementing the same class name (contractId : @mozilla.org/network/protocol/about;1?what=debugging) So that you use ID to identify/get the right one.

I don't think we should move this much more before the low-level + UI shim land, but this new series of patches follows your suggestions. About:debugging is now registered using a manifest, and we are using the shim to know whether we should display the real about:debugging or a shim page. The shim page will need localized strings that will stay in mozilla-central. I think we will have localized strings in mozilla central for the UI shim too, so they will probably end up in the same place. When DevTools are registered, the about:debugging fake landing page now simply reloads the page. > 3) It would be even better to retrieve the chrome://devtools/ url via the shim, from the addon. Agreed but I think that's something we can change later on.

I left out the last changeset which adds a shim landing page but I figure we could start reviewing the other patches already. One thing I don't like very much at the moment is that the manifest for registering about:debugging is in the shim folder. Since we're registering it for both the shim page and the real page, it could make sense to let it live somewhere else, but where? at the root of devtools? (otherwise devtools/shim/ is probably not that bad for the time being).

(In reply to Julian Descottes [:jdescottes] from comment #17) > One thing I don't like very much at the moment is that the manifest for > registering about:debugging is in the shim folder. Since we're registering > it for both the shim page and the real page, it could make sense to let it > live somewhere else, but where? at the root of devtools? (otherwise > devtools/shim/ is probably not that bad for the time being). It doesn't look bad to be. It is about the same than DevToolsShim.jsm. You can think about /devtools/shim/ as "whatever stays on m-c" once devtools move to github. Most of the files in /devtools/ should also go away, all but moz.build.

Comment on attachment 8865852 [details] Bug 1363327 - simplify aboutdebugging landing page; https://reviewboard.mozilla.org/r/137460/#review145902 Wha! We were importing react for nothing?! I'm expecting this to speed up about:debugging load time!

Comment on attachment 8867207 [details] Bug 1363327 - register about:debugging dynamically using a manifest; https://reviewboard.mozilla.org/r/138770/#review145880 ::: devtools/shim/aboutdebugging-registration.js:10 (Diff revision 2) > +"use strict"; > + > +// Register the about:debugging URL, that allows to debug various targets such as addons, > +// workers and tabs by launching a dedicated DevTools toolbox for the selected target. > +// If DevTools are not installed, this about page will display a shim landing page > +// encouraging the user download and install DevTools. I think you miss "to" to be correct: `the user +to+ download` ::: devtools/shim/aboutdebugging-registration.js:39 (Diff revision 2) > + chan.owner = Services.scriptSecurityManager.getSystemPrincipal(); > + return chan; > + }, > + > + getURIFlags: function (uri) { > + return nsIAboutModule.ALLOW_SCRIPT || nsIAboutModule.ENABLE_INDEXED_DB; Do we use/need indexedDB?

Comment on attachment 8867208 [details] Bug 1363327 - remove unused config variable MOZ_DEVTOOLS_ALL; https://reviewboard.mozilla.org/r/138772/#review145904

Comment on attachment 8867207 [details] Bug 1363327 - register about:debugging dynamically using a manifest; https://reviewboard.mozilla.org/r/138770/#review145880 Thanks for the reviews, try https://treeherder.mozilla.org/#/jobs?repo=try&revision=96c3985d4b46f1107945177e77fbbcc37216a495 > I think you miss "to" to be correct: > `the user +to+ download` thanks! > Do we use/need indexedDB? I don't think so, I must have copied it from another registration.

Getting TEST-UNEXPECTED-FAIL | browser/base/content/test/static/browser_all_files_referenced.js | unreferenced file: chrome://devtools/content/aboutdebugging/aboutdebugging.xhtml Even though the file is referenced in https://hg.mozilla.org/try/file/137fba8b2e64/devtools/shim/aboutdebugging-registration.js#l22 Alex, do you know if I am missing something something here?

Other about:debugging tests are failing on try too. I guess I am probably missing an entry in package-manifest.in . Will look at that a bit later.

Hopefully this one should be greenish: https://treeherder.mozilla.org/#/jobs?repo=try&revision=76cb1b163e846321cb891f8e89ddaa39866f4d74

Pushed by jdescottes@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c72c2c5785e8 simplify aboutdebugging landing page;r=ochameau https://hg.mozilla.org/integration/autoland/rev/ccd1dc27a473 register about:debugging dynamically using a manifest;r=ochameau https://hg.mozilla.org/integration/autoland/rev/c735b95bef36 remove unused config variable MOZ_DEVTOOLS_ALL;r=ochameau

https://hg.mozilla.org/mozilla-central/rev/c72c2c5785e8 https://hg.mozilla.org/mozilla-central/rev/ccd1dc27a473 https://hg.mozilla.org/mozilla-central/rev/c735b95bef36