bug 1543115: remote: merge init() and listen(); r=remote-protocol-reviewers,maja_zf

Having init() as a separate function leads to inconsistencies about
how the required state is checked.

init() prevents the remote agent from being loaded when the
remote.enabled preference is false or it is attempted loaded into a
child process, but listen() already manipulates state before these
checks are run.  This is probably not the intention, but an easy
mistake to make when the code flow is not crystal clear.

Since we never have a need to call init() independently, this patch
merges init() into listen().

Differential Revision: https://phabricator.services.mozilla.com/D50281

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Andreas Tolfsen 2019-11-22 08:02:56 +00:00
parent 2cf78c4ad5
commit 795f248c54

View File

@ -32,9 +32,13 @@ const DEFAULT_PORT = 9222;
const LOOPBACKS = ["localhost", "127.0.0.1", "[::1]"];
class RemoteAgentClass {
async init() {
get listening() {
return !!this.server && !this.server.isStopped();
}
async listen(address) {
if (!Preferences.get(ENABLED, false)) {
throw new Error("Remote agent is disabled by its preference");
throw new Error("Remote agent is disabled by preference");
}
if (Services.appinfo.processType != Ci.nsIXULRuntime.PROCESS_TYPE_DEFAULT) {
throw new Error(
@ -42,37 +46,6 @@ class RemoteAgentClass {
);
}
if (this.server) {
return;
}
this.server = new HttpServer();
this.targets = new Targets();
// Register the static HTTP endpoints like /json/version or /json/list
this.server.registerPrefixHandler("/json/", new JSONHandler(this));
// Register the dynamic HTTP endpoint of each target.
// These are WebSocket URL where the HTTP request will be morphed
// into a WebSocket connectiong after an handshake.
this.targets.on("target-created", (eventName, target) => {
if (!target.path) {
throw new Error(`Target is missing 'path' attribute: ${target}`);
}
this.server.registerPathHandler(target.path, target);
});
this.targets.on("target-destroyed", (eventName, target) => {
// TODO: This removes the entry added by registerPathHandler, should rather expose
// an unregisterPathHandler method on nsHttpServer.
delete this.server._handler._overridePaths[target.path];
});
}
get listening() {
return !!this.server && !this.server._socketClosed;
}
async listen(address) {
if (!(address instanceof Ci.nsIURI)) {
throw new TypeError(`Expected nsIURI: ${address}`);
}
@ -91,13 +64,25 @@ class RemoteAgentClass {
return;
}
await this.init();
this.server = new HttpServer();
this.server.registerPrefixHandler("/json/", new JSONHandler(this));
// Start watching for targets *after* registering the target listeners
// as this will fire event for already-existing targets.
await this.targets.watchForTargets();
this.targets = new Targets();
this.targets.on("target-created", (eventName, target) => {
if (!target.path) {
throw new Error(`Target is missing 'path' attribute: ${target}`);
}
this.server.registerPathHandler(target.path, target);
});
this.targets.on("target-destroyed", (eventName, target) => {
// TODO: removes the entry added by registerPathHandler,
// but we should instead have nsIHttpServer.unregisterPathHandler
delete this.server._handler._overridePaths[target.path];
});
try {
await this.targets.watchForTargets();
// Immediatly instantiate the main process target in order
// to be accessible via HTTP endpoint on startup
const mainTarget = this.targets.getMainProcessTarget();