709088 - Put dump() calls in debugger code behind a pref

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P2)

Description

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

There are quite a few dump() calls in the debugger code currently. When we are past the initial development phase and approaching turning it on, we should get rid of them.

Comment 1

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

The removal deadline as per bug 697762 comment 53 is before the debugger migrates to aurora.

Comment 2

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

I'll get to this next week.

Comment 3

[Rob Campbell [:rc] (:robcee)]

In conversation about this today, Panos asked if he could put the dump()s behind a pref as the protocol is still changing and this is useful debugging information for us to have. The debugger is still preffed off by default, so I suggested just leaving them in rather than writing a patch for the sole purpose of hiding these debug statements.

Mossop: Do you think it's reasonable to just leave these in while we finish this feature up? I'd prefer it, though could understand objections to leaving them in a release product.

Comment 4

[Dave Camp (:dcamp)]

How hard would it be to put the dumps behind a separate pref?  Most go through a separate dumpn call already, we could update the rest to go through a special dump that checks a pref.

We could consider leaving that pref turned on by default until we turn the debugger on by default.

Comment 5

[Dave Camp (:dcamp)]

(disclaimer: chatty debugger has been causing me trouble while working on the developer toolbar)

Comment 6

[Rob Campbell [:rc] (:robcee)]

yeah, if it's annoying, then we can certainly hide it. Thanks!

Comment 7

[Dave Townsend [:mossop]]

I'm fine with the dumping not requiring a pref as long as the debugger defaults to off, but if you're going to want to leave it in then you might as well stick it behind a pref now anyway

Comment 8

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

Attachment: Working patch

This should do it.

Comment 9

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 603316 [details] [diff] [review]
Working patch

looks fine.

Comment 10

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

https://hg.mozilla.org/integration/fx-team/rev/1fd0ed258309

Comment 11

[Rob Campbell [:rc] (:robcee)]

https://hg.mozilla.org/mozilla-central/rev/1fd0ed258309

Comment 12

[Serge Gautherie (:sgautherie)]

Thunderbird:
http://hg.mozilla.org/comm-central/rev/a7fa3f3063dc
"Unit test bustage fix from bug 709088 - include devtools.debugger.log preference."

SeaMonkey: (at least)
is affected too.

***

Why is this preference added at application level only when its (2) consumer is in Toolkit?

Comment 13

[Dave Camp (:dcamp)]

This is being addressed in bug 734314.