Closed Bug 770145 Opened 10 years ago Closed 10 years ago

GCLI needs a command to log function calls

Categories

(DevTools :: General, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 16

People

(Reporter: miker, Assigned: jwalker)

References

Details

(Whiteboard: [gclicommands])

Attachments

(1 file, 2 obsolete files)

Attached patch v1 (obsolete) — Splinter Review
Assignee: nobody → jwalker
Status: NEW → ASSIGNED
Attachment #641807 - Flags: review?(past)
Attached patch v2 (obsolete) — Splinter Review
Mihai - I'd like to change the destination of the logs from the error console to the web console. Also I'm being slightly lazy. ;-)
What's the best way to replace onEnterFrame to do that?

Also I'm thinking that I could add:
    let tab = context.environment.chromeDocument.defaultView.gBrowser.selectedTab
    HUDService.activateHUDForContext(tab);

To the end of the 'calllog start'.exec() to open the web console, so people can see where the output is going.
Attachment #641807 - Attachment is obsolete: true
Attachment #641807 - Flags: review?(past)
Attachment #641822 - Flags: review?(past)
Attachment #641822 - Flags: feedback?(mihai.sucan)
Attachment #641822 - Flags: feedback?(bmcbride)
Comment on attachment 641822 [details] [diff] [review]
v2

Review of attachment 641822 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. Just a few nits that you may or may not want to consider.

::: browser/devtools/commandline/GcliCommands.jsm
@@ +331,5 @@
> +               "component javascript");
> +      Services.console.logMessage(msg);
> +    }
> +    catch(ex) {
> +      // Ignore

Are you sure this is what we want here? Wouldn't reporting errors help us make this more robust?

@@ +342,5 @@
> +    }
> +    return "[object " + value.class + "]";
> +  },
> +
> +  framePosition: function(frame) {

This method is unused.

@@ +353,5 @@
> +    return source + ":" + line;
> +  },
> +
> +  callDescription: function(frame) {
> +    let name = frame.callee.name || "<anonymous>";

In dbg-script-actors.js we also support the fn.displayName convention for naming functions. Not terribly important, but some frameworks use it AFAIK.

http://mxr.mozilla.org/mozilla-central/source/toolkit/devtools/debugger/server/dbg-script-actors.js#1647

@@ +367,5 @@
> +  name: "calllog stop",
> +  description: gcli.lookup("calllogStopDesc"),
> +
> +  exec: function(args, context) {
> +    let global = Components.utils.getGlobalForObject(this);

This is redundant and can be removed.
Attachment #641822 - Flags: review?(past) → review+
(In reply to Joe Walker from comment #2)
> Created attachment 641822 [details] [diff] [review]
> v2
> 
> Mihai - I'd like to change the destination of the logs from the error
> console to the web console. Also I'm being slightly lazy. ;-)
> What's the best way to replace onEnterFrame to do that?

I assume you mean replace Services.console.logMessage (onEnterFrame is the necessary debugger hook). Wouldn't contentWindow.console.log work?

> Also I'm thinking that I could add:
>     let tab =
> context.environment.chromeDocument.defaultView.gBrowser.selectedTab
>     HUDService.activateHUDForContext(tab);
> 
> To the end of the 'calllog start'.exec() to open the web console, so people
> can see where the output is going.

Good idea.
Comment on attachment 641822 [details] [diff] [review]
v2

This looks good to me. I suggest you use contentWindow.console.log() instead of the nsIConsoleService.

Otherwise you can do createElement() for the richlistitem and all of the things you want inside, then call outputMessage(), but things will get complicated and it's really prone to errors. If you want to land this patch in time for Firefox 16, I believe console.log() is the safest solution.
Attachment #641822 - Flags: feedback?(mihai.sucan) → feedback+
Attached patch v3Splinter Review
Attachment #641822 - Attachment is obsolete: true
Attachment #641822 - Flags: feedback?(bmcbride)
https://tbpl.mozilla.org/?tree=Fx-Team&rev=e7d2e3c6d861
Whiteboard: [gclicommands] → [gclicommands][fixed-in-fx-team]
Priority: -- → P2
Target Milestone: --- → Firefox 16
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
https://hg.mozilla.org/mozilla-central/rev/e7d2e3c6d861
Whiteboard: [gclicommands][fixed-in-fx-team] → [gclicommands]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.