790870 - Implement install/update API during installation of hosted apps

Status: RESOLVED FIXED

(Core Graveyard :: DOM: Apps, defect)

Description

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

Description in bug 790558 comment 0 for what this should look like for both apps with an appcache manifest, and for apps without an appcache manifest

Comment 1

[Jason Smith [:jsmith]]

See also https://bugzilla.mozilla.org/show_bug.cgi?id=790872#c1.

Comment 2

[[:fabrice] Fabrice Desré]

Attachment: Part 1 : IDL

The only change I made is to use a long for the lastUpdateCheck since this is already what we use for installTime.

Comment 3

[[:fabrice] Fabrice Desré]

Attachment: Part 2 : hosted apps implementation

wip, pushed to try at https://tbpl.mozilla.org/?tree=Try&rev=52afca36060f

Comment 4

[[:fabrice] Fabrice Desré]

Myk, would you have some time to help us by writing proper tests for these changes?

Comment 5

[[:fabrice] Fabrice Desré]

test failures on the try push, I'm looking into that.

Comment 6

[[:fabrice] Fabrice Desré]

Attachment: Part 2 : hosted apps implementation

This one passes tests, but I'm returning a string instead of a DOMDOMError for downloadError. Returning a real DOMDOMError makes the tests fail, for a reason that I don't understand yet.

Comment 7

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

if you remove 'builtinclass' from nsIDOMDOMError you'll be able to implement it in JS.

If you do do that, please file a followup to have that reverted and add an API for JS to instantiate DOMErrors.

Comment 8

[[:fabrice] Fabrice Desré]

Attachment: Part 2 : hosted apps implementation

I removed the builtinclass, and now test are passing locally. I pushed to try at https://tbpl.mozilla.org/?tree=Try&rev=f16a8bd51959

Comment 9

[[:fabrice] Fabrice Desré]

Attachment: Part 2 : hosted apps implementation

Rebased patch, passing tests locally.

Gregor, here's the behavior implemented:

Installation:
============

Hosted apps *with* an appcache manifest:
---------------------------------------
When the app is first installed its .installState is "pending" and it's "downloadAvailable" and "downloading" properties are set to true. The downloadSize is set to null. As we download the appcache we fire "progress" events. Once the appcache has been downloaded we set .installState to "installed" and fire "downloadsuccess" and "downloadapplied".

Attempting to launch the app while still in "pending" throws an error.

If there's an error while downloading the appcache we set .downloadError appropriately. .installState remains as "pending".

If the cancelDownload() function is called we delete all downloaded data (do we have that ability for appcache?) and set downloadprogress back to 0.


Hosted apps *without* an appcache manifest:
------------------------------------------
When the app is first installed its .state is "installed" and it's "downloadAvailable" and "downloading" properties are set to false.

Attempting to launch the app will succeed.

Updates:
=======

Hosted apps *with* an appcache manifest:
---------------------------------------
Since the appcache can't separate "check for update" from "download and apply update" I think we are pretty limited here. I'm still looking into what we can make work here for v1.


Hosted apps *without* an appcache manifest:
------------------------------------------
When we detect that an updated manifest is availble we simply update the various properties on the app object and fire "downloadapplied"

Comment 11

[[:fabrice] Fabrice Desré]

Attachment: Part 2 : hosted apps implementation

Rebased to current tree.

Comment 12

[Gregor Wagner [:gwagner]]

Comment on attachment 664735 [details] [diff] [review]
Part 2 : hosted apps implementation

> 
>+function debug(aMsg) {
>+  dump("-*-*- Webapps.jsm : " + aMsg + "\n");
>+}


Do you want this in?




>   },
> 
>+  launchApp: function launchApp(aData, aMm) {
>+    let app = this.getAppByManifestURL(aData.manifestURL);
>+    if (!app) {
>+      return;

What does that mean? Shouldn't we throw some exception/error here?



+    // Fire an error when trying to launch an app that is not
+    // yet fully installed.
+    if (app.installState == "pending") {
+      aMm.sendAsyncMessage("Webapps:Launch:Return:KO", aData);
+      return;
+    }

Can we hit this with installState == "updating" as well?


We also never set downloadSize and readyToApplyDownload is always false and 
we don't use lastUpdateCheck. I guess that's coming in the following patches?

Comment 13

[[:fabrice] Fabrice Desré]

(In reply to Gregor Wagner [:gwagner] from comment #12)
> Comment on attachment 664735 [details] [diff] [review]
> Part 2 : hosted apps implementation
> 
> > 
> >+function debug(aMsg) {
> >+  dump("-*-*- Webapps.jsm : " + aMsg + "\n");
> >+}
> 
> 
> Do you want this in?

nope, commented!

> 
> >   },
> > 
> >+  launchApp: function launchApp(aData, aMm) {
> >+    let app = this.getAppByManifestURL(aData.manifestURL);
> >+    if (!app) {
> >+      return;
> 
> What does that mean? Shouldn't we throw some exception/error here?

good catch, I'm sending back an error now.
 
> +    // Fire an error when trying to launch an app that is not
> +    // yet fully installed.
> +    if (app.installState == "pending") {
> +      aMm.sendAsyncMessage("Webapps:Launch:Return:KO", aData);
> +      return;
> +    }
> 
> Can we hit this with installState == "updating" as well?

We can launch while updating since updates are stored in another location while we download them.

> We also never set downloadSize and readyToApplyDownload is always false and 
> we don't use lastUpdateCheck. I guess that's coming in the following patches?

Yes!

Comment 14

[[:fabrice] Fabrice Desré]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2649ac9b652f
https://hg.mozilla.org/integration/mozilla-inbound/rev/54fcc1121824

Comment 15

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/2649ac9b652f
https://hg.mozilla.org/mozilla-central/rev/54fcc1121824