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

RESOLVED FIXED in Firefox 26

Status

RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: marco, Assigned: marco)

Tracking

Trunk
Firefox 26

Details

Attachments

(1 attachment, 2 obsolete attachments)

Created attachment 797498 [details] [diff] [review]
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?
Attachment #797498 - Flags: review?(fabrice)
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)
Attachment #797498 - Flags: review?(fabrice)
(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).
Flags: needinfo?(fabrice)
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.
Flags: needinfo?(fabrice)
(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.
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?
(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).
Created attachment 799675 [details] [diff] [review]
Patch
Assignee: nobody → mcastelluccio
Attachment #797498 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #799675 - Flags: review?(myk)
Attachment #799675 - Flags: review?(fabrice)
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?
Attachment #799675 - Flags: review?(fabrice) → feedback+
Created attachment 801677 [details] [diff] [review]
Patch
Attachment #799675 - Attachment is obsolete: true
Attachment #799675 - Flags: review?(myk)
Attachment #801677 - Flags: review?(myk)
Attachment #801677 - Flags: review?(fabrice)
Attachment #801677 - Flags: review?(fabrice) → review+
Attachment #801677 - Flags: review?(myk) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/6fbeefd18043
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.