Closed Bug 734314 Opened 10 years ago Closed 10 years ago

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

Categories

(DevTools :: Debugger, defect, P2)

defect

Tracking

(Not tracked)

VERIFIED FIXED
Firefox 13

People

(Reporter: jgriffin, Assigned: jgriffin)

References

Details

(Keywords: regression, Whiteboard: [perma-orange])

Attachments

(1 file, 2 obsolete files)

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.
Attached patch patch (obsolete) — Splinter Review
Attachment #604294 - Flags: review?
Attachment #604294 - Flags: review? → review?(rcampbell)
Attachment #604294 - Flags: review?(rcampbell) → review+
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"?
Blocks: 709088
Severity: normal → major
Keywords: regression
Priority: -- → P2
Target Milestone: --- → Firefox 13
Version: unspecified → Trunk
Attached patch dbg-client.jsm patch (obsolete) — Splinter Review
Thanks Serge.  Here's the corresponding patch for the dbg-client.jsm
Attachment #604457 - Flags: review?(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?
Attachment #604457 - Flags: review?(dcamp) → review+
(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.
Attached patch prefs patchSplinter Review
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.
Attachment #604294 - Attachment is obsolete: true
Attachment #604457 - Attachment is obsolete: true
Attachment #604595 - Flags: review?(past)
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.
Attachment #604595 - Flags: review?(past) → review+
> https://hg.mozilla.org/integration/fx-team/rev/849150e0fb09

Thanks!  Rob (or anyone), can you merge to m-c soonish?
https://hg.mozilla.org/mozilla-central/rev/849150e0fb09
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
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
Assignee: nobody → jgriffin
Status: RESOLVED → VERIFIED
Flags: in-testsuite-
Whiteboard: [perma-orange]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.