Closed Bug 888079 Opened 6 years ago Closed 6 years ago

control debugger start behind a pref, control debugger port with pref

Categories

(Firefox for Metro Graveyard :: Browser, defect)

x86_64
Windows 8
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 25

People

(Reporter: ally, Assigned: ally)

References

Details

Attachments

(2 files, 1 obsolete file)

followup from 886546
Assignee: nobody → ally
Attached patch wip (obsolete) — Splinter Review
- wip, code complete.
-has debug code in it, untested due a startup issue that I can't see due to bug 888073
Attached patch pref listenersSplinter Review
Talked to devtools about what is expected when port is changed. Currently on desktop they kill the server & force the user to restart the browser. Here, I kill and restart the server in that case.
Attachment #769930 - Attachment is obsolete: true
Attachment #770356 - Flags: review?(jmathies)
Comment on attachment 770356 [details] [diff] [review]
pref listeners

Since you use 'devtools.debugger.remote-enabled' and 'devtools.debugger.remote-port' multiple times, I'd suggest making them consts at the top of the file.
Attachment #770356 - Flags: review?(jmathies) → review+
Comment on attachment 770356 [details] [diff] [review]
pref listeners

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

::: browser/metro/base/content/browser-ui.js
@@ +194,4 @@
>    },
>  
> +/************************************
> + * Devtools Debugger

nit - your spacing is off on this comment.
Keywords: checkin-needed
(In reply to :Ally Naaktgeboren from comment #2)
> Talked to devtools about what is expected when port is changed. Currently on
> desktop they kill the server & force the user to restart the browser. Here,
> I kill and restart the server in that case.

Actually what happens on desktop Firefox is that the port change is ignored, until a new server instance is started, which will use the new setting. Killing the server would severe any existing connections, which is something that we don't handle well currently, and is IMHO rather heavy-handed for an obscure pref change.
https://hg.mozilla.org/mozilla-central/rev/04c800fb45a6
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 25
You need to log in before you can comment on or make changes to this bug.