Last Comment Bug 709088 - Put dump() calls in debugger code behind a pref
: Put dump() calls in debugger code behind a pref
Product: Firefox
Classification: Client Software
Component: Developer Tools: Debugger (show other bugs)
: Trunk
: All All
P2 normal (vote)
: Firefox 13
Assigned To: Panos Astithas [:past]
: Jason Laster [:jlast]
Depends on: 734314
Blocks: minotaur
  Show dependency treegraph
Reported: 2011-12-09 07:55 PST by Panos Astithas [:past]
Modified: 2012-03-09 07:54 PST (History)
3 users (show)
bugzillamozillaorg_serge_20140323: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Working patch (9.81 KB, patch)
2012-03-06 09:30 PST, Panos Astithas [:past]
rcampbell: review+
Details | Diff | Splinter Review

Description User image Panos Astithas [:past] 2011-12-09 07:55:29 PST
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 User image Panos Astithas [:past] 2012-01-22 23:55:26 PST
The removal deadline as per bug 697762 comment 53 is before the debugger migrates to aurora.
Comment 2 User image Panos Astithas [:past] 2012-03-02 06:21:26 PST
I'll get to this next week.
Comment 3 User image Rob Campbell [:rc] (:robcee) 2012-03-05 07:33:15 PST
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 User image Dave Camp (:dcamp) 2012-03-05 09:01:54 PST
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 User image Dave Camp (:dcamp) 2012-03-05 09:04:10 PST
(disclaimer: chatty debugger has been causing me trouble while working on the developer toolbar)
Comment 6 User image Rob Campbell [:rc] (:robcee) 2012-03-05 09:09:55 PST
yeah, if it's annoying, then we can certainly hide it. Thanks!
Comment 7 User image Dave Townsend [:mossop] 2012-03-05 09:59:33 PST
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 User image Panos Astithas [:past] 2012-03-06 09:30:30 PST
Created attachment 603316 [details] [diff] [review]
Working patch

This should do it.
Comment 9 User image Rob Campbell [:rc] (:robcee) 2012-03-07 07:55:48 PST
Comment on attachment 603316 [details] [diff] [review]
Working patch

looks fine.
Comment 10 User image Panos Astithas [:past] 2012-03-08 01:14:31 PST
Comment 11 User image Rob Campbell [:rc] (:robcee) 2012-03-08 06:49:37 PST
Comment 12 User image Serge Gautherie (:sgautherie) 2012-03-09 03:18:26 PST
"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 User image Dave Camp (:dcamp) 2012-03-09 07:41:43 PST
This is being addressed in bug 734314.

Note You need to log in before you can comment on or make changes to this bug.