For now, we need to push the app package to /tmp/b2g/$(app-id)/application.zip That introduce a hard dependency on adb and requires to find adb and run it on multiple platform. That's not an easy task. We should allow file upload via the remote debugger protocol in order to not depend on adb and eventually offer other communication channels for the remote debugger protocol, like wifi.

I'm waiting for bug 908198 before asking for review for this patch as it depends on it, but this patch is ready. I renable one webapps xpcshell test in order to cover this new feature.

Now with hosted app support.

I'm still using FileUtils, mostly to keep synchronous code when creating empty unique temporary file and automatically mkdir temporary folder. https://tbpl.mozilla.org/?tree=Try&rev=432644fbedf6

Panos, Given the previous feedback on bug 908198, I opened the two followups: bug 912475 and bug 912476

Comment on attachment 799462 [details] [diff] [review] Moved FileUploadActor to webapps actor and updated test to use OS.File Review of attachment 799462 [details] [diff] [review]: ----------------------------------------------------------------- This is much simpler now, cool! I have a few comments, but only the prefix/constructor name is a somewhat important one. ::: toolkit/devtools/apps/tests/unit/test_webappsActor.js @@ +37,5 @@ > > // The install request is asynchronous and send back an event to tell > // if the installation succeed or failed > + gClient.addListener("webappsEvent", function listener(aState, aType, aPacket) { > + gClient.removeListener("webappsEvent", listener); You could just use addOneTimeListener here and avoid having to remove it manually. @@ +191,5 @@ > + .then(function (bytes) { > + let content = new TextDecoder("utf-8").decode(bytes); > + // Encode the typed array as a string as JSON stringify > + // translate them as objects > + content = String.fromCharCode.apply(null, bytes); I'm not familiar with the encoding API, but are you sure that you need to specify UTF-8 (isn't it the default?) and then use charFromCode to get a string? The documentation seems to imply that just decode() should suffice, but I haven't actually tried it. In any case the comment is confusing and needs some rewording. @@ +220,5 @@ > + webappActorRequest(request, function (aResponse) { > + do_check_eq(aResponse.appId, gAppId); > + }); > + gClient.addListener("webappsEvent", function listener(aState, aType, aPacket) { > + gClient.removeListener("webappsEvent", listener); You could use addOneTimeListener here too, if you want. @@ +266,5 @@ > + > + // The install request is asynchronous and send back an event to tell > + // if the installation succeed or failed > + gClient.addListener("webappsEvent", function listener(aState, aType, aPacket) { > + gClient.removeListener("webappsEvent", listener); Same here. ::: toolkit/devtools/server/actors/webapps.js @@ +27,5 @@ > + this.size = 0; > +} > + > +UploadActor.prototype = { > + actorPrefix: "packageUploadActor", It's usually a good idea to have the actor prefix match the constructor, otherwise a new actor might appear that would conflict with one but not the other. Registration would fail if constructors conflicted, but not if prefixes did IIRC. In any case, PackageUploadActor sounds nice. @@ +213,5 @@ > }, > > + uploadPackage: function () { > + debug("uploadPackage\n"); > + let tmpDir = FileUtils.getDir("TmpD", ["file-upload"], true, false); I don't believe you need FileUtils for the temporary directory, there is tmpDir in OS.Constants.Path: https://developer.mozilla.org/docs/JavaScript_OS.File/OS.Path @@ +430,5 @@ > > if (!appDir || !appDir.exists()) { > + if (aRequest.upload) { > + // Ensure creating the directory (recursively) > + appDir = FileUtils.getDir("TmpD", ["b2g", appId], true, false); Same here. @@ +440,5 @@ > + } > + let appFile = FileUtils.File(actor.getFilePath()); > + if (!appFile.exists()) { > + return { error: "badParameter", > + message: "The uploaded file doesn't exists on device" }; Typo: exist.

Comment on attachment 800081 [details] [diff] [review] Address last comments Review of attachment 800081 [details] [diff] [review]: ----------------------------------------------------------------- https://tbpl.mozilla.org/?tree=Try&rev=783b6b144ed6

Wrong try build... this one is going to run xpcshell tests: https://tbpl.mozilla.org/?tree=Try&rev=b4f4ae787b1b (With this patch, we are going to re-enable webapp actor xpcshell test!)

Comment on attachment 800131 [details] [diff] [review] Fix xpcshell failure because of strict exceptions in test_add_actor Review of attachment 800131 [details] [diff] [review]: ----------------------------------------------------------------- Carrying r+ after some fixes in xpcshell tests.