Implement a WebSocket server for debugging.

[ejpbruel]

This pull request adds a very simple WebSocket server to Servo, that we intend to use for debugging. It currently only echoes back messages, but eventually we want this server to implement the Chrome Debugging Protocol.

---

• ./mach build -d does not report any errors
• ./mach test-tidy does not report any errors
• These changes fix #__ (github issue number if applicable).

• There are tests for these changes OR
• These changes do not require tests because this is a prototype.

---

This change is

[highfive]

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @emilio (or someone else) soon.

[jdm]

-S-awaiting-review +S-needs-code-changes

---

Reviewed 6 of 6 files at r1.
Review status: all files reviewed at latest revision, 4 unresolved discussions.

---

components/debugger/lib.rs, line 6 [r1] (raw file):

#![crate_name = "debugger"]
#![crate_type = "rlib"]

http://doc.crates.io/manifest.html#building-dynamic-or-static-libraries

---

components/servo/Cargo.lock, line 475 [r1] (raw file):

]

[[package]]

Run ./mach update-cargo -p servo so cef/Cargo.lock is updated as well.

---

components/servo/lib.rs, line 129 [r1] (raw file):

                                                                opts.time_profiler_trace_path.clone());
        let mem_profiler_chan = profile_mem::Profiler::create(opts.mem_profiler_period);
        opts.debugger_port.map(|port| {

if let insteawd.

---

components/util/opts.rs, line 570 [r1] (raw file):

    opts.optflag("f", "hard-fail", "Exit on thread failure instead of displaying about:failure");
    opts.optflag("F", "soft-fail", "Display about:failure on thread failure instead of exiting");
    opts.optflagopt("", "debugger", "Start remote debugger server on port", "2794");

I would prefer jsdebugger, since otherwise this could conflict in a confusing fashion with a forthcoming --debugger [binary] argument for ./mach run.

---

Comments from Reviewable

[ejpbruel]

components/debugger/lib.rs, line 6 [r1](raw file):

#![crate_name = "debugger"]
#![crate_type = "rlib"]
http://doc.crates.io/manifest.html#building-dynamic-or-static-libraries

I am not sure how to interpret this. Are you saying I shouldn't use crate_type here (note that I've done so because the devtools component does)? Please clarify.

components/servo/lib.rs, line 129 [r1](raw file):

                                                            opts.time_profiler_trace_path.clone());
    let mem_profiler_chan = profile_mem::Profiler::create(opts.mem_profiler_period);
    opts.debugger_port.map(|port| {

if let insteawd.

Should we change the surrounding code (i.e. line 132) to use if let as well?

opts.optflag("f", "hard-fail", "Exit on thread failure instead of displaying about:failure");
opts.optflag("F", "soft-fail", "Display about:failure on thread failure instead of exiting");
opts.optflagopt("", "debugger", "Start remote debugger server on port", "2794");
I would prefer jsdebugger, since otherwise this could conflict in a confusing fashion with a forthcoming --debugger [binary] argument for ./mach run.

How about --remote-debugging-port? This is consistent what Chrome uses, and avoids the name clashes you mentioned.

[jdm]

-S-awaiting-review +S-needs-code-changes +S-fails-tidy

---

Reviewed 3 of 3 files at r2.
Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

components/debugger/lib.rs, line 6 [r1] (raw file):

Previously, jdm (Josh Matthews) wrote…

http://doc.crates.io/manifest.html#building-dynamic-or-static-libraries

Since we can do this in the Cargo.toml instead, we should. The devtools code is from a time before it was possible, iirc.

---

components/util/opts.rs, line 731 [r2] (raw file):

    let debugger_port = opt_match.opt_default("remote-debugging-port", "2794").map(|port| {
        port.parse().unwrap_or_else(|err| args_fail(&format!("Error parsing option: --debugger ({})", err)))

Update the error message, too.

---

Comments from Reviewable

[fitzgen]

NIt: ;;

[jdm]

-S-awaiting-review +S-needs-code-changes

---

Reviewed 5 of 5 files at r3.
Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

Comments from Reviewable

[jdm]

@bors-servo: r+

---

Reviewed 4 of 4 files at r4.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

Comments from Reviewable

[bors-servo]

📌 Commit 8856671 has been approved by jdm

[bors-servo]

⌛ Testing commit 8856671 with merge f834c89...

[bors-servo]

☀️ Test successful - arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows-dev