910911 - Add more application data to the webapp.json file in the installation directory

Status: RESOLVED FIXED

(Firefox Graveyard :: Web Apps, defect)

Description

[Marco Castelluccio [:marco]]

Attachment: add_app_data_localinstallation

We should add more data about the application to the webapp.json file, so that we can add applications running in the webapp runtime in the registry (bug 906223).

I think we should store only the necessary data, that is the data that we can't regenerate automatically from the manifests and from the package.

Fabrice, do we need anything else?

Comment 1

[[:fabrice] Fabrice Desré]

Yes, you potentially need more than that. Basically anything that we use to clone app objects could be useful (see https://mxr.mozilla.org/mozilla-central/source/dom/apps/src/AppsUtils.jsm#65)

Comment 2

[Marco Castelluccio [:marco]]

(In reply to Fabrice Desré [:fabrice] from comment #1)
> Yes, you potentially need more than that. Basically anything that we use to
> clone app objects could be useful (see
> https://mxr.mozilla.org/mozilla-central/source/dom/apps/src/AppsUtils.jsm#65)

If I remove from the list the data that can be retrieved from the manifests and from the package, the data that we can avoid storing (that is the things here: http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.jsm#149 and the things that have a default value in cloneAppObject), and the data that I've added in the patch, the only things remaining are:

installTime
downloadAvailable
lastUpdateCheck
updateTime
staged

I think these shouldn't be stored as application data. Actually I think we shouldn't store also the etag and the packageEtag.
This file shouldn't be another copy of the registry that then we need to keep in sync with what we store in the registry. I think we should only store the minimal set of information needed to "reinstall" the application (that is the same information a developer would need to call the mozApps.install function).

Comment 3

[[:fabrice] Fabrice Desré]

Then you only need the manifest url, the manifest itself, installTime, receipts and categories. At this point we consider the application as installed, right? 

downloadAvailable, lastUpdateCheck, updateTime and staged are all used for updates. It's not clear to me if we expect updates to only happen in the firefox profile or also in the individual app profiles.

Comment 4

[Marco Castelluccio [:marco]]

(In reply to Fabrice Desré [:fabrice] from comment #3)
> Then you only need the manifest url, the manifest itself, installTime,
> receipts and categories. At this point we consider the application as
> installed, right? 

And the installOrigin also, to make getInstalled work.

> downloadAvailable, lastUpdateCheck, updateTime and staged are all used for
> updates. It's not clear to me if we expect updates to only happen in the
> firefox profile or also in the individual app profiles.

I don't know if we will trigger updates from Firefox or from the webapp runtime, but the updated data will likely be stored both in the Firefox profile and in the webapp installation directory.

Comment 5

[[:fabrice] Fabrice Desré]

What you must make sure is that the DOM object has all the properties with sane values. Do we have tests for that part of the webrt yet?

Comment 6

[Marco Castelluccio [:marco]]

(In reply to Fabrice Desré [:fabrice] from comment #5)
> What you must make sure is that the DOM object has all the properties with
> sane values. Do we have tests for that part of the webrt yet?

No, not yet. But at the moment the properties are taken from the registry.

When we'll support bug 906223 we'll need to create a test where we run an application that isn't in the registry and check if it's correctly added (it's probably going to be tricky).

Comment 7

[Marco Castelluccio [:marco]]

Attachment: Patch

Comment 8

[[:fabrice] Fabrice Desré]

Comment on attachment 799675 [details] [diff] [review]
Patch

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

::: toolkit/webapps/WebappsInstaller.jsm
@@ +175,2 @@
>      // was installed.
>      let registryFolder = Services.dirsvc.get("ProfD", Ci.nsIFile);

let webapp = aData.app;
and use webapp everywhere you used aData.app

@@ +190,5 @@
>  
> +    if (aData.app.etag) {
> +      this.webappJson.app.etag = aData.app.etag;
> +    }
> +

Don't you want also the packageEtag if available?

Comment 9

[Marco Castelluccio [:marco]]

Attachment: Patch

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/6fbeefd18043

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/6fbeefd18043