The goal is to let developers install applications on their phones with privileged status without to use a signing service.

Panagiotis, can you review the remote debugging part? I also have a xpcshell script to test it if you want to try it.

Comment on attachment 700246 [details] [diff] [review] patch v1 Review of attachment 700246 [details] [diff] [review]: ----------------------------------------------------------------- I have a few comments about making the packet form fit well with the rest of the protocol in some cases, but it looks great overall. As an aside: have you considered adding this actor to desktop Firefox as well? I'm not sure if it will be of much benefit to a developer, but we could add tests to exercise it and consider supporting it in Firefox Developer Tools. ::: b2g/chrome/content/dbg-webapps-actors.js @@ +55,5 @@ > + type: "webappsEvent", > + event: "InstallDone", > + subject: null, > + data: { status: "ok" } > + }); I see you mostly copied the eventNotification structure verbatim, which essentially mirrors the observer notifications, but you could simplify a bit. I'd go for: { from: self.actorID, type: "webappsEvent", appId: aId } appId seems useful, otherwise there is no indication which installation this event corresponds to. Note that in the protocol in general we don't return a success status, since the lack of an "error" property denotes success. @@ +72,5 @@ > + subject: null, > + data: { status: "error", > + message: aMsg > + } > + }); In this case I'd just return: { from: this.actorID, type: "webappsEvent", appId, aId, error: "installationFailed", message: aMsg } and make sure that _sendError is called with human-readable descriptive text. aId would have to be provided as a parameter of course. @@ +139,5 @@ > + > + // Move application.zip to the destination directory. > + let zipFile = aDir.clone(); > + zipFile.append("application.zip"); > + zipFile.moveTo(appDir, "application.zip"); Nit: aDir and appDir were slightly confusing to read. If you care, perhaps consider renaming one to installDir or somesuch. @@ +178,5 @@ > + > + // @param appId : The id of the app we want to install. We will look for the > + // files for the app in $TMP/b2g/$appId > + // For packaged apps: application.zip > + // For hosted apps: metadata.json and manifest.webapp A multi-line comment (/* */) would be more consistent, but I won't be picky about it. It would also be cool if the comment described all expected request parameters. @@ +186,5 @@ > + > + let appId = aRequest.appId; > + if (!appId) { > + return { status: "error", > + message: "MISSING_PARAMETER_APPID"} The standard protocol response in this case is: { error: "missingParameter", message: "missing parameter appId" } See: https://wiki.mozilla.org/Remote_Debugging_Protocol#Requests_and_Replies Feel free to make the message as explanatory or verbose as you want. @@ +196,5 @@ > + if (!appDir || !appDir.exists()) { > + return { status: "error", > + message: "MISSING_DIRECTORY", > + detail: appDir.path > + } Same as above, this should return missingParameter if no appDir is specified and badParameterType if the directory is invalid in some way: { error: "badParameterType", message: "missing directory " + appDir.path } @@ +217,5 @@ > + if (missing) { > + try { > + aDir.remove(true); > + } catch(e) {} > + return { status: "error", message: "MISSING_HOSTED_APP_FILE" } Same here: { error: "badParameterType", message: missing + " hosted app file is missing" } @@ +223,5 @@ > + > + this.installHostedApp(appDir, appId, appType); > + } > + > + return { status: "ok", appId: appId, path: appDir.path } Protocol responses in general are considered successful if they don't contain an error property. In this case you'd just return: { appId: appId, path: appDir.path }

Comment on attachment 700246 [details] [diff] [review] patch v1 Review of attachment 700246 [details] [diff] [review]: ----------------------------------------------------------------- ::: b2g/chrome/content/dbg-webapps-actors.js @@ +118,5 @@ > + self._registerApp(app, aId, aDir); > + }); > + } catch(e) { > + // If anything goes wrong, just send it back. > + self.sendError(e.toString()); Don't know why I didn't notice this before, but this should have been self._sendError... (notice the underscore). @@ +166,5 @@ > + > + self._registerApp(app, aId, aDir); > + } catch(e) { > + // If anything goes wrong, just send it back. > + self.sendError(e.toString()); Ditto.

followup for the .sendError I didn't caught myself https://hg.mozilla.org/integration/mozilla-inbound/rev/bd7d549c42ad

https://hg.mozilla.org/releases/mozilla-b2g18/rev/3a5ba563fa69 https://hg.mozilla.org/releases/mozilla-b2g18/rev/608961f27944

https://hg.mozilla.org/mozilla-central/rev/4e1fe4baad60 https://hg.mozilla.org/mozilla-central/rev/bd7d549c42ad