For ex. this addon : https://addons.mozilla.org/en-US/firefox/addon/user-style-manager/ has an options window, and thus a js file linked with that xul window. The addon installs the options window using install.rdf's optionsURL property. this options.js file is not listed in Addon Debugger.

We need to add DOM windows created from add-on URLs as debuggees. I'm not sure if the best way to do that is in the onNewGlobal event or if we should use a chrome-document-global-created observer. The latter I know will have the location set so we can tie it to add-ons. The former might have through aGlobal.unsafeDereference.location.

Hrm except that chrome documents load about:blank first so in both cases all we get is that. We'll have to use a load listener to spot anything more useful which means we could miss scripts.

This listens for new opened windows as long as the AddonThreadActor is alive. Any that map to the add-on get added as debuggees. The notification is necessary because checkGlobal is called very early in the window creation, at a time when it's location is still about:blank for chrome windows. The notification should be sent before any scripts have run though. This depends on bug 990685.

Comment on attachment 8404283 [details] [diff] [review] patch Review of attachment 8404283 [details] [diff] [review]: ----------------------------------------------------------------- LGTM, just some comments below. ::: browser/devtools/debugger/test/head.js @@ +119,5 @@ > onInstallEnded: function(aAddon, aAddonInstall) { > aInstaller.removeListener(listener); > + executeSoon(function() { > + deferred.resolve(aAddonInstall); > + }); :( Why is this necessary? It shouldn't be necessart after bug 940537, right? If so, add a TODO comment that links the bug number. ::: toolkit/devtools/server/actors/script.js @@ +4777,5 @@ > // A constant prefix that will be used to form the actor ID by the server. > actorPrefix: "addonThread", > > + onAttach: function(aRequest) { > + if (!this.attached) Nit: Brackets. Here and throughout. @@ +4788,5 @@ > + Services.obs.removeObserver(this, "document-element-inserted"); > + return ThreadActor.prototype.disconnect.call(this); > + }, > + > + observe: function(aSubject, aTopic, aData) { Nit: comment above this function explaining what it is observing and what it does when the observer fires, etc. @@ +4871,1 @@ > if (metadata) Nit from another time: brackets here please. @@ +4888,2 @@ > try { > + var uri = Services.io.newURI(uridescriptor.value, null, null); Rather than using |var|, can we just add |let uri;| at a high enough scope that it is visible to both scopes that use it? That looks like right before the try block to me.

(In reply to Nick Fitzgerald [:fitzgen] from comment #4) > Comment on attachment 8404283 [details] [diff] [review] > patch > > Review of attachment 8404283 [details] [diff] [review]: > ----------------------------------------------------------------- > > LGTM, just some comments below. > > ::: browser/devtools/debugger/test/head.js > @@ +119,5 @@ > > onInstallEnded: function(aAddon, aAddonInstall) { > > aInstaller.removeListener(listener); > > + executeSoon(function() { > > + deferred.resolve(aAddonInstall); > > + }); > > :( Why is this necessary? > > It shouldn't be necessart after bug 940537, right? If so, add a TODO comment > that links the bug number. I've added a comment but this is bug 997408. If bug 940537 causes promise callbacks to be called asynchronously then I guess that will fix this but I think it makes some sense to still use this so we don't even resolve the promise until what we're waiting for has actually happened.

Backed out: https://hg.mozilla.org/integration/fx-team/rev/a9cddc54b6f4

I could have sworn that when I tested chrome-document-global-created before it didn't have the url set right but here we are...

Comment on attachment 8412144 [details] [diff] [review] patch Review of attachment 8412144 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/devtools/server/actors/script.js @@ +4796,5 @@ > + }, > + > + /** > + * Called when a new DOM document global is created. Check if the DOM was > + * laoded from an add-on and if so make the window a debuggee. laoded

(In reply to Dave Townsend [:mossop] from comment #9) > > I could have sworn that when I tested chrome-document-global-created before > it didn't have the url set right but here we are... This behavior is not guaranteed! For windows like Options or browser.xul, window.location will be empty or about:blank.

(In reply to Jeferson Hultmann from comment #13) > (In reply to Dave Townsend [:mossop] from comment #9) > > > > I could have sworn that when I tested chrome-document-global-created before > > it didn't have the url set right but here we are... > > > This behavior is not guaranteed! For windows like Options or browser.xul, > window.location will be empty or about:blank. What makes you say that and why is this the case (it wasn't in my tests)?