Blair has created the command at: https://github.com/Unfocused/gcli-commands/blob/master/chrome-calllog.mozcmd

Need to find out where the leaks are coming from ... I suspect a debugger bug

dcamp says to try dbg.enabled = false;

No leaks, green on try

Comment on attachment 645751 [details] [diff] [review] Patch Review of attachment 645751 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/devtools/commandline/commands/GcliCalllogChromeCommands.jsm @@ +86,5 @@ > + } catch(e) { > + // We need to save the message before cleaning up else e contains a dead > + // object. > + let msg = gcli.lookup("callLogChromeEvalException") + ": " + e; > + cleanUp(); If we get an exception adding a second or third call-logger, this will kill all other existing call-loggers - which seems wrong/unexpected.

(In reply to Blair McBride (:Unfocused) from comment #4) > Comment on attachment 645751 [details] [diff] [review] > Patch > > Review of attachment 645751 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: browser/devtools/commandline/commands/GcliCalllogChromeCommands.jsm > @@ +86,5 @@ > > + } catch(e) { > > + // We need to save the message before cleaning up else e contains a dead > > + // object. > > + let msg = gcli.lookup("callLogChromeEvalException") + ": " + e; > > + cleanUp(); > > If we get an exception adding a second or third call-logger, this will kill > all other existing call-loggers - which seems wrong/unexpected. Of course it would, fixed.

Rebased due to HUDService changes

Remove dependency on alias command

Comment on attachment 649254 [details] [diff] [review] Rebased Review of attachment 649254 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/devtools/commandline/commands/GcliCalllogChromeCommands.jsm @@ +30,5 @@ > + * 'calllog chromestart' command > + */ > +gcli.addCommand({ > + name: "calllog chromestart", > + description: gcli.lookup("calllogChromeStartDesc"), So I think these commands will be visible to any GCLI users right? Can I suggest: get hidden() { return gcli.hiddenByChromePref(); }, And then we'll need to add to gcli.jsm to make that work let prefSvc = "@mozilla.org/preferences-service;1"; XPCOMUtils.defineLazyGetter(this, "prefBranch", function() { let prefService = Cc[prefSvc].getService(Ci.nsIPrefService); return prefService.getBranch(null).QueryInterface(Ci.nsIPrefBranch2); }); gcli.hiddenByChromePref = function() { // will need to check the name of the pref return !prefBranch.prefHasUserValue("devtools.chrome.enabled"); }; And to do the latter you'll need to muck with GCLI, which should be something like: $ git clone git@github.com:joewalker/gcli.git $ cd gcli $ npm install . $ vi mozilla/gcli/index.js (add code above) $ node gcli firefox $FIREFOX_HOME (if you get problems about fs.existsSync, see https://github.com/joewalker/gcli/pull/3)

Also see https://github.com/mozilla/gcli/tree/bug-771526 for the gcli side (included in this patch) Works fine except for the

... except for the help (bug 781856).

Comment on attachment 650921 [details] [diff] [review] Addressed reviewers comments Review of attachment 650921 [details] [diff] [review]: ----------------------------------------------------------------- I like it, thanks. ::: browser/devtools/commandline/CmdCalllogChrome.jsm @@ +37,5 @@ > + { > + name: "sourceType", > + type: { > + name: "selection", > + data: ["content-variable", "chrome-variable", "jsm", "javascript"] If we were being posh we might consider having 4 parameters, so you could say: >> calllog chromestart --sourcejsm blah.jsm That way we could have a type which is the list of known JSMs and another which is a list of the known javascript files, etc. However that's probably quite hard, so I think the way you've done it makes sense. @@ +62,5 @@ > + } else if (args.sourceType == "content-variable") { > + if (args.source in contentWindow) { > + globalObj = Cu.getGlobalForObject(contentWindow[args.source]); > + } else { > + return gcli.lookup("callLogChromeVarNotFoundContent"); I think we can probably do: throw new Error(gcli.lookup("...")); The output should still show up as normal, but GCLI will know something went wrong, and will be able to act accordingly. ::: browser/devtools/commandline/test/browser_cmd_calllog_chrome.js @@ +1,2 @@ > +/* Any copyright is dedicated to the Public Domain. > +* http://creativecommons.org/publicdomain/zero/1.0/ */ Just random context: I've made some tweaks to the test helpers in GCLI. See for example https://github.com/joewalker/gcli/blob/multinode-770213/lib/gclitest/testIncomplete.js#L96 I'll probably add testing of async results to this and then port it across. But we certainly should not delay new tests for me to do that.

(In reply to Joe Walker from comment #11) > Comment on attachment 650921 [details] [diff] [review] > Addressed reviewers comments > > Review of attachment 650921 [details] [diff] [review]: > ----------------------------------------------------------------- > > I like it, thanks. > > ::: browser/devtools/commandline/CmdCalllogChrome.jsm > @@ +37,5 @@ > > + { > > + name: "sourceType", > > + type: { > > + name: "selection", > > + data: ["content-variable", "chrome-variable", "jsm", "javascript"] > > If we were being posh we might consider having 4 parameters, so you could > say: > >> calllog chromestart --sourcejsm blah.jsm > > That way we could have a type which is the list of known JSMs and another > which is a list of the known javascript files, etc. However that's probably > quite hard, so I think the way you've done it makes sense. > > @@ +62,5 @@ > > + } else if (args.sourceType == "content-variable") { > > + if (args.source in contentWindow) { > > + globalObj = Cu.getGlobalForObject(contentWindow[args.source]); > > + } else { > > + return gcli.lookup("callLogChromeVarNotFoundContent"); > > I think we can probably do: > throw new Error(gcli.lookup("...")); > > The output should still show up as normal, but GCLI will know something went > wrong, and will be able to act accordingly. > Done > ::: browser/devtools/commandline/test/browser_cmd_calllog_chrome.js > @@ +1,2 @@ > > +/* Any copyright is dedicated to the Public Domain. > > +* http://creativecommons.org/publicdomain/zero/1.0/ */ > > Just random context: I've made some tweaks to the test helpers in GCLI. See > for example > https://github.com/joewalker/gcli/blob/multinode-770213/lib/gclitest/ > testIncomplete.js#L96 > I'll probably add testing of async results to this and then port it across. > But we certainly should not delay new tests for me to do that.

Considering what you did in bug 782820, this string should spell "JavaScript" callLogChromeEvalException=Evaluated javascript threw the following exception

(In reply to Francesco Lodolo [:flod] from comment #15) > Considering what you did in bug 782820, this string should spell "JavaScript" > callLogChromeEvalException=Evaluated javascript threw the following exception bug 786193 logged