796198 - Use the right cookie jar when downloading package and minimanifest for packaged apps

Status: RESOLVED FIXED

(Core Graveyard :: DOM: Apps, defect, P1)

Description

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

When downloading the mini manifest as well as the app package we should use the same cookie jar as was used to load the page which called apps.install/installPackage.

This means that we for all packaged apps have to remember which cookie jar was used at the time of the install call and then make sure to use the same cookie jar during both the initial download as well as during updates.

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I don't think we need any additional necko support for this. "All" we need to do is to create a custom nsILoadContext and hook up to the channels used for the mini manifest and the package.

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Nominating for blocking as I my understanding is that the marketplace needs this in order to restrict access to non-reviewed packaged apps such that only reviewers can install them.

Comment 3

[[:fabrice] Fabrice Desré]

We currently download the manifests from the content process, and I think it would be easier to implement that cookie jar if we move all the network access in the parent since this is where we have direct access to all the meta-data associated with apps.

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Indeed, we'd have to do that since we're planning on adding security checks in the necko code which prevents apps from using each other's cookie jars.

Comment 5

[Andrew Overholt [:overholt]]

I know you're at TPAC, Jonas, but can you provide a status update when you get home?  Should we give this to someone else?

Comment 6

[Alex Keybl [:akeybl]]

This bug has been called out as likely having risk to non-B2G platforms. Given that, marking as P1, and moving into the C2 milestone. We should prioritize this landing to mozilla-beta as soon as possible, to prevent late-breaking regressions to other platforms.

Comment 7

[Jason Smith [:jsmith]]

Not marketplace related - removing from tracking bug.

Comment 8

[Dietrich Ayala (:dietrich)]

Jonas, there's been no update from you since October. What needs to happen in this bug?

Comment 9

[Wil Clouser [:clouserw]]

What is the status of this bug?

Comment 10

[[:fabrice] Fabrice Desré]

stealing from Jonas since he has no time to work on this.

Comment 11

[[:fabrice] Fabrice Desré]

Attachment: part 1 : move all the manifests loading into the parent

This patch is just doing some preliminary set up, moving all the network accessing code into the parent.

Comment 12

[[:fabrice] Fabrice Desré]

Attachment: part 2 : set the cookie jars

In this part we save the installer appId and isBrowser flag, and reuse it on all the network channels except appcache downloads.

Unfortunately I could not really test it since the reviewer tools  from the marketplace app redirect into the browser.

Comment 13

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 689943 [details] [diff] [review]
part 2 : set the cookie jars

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

r=me with that fixed.

::: dom/apps/src/Webapps.jsm
@@ +1209,5 @@
>    },
>  
> +  // Creates a nsILoadContext object with a given appId and isBrowser flag.
> +  createLoadContext: function createLoadContext(aAppId, aIsBrowser) {
> +    return LoadContextCallback.prototype = {

This looks weird. Why are you setting LoadContextCallback.prototype rather than simply returning the new object?

Comment 14

[[:fabrice] Fabrice Desré]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8a2d8c34d905
https://hg.mozilla.org/integration/mozilla-inbound/rev/9f29dda59ef7

Comment 15

[(no longer active)]

Backed out because this broke mochitest-chrome tests:

https://hg.mozilla.org/integration/mozilla-inbound/rev/b901ea202aeb
https://hg.mozilla.org/integration/mozilla-inbound/rev/eedefdd3375d

Examples: https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=9f29dda59ef7

Comment 16

[[:fabrice] Fabrice Desré]

Now with tests passing:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f4c6bbc050f6
https://hg.mozilla.org/integration/mozilla-inbound/rev/dfaef416ac16

Comment 17

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/f4c6bbc050f6
https://hg.mozilla.org/mozilla-central/rev/dfaef416ac16

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4e628b270773
https://hg.mozilla.org/releases/mozilla-aurora/rev/0a3bc21498fd

https://hg.mozilla.org/releases/mozilla-b2g18/rev/f61ba33ba987
https://hg.mozilla.org/releases/mozilla-b2g18/rev/796f6f5dd6e8