734314 - Unwrapped getBoolPref call causes dbg-server.jsm to fail to load in B2G

Status: VERIFIED FIXED

(DevTools :: Debugger, defect, P2)

Description

[Jonathan Griffin (:jgriffin)]

This in turn causes Marionette to fail to load in B2G.

The offending call is here:

http://mxr.mozilla.org/mozilla-central/source/toolkit/devtools/debugger/dbg-client.jsm#60

The pref it's trying to read, "devtools.debugger.log", was added to firefox.js in the patch to bug 709088, but in other builds that don't use firefox.js, such as B2G, this will throw and cause failure of the remote debugger.

I'm attaching a patch which wraps this in try/catch.

Comment 1

[Jonathan Griffin (:jgriffin)]

Attachment: patch

Comment 2

[Serge Gautherie (:sgautherie)]

http://mxr.mozilla.org/mozilla-central/search?string=devtools.debugger.log&case=on
What about dbg-client.jsm?

Why not just move the preference definition to "Toolkit"?

Comment 3

[Jonathan Griffin (:jgriffin)]

Attachment: dbg-client.jsm patch

Thanks Serge.  Here's the corresponding patch for the dbg-client.jsm

Comment 4

[Dave Camp (:dcamp)]

Comment on attachment 604457 [details] [diff] [review]
dbg-client.jsm patch

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

This patch is fine, but we should either move the default pref to toolkit (if we care about it showing up in about:config) or remove it from browser.js (if we're ok with create-this-pref-if-you-care).

Panos, do you have a preference?

Comment 5

[Serge Gautherie (:sgautherie)]

(In reply to Dave Camp (:dcamp) from comment #4)

> This patch is fine, but we should either move the default pref to toolkit
> (if we care about it showing up in about:config) or remove it from

I would prefer that: more obvious, less code, less test logs about missing pref (iirc).

> browser.js (if we're ok with create-this-pref-if-you-care).

If you do this, please merge all 3 files in 1 patch.

Comment 6

[Jonathan Griffin (:jgriffin)]

Attachment: prefs patch

This resolves the problem by moving the pref to libpref/init/all.js, where it is available in all gecko builds, and removing it from firefox.js.  dbg-server.jsm and dbg-client.jsm do not need to be changed in this case.

Comment 7

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

Comment on attachment 604595 [details] [diff] [review]
prefs patch

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

Thanks, I wasn't aware of this file. This is the best solution, indeed.

Comment 8

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

https://hg.mozilla.org/integration/fx-team/rev/849150e0fb09

Comment 9

[Jonathan Griffin (:jgriffin)]

> https://hg.mozilla.org/integration/fx-team/rev/849150e0fb09

Thanks!  Rob (or anyone), can you merge to m-c soonish?

Comment 10

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/849150e0fb09

Comment 11

[Serge Gautherie (:sgautherie)]

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1331610010.1331612989.32116.gz
OS X 10.6 comm-central-trunk debug test xpcshell on 2012/03/12 20:40:10

The "47" related failures are gone.

V.Fixed