752116 - Allow 'dump' to work in debugger actors

Status: RESOLVED DUPLICATE of bug 753033

(DevTools :: Debugger, defect)

Description

[Jonathan Griffin (:jgriffin)]

Attachment: patch

Because the debugger actors are loaded in a new system principle, they don't have access to dump().  This makes debugging code in the actor, or loaded by the actor, very un-fun.

This patch just imports the dump statement into the sandbox used to import the actor, which allows dump to work in that context.

Comment 1

[Panos Astithas (he/him) [:past] (please ni?)]

(In reply to Jonathan Griffin (:jgriffin) from comment #0)
> Created attachment 621224 [details] [diff] [review]
> patch
> 
> Because the debugger actors are loaded in a new system principle, they don't
> have access to dump().  This makes debugging code in the actor, or loaded by
> the actor, very un-fun.
> 
> This patch just imports the dump statement into the sandbox used to import
> the actor, which allows dump to work in that context.

In dbg-server.js we define a dumpn() function that is used throughout the server code for such purposes. In addition to adding a newline, it is controlled by a pref, so that we can keep debugging code in the product even in production releases. Have you tried using that?

Comment 2

[Jonathan Griffin (:jgriffin)]

(In reply to Panos Astithas [:past] from comment #1)
> 
> In dbg-server.js we define a dumpn() function that is used throughout the
> server code for such purposes. In addition to adding a newline, it is
> controlled by a pref, so that we can keep debugging code in the product even
> in production releases. Have you tried using that?

So, dump() (and thus dumpn) works ok from debugger actors in Firefox, without this patch, but not in B2G.  I don't understand where the difference comes from, but importing dump in the dbg-server sandbox resolves it.

Are there multiple implementations of dump?  If so, it's possible that some work in B2G and others don't, and doing this import causes us to get a version that works in B2G in this context.

Comment 3

[Panos Astithas (he/him) [:past] (please ni?)]

(In reply to Jonathan Griffin (:jgriffin) from comment #2)
> (In reply to Panos Astithas [:past] from comment #1)
> > 
> > In dbg-server.js we define a dumpn() function that is used throughout the
> > server code for such purposes. In addition to adding a newline, it is
> > controlled by a pref, so that we can keep debugging code in the product even
> > in production releases. Have you tried using that?
> 
> So, dump() (and thus dumpn) works ok from debugger actors in Firefox,
> without this patch, but not in B2G.  I don't understand where the difference
> comes from, but importing dump in the dbg-server sandbox resolves it.
> 
> Are there multiple implementations of dump?  If so, it's possible that some
> work in B2G and others don't, and doing this import causes us to get a
> version that works in B2G in this context.

Do you see this problem with using dump() in a real phone? Because it seems to work fine for me in desktop b2g.

Comment 4

[Jonathan Griffin (:jgriffin)]

yes, it only happens on a real phone.  On real phones, the dump command is hooked up to the android logger.

Comment 5

[Jonathan Griffin (:jgriffin)]

E.g., http://mxr.mozilla.org/mozilla-central/source/dom/base/nsGlobalWindow.cpp#4377

Apparently, the implementation of dump that gets called by dbg-server.js and other things loaded by it doesn't go through a code path where this is defined.  Do you know which dump implementation is used in this case?

Comment 6

[Panos Astithas (he/him) [:past] (please ni?)]

(In reply to Jonathan Griffin (:jgriffin) from comment #5)
> E.g.,
> http://mxr.mozilla.org/mozilla-central/source/dom/base/nsGlobalWindow.
> cpp#4377
> 
> Apparently, the implementation of dump that gets called by dbg-server.js and
> other things loaded by it doesn't go through a code path where this is
> defined.  Do you know which dump implementation is used in this case?

I'm not sure ANDROID is defined for b2g. In line 4348 above that part, there is a hint to setting browser.dom.window.dump.enabled to true. I see the following in b2g/app/b2g.js:

// True if you always want dump() to work
//
// On Android, you also need to do the following for the output
// to show up in logcat:
//
// $ adb shell stop
// $ adb shell setprop log.redirect-stdio true
// $ adb shell start
pref("browser.dom.window.dump.enabled", false);

Does that make any difference?

Comment 7

[Jonathan Griffin (:jgriffin)]

We're going to handle this by fixing the various dump implementations so that they go to logcat properly; see bug 753033.