881499 - Modify webapps actor to receive a receipt

Status: RESOLVED FIXED

(DevTools :: General, defect)

Description

[(not reading bugmail) Nick Desaulniers [:\n]]

Attachment: patch v1

This is would be helpful for the FxOS Simulator when pushing an app to device to also push the receipt, to test paid apps.

https://github.com/mozilla/r2d2b2g/issues/496

Comment 1

[[:fabrice] Fabrice Desré]

Comment on attachment 760609 [details] [diff] [review]
patch v1

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

Also, can you create your patches with 8 lines of context (see https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F)? thanks!

::: toolkit/devtools/server/actors/webapps.js
@@ +107,4 @@
>      return type;
>    },
>  
> +  installHostedApp: function wa_actorInstallHosted({ aDir, aId, aReceipts }) {

There's no reason to change the signature style here.

@@ +176,4 @@
>                                         Ci.nsIThread.DISPATCH_NORMAL);
>    },
>  
> +  installPackagedApp: function wa_actorInstallPackaged({ aDir, aId, aReceipts }) {

idem.

Comment 2

[(not reading bugmail) Nick Desaulniers [:\n]]

Attachment: patch v2

Comment 3

[[:fabrice] Fabrice Desré]

Comment on attachment 760649 [details] [diff] [review]
patch v2

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

::: toolkit/devtools/server/actors/webapps.js
@@ +270,5 @@
>  
>      let testFile = appDir.clone();
>      testFile.append("application.zip");
>  
> +    let receipts = aRequest.receipt ? [aRequest.receipt] : null;

That will go bad if aRequest.receipt is actually an array.

Comment 4

[(not reading bugmail) Nick Desaulniers [:\n]]

Attachment: patch v3

Comment 5

[[:fabrice] Fabrice Desré]

Comment on attachment 760996 [details] [diff] [review]
patch v3

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

::: toolkit/devtools/server/actors/webapps.js
@@ +270,5 @@
>  
>      let testFile = appDir.clone();
>      testFile.append("application.zip");
>  
> +    let receipts = typeof aRequest.receipt === "string" ? [aRequest.receipt] : null;

We do it differently in the DOM API: https://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.js#134

I'd like both to stay in sync as much as possible, so let me know if you have a good reason to not follow the DOM api pattern.

Comment 6

[(not reading bugmail) Nick Desaulniers [:\n]]

The DOM api expects an array of strings, the simulator only sends a single string or null.

Comment 7

[Jason Smith [:jsmith]]

(In reply to Nick Desaulniers [:\n] from comment #6)
> The DOM api expects an array of strings, the simulator only sends a single
> string or null.

That sounds like a bug in the simulator. I don't see any reason why we shouldn't support multiple receipts.

Comment 8

[[:fabrice] Fabrice Desré]

The point is not was the simulator does, it's what the public sideloading API should accept. I would argue that it has to support an API as close as possible to the DOM API.

Comment 9

[(not reading bugmail) Nick Desaulniers [:\n]]

Attachment: patch v4

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a0e3d36a4acb

Should this have tests?

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/a0e3d36a4acb