If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Implement update mechanism for packaged apps

RESOLVED FIXED in mozilla18

Status

()

Core
DOM: Apps
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: sicking, Assigned: fabrice)

Tracking

({feature})

unspecified
mozilla18
feature
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-kilimanjaro:+, blocking-basecamp:+)

Details

(Whiteboard: [LOE:M][WebAPI:P1][qa-])

Attachments

(1 attachment, 5 obsolete attachments)

What this will look like is still to be determined. Either we just re-ping the same URL which we originally downloaded the app from. Or we define a new HTTP-API which the browser can ping to get a list of apps which have new versions available.

Or something else.
Blocks: 728081
blocking-basecamp: --- → +
blocking-kilimanjaro: --- → +
OS: Mac OS X → All
Hardware: x86 → All
(Assignee)

Comment 1

5 years ago
We want to avoid downloading apps that have not changed, since packages can be quite big.

We can do it like add-ons: reference a small update manifest from the app manifest and check this update manifest for new versions (note that we don't mandate a version number currently in apps manifests).

Or maybe just using the If-Modified-Since http header could be enough?
(In reply to Fabrice Desré [:fabrice] from comment #1)
> Or maybe just using the If-Modified-Since http header could be enough?

Using conditional requests is a very clean solution and one that doesn't re

It is usually better to use ETags (If-None-Match), and Necko will take care of that automatically for us.
Sorry, I hit "Save Changes" too early.

(In reply to Brian Smith (:bsmith) from comment #2)
> (In reply to Fabrice Desré [:fabrice] from comment #1)
> > Or maybe just using the If-Modified-Since http header could be enough?
> 
> Using conditional requests is a very clean solution and one that doesn't re

...require a lot of extra work to implement.

> It is usually better to use ETags (If-None-Match), and Necko will take care
> of that automatically for us.

Actually, that's not true because we're not storing these in the Necko HTTP cache, so we'd need to store the HTTP validation information (Last-Modified or ideally the ETag) as part of the metadata of what app has been installed.
(Assignee)

Comment 4

5 years ago
(In reply to Brian Smith (:bsmith) from comment #3)

> Actually, that's not true because we're not storing these in the Necko HTTP
> cache, so we'd need to store the HTTP validation information (Last-Modified
> or ideally the ETag) as part of the metadata of what app has been installed.

We already store an installTime in the DOM registry so we could just reuse it to set the header(s) on the http channel.
(In reply to Fabrice Desré [:fabrice] from comment #4)
> We already store an installTime in the DOM registry so we could just reuse
> it to set the header(s) on the http channel.

It would be good to store the exact validator (Last-Modified or ETag header) to use because then we don't have to worry about clock mismatches between the server and the client or any other such things.
Blocks: 777048
(Assignee)

Comment 6

5 years ago
Created attachment 645699 [details] [diff] [review]
wip Part 1 : add packageURL and lastModified to the DOM registry
(Assignee)

Comment 7

5 years ago
Created attachment 645700 [details] [diff] [review]
wip Part 2 : updater component
Assignee: nobody → fabrice
(Assignee)

Comment 8

5 years ago
Created attachment 645701 [details] [diff] [review]
wip Part 3 : b2g packaging
(Assignee)

Comment 9

5 years ago
These patches use the If-Modified-Since header to only update modified apps. That works well with my local apache install serving static files.

Some things that are still unknown to me:
- Some parts of the user flow : when and which kind of prompts do we want to show to the user?
- The mozApps API may need a new event on the app object "onupdate" to be used by dashboards when we have finished an update.
- Integration with the marketplace and sites using browserID to authenticate users.
please use etag/if-none-match instead of l-m/i-m-s..

the former doesn't have problems with clock warps, is easier to parse, and doesn't have a problem with subsecond granularity.

Updated

5 years ago
Component: General → DOM: Mozilla Extensions
Product: Web Apps → Core
(In reply to Fabrice Desré [:fabrice] from comment #9)
> These patches use the If-Modified-Since header to only update modified apps.
> That works well with my local apache install serving static files.
> 
> Some things that are still unknown to me:
> - Some parts of the user flow : when and which kind of prompts do we want to
> show to the user?
> - The mozApps API may need a new event on the app object "onupdate" to be
> used by dashboards when we have finished an update.
> - Integration with the marketplace and sites using browserID to authenticate
> users.

Might be good to add these open questions here - https://wiki.mozilla.org/Talk:Apps/PackagingProposal. Wil, myself, Krupa have been adding questions here to build an understanding of the packaged apps (documented here - https://wiki.mozilla.org/Apps/PackagingProposal).
(Assignee)

Comment 12

5 years ago
Created attachment 645779 [details] [diff] [review]
wip Part 1 : add packageURL and lastModified to the DOM registry

Using Etag instead of If-Modified-Since
Attachment #645699 - Attachment is obsolete: true
(Assignee)

Comment 13

5 years ago
Created attachment 645781 [details] [diff] [review]
wip Part 2 : updater component

Also updated to use Etag.
Attachment #645700 - Attachment is obsolete: true

Updated

5 years ago
Component: DOM: Mozilla Extensions → DOM: Apps
Hi Fabrice,
I'm going to fit my implementation in bug 744715 into this bug. The attachments was created a few days ago. Please let me know if there's any update from you. Thanks.
Is there any test case?
Blocks: 780662
(Assignee)

Updated

5 years ago
Whiteboard: [LEO:M]
(Assignee)

Updated

5 years ago
Whiteboard: [LEO:M] → [LOE:M]

Updated

5 years ago
QA Contact: jsmith
Whiteboard: [LOE:M] → [LOE:M], [qa+]
No longer blocks: 780662
(Assignee)

Comment 16

5 years ago
re-assigning to mounir.
Assignee: fabrice → mounir

Updated

5 years ago
Whiteboard: [LOE:M], [qa+] → [LOE:M]
Whiteboard: [LOE:M] → [LOE:M][WebAPI:P1]
Keywords: feature
Ping.. this seems to have stalled out?
(Assignee)

Comment 18

5 years ago
That will start again now that we have the API we need to implement designed in bug 790558.
Depends on: 790558
Blocks: 790558
No longer depends on: 790558
Actually, i'm going to close this bug in favor of bug 790558. It tracks the platform work needed to implement update.
No longer blocks: 790558
Depends on: 790558
Arg, I meant to change that comment into a question.

Is there a reason to keep this bug open? I would imagine that bug 790558 will cover the platform work and then we'll have a gaia issue for the remaining UI work.
Actually, ignore my ramblings. We'll just use this bug for implementing the API described in bug 790558 comment 0 during the update of a packaged app. That's basically what it was originally intended for.
Blocks: 790558
No longer depends on: 790558
Fabrice told me he was working/will work on this. Updating the assignee field to make this clearer.
Assignee: mounir → fabrice
Attachment #645701 - Attachment is obsolete: true
Attachment #645779 - Attachment is obsolete: true
Comment on attachment 645781 [details] [diff] [review]
wip Part 2 : updater component

Marking these obsolete as they implement an old proposal.
Attachment #645781 - Attachment is obsolete: true
(Assignee)

Comment 24

5 years ago
Created attachment 666090 [details] [diff] [review]
patch

During update of packaged app:
When we detect that an update is available we set "downloadAvailable" to true and fire "downloadavailable".

When the download() function is called, we set .installState to "updating" and start downloading and fire progress events as needed.

If .launch is called during this time I think we should allow the app to start. But it would be running the old version.

When we're done downloading the package we set readyToApplyDownload to true and fire "downloaded".

When mgmt.applyDownload() is called we swap out the package and fire "downloadapplied".

If cancelDownload() is called we reset .installState to "installed" and .downloadProgress to 0. We also delete all downloaded data for the new version.

If an error occurs we behave like for cancelDownload but also set .downloadError appropriately.


Try run at : https://tbpl.mozilla.org/?tree=Try&rev=e36bdd58dc97
Attachment #666090 - Flags: review?(anygregor)
Comment on attachment 666090 [details] [diff] [review]
patch

>     return request;
>diff --git a/dom/apps/src/Webapps.jsm b/dom/apps/src/Webapps.jsm
>--- a/dom/apps/src/Webapps.jsm
>+++ b/dom/apps/src/Webapps.jsm
>@@ -13,17 +13,17 @@ let EXPORTED_SYMBOLS = ["DOMApplicationR
> 
> Cu.import("resource://gre/modules/XPCOMUtils.jsm");
> Cu.import("resource://gre/modules/Services.jsm");
> Cu.import("resource://gre/modules/FileUtils.jsm");
> Cu.import('resource://gre/modules/ActivitiesService.jsm');
> Cu.import("resource://gre/modules/AppsUtils.jsm");
> 
> function debug(aMsg) {
>-  //dump("-*-*- Webapps.jsm : " + aMsg + "\n");
>+  dump("-*-*- Webapps.jsm : " + aMsg + "\n");
> }


comment


 
>     this.frameMessages = ["Webapps:ClearBrowserData"];
> 
>     this.messages.forEach((function(msgName) {
>       ppmm.addMessageListener(msgName, this);
>     }).bind(this));
> 
>     cpmm.addMessageListener("Activities:Register:OK", this);
>@@ -481,22 +482,28 @@ let DOMApplicationRegistry = {
>         this.addMessageListener(msg, mm);
>         break;
>       case "Webapps:UnregisterForMessages":
>         this.removeMessageListener(msg, mm);
>         break;
>       case "Webapps:GetList":
>         this.addMessageListener(["Webapps:AddApp", "Webapps:RemoveApp"], mm);
>         return this.webapps;
>+      case "Webapps:Download":
>+        this.startDowload(msg.manifestURL);
>+        break;


startDownload!
I guess we don't have many tests :(


>       case "Webapps:CancelDownload":
>         this.cancelDowload(msg.manifestURL);
>         break;

also Download! Please grep for it. There are more dowload in the code!



> 
>+  startDownload: function cancelDowload(aManifestURL) {


startDownload



>+        function(aId, aManifest) {
>+          // Success! Keep the zip in of TmpD, we'll move it out when
>+          // applyDowload() will be called.
>+          let tmpDir = FileUtils.getDir("TmpD", ["webapps", aId], true, true);
>+
>+          // Save the manifest in TmpD also
>+          let manFile = tmpDir.clone();
>+          manFile.append("manifest.webapp");
>+          DOMApplicationRegistry._writeFile(manFile,
>+                                            JSON.stringify(aManifest),
>+                                            function() { });
>+          // Set state and fire events.
>+          app.dowloading = false;
>+          app.dowloadavailable = false;


download




>+    let dir = FileUtils.getDir(DIRECTORY_NAME, ["webapps", id], true, true);
>+    appFile.moveTo(dir, "application.zip");
>+    manFile.moveTo(dir, "manifest.webapp");
>+
>+    try {
>+      tmpDir.remove(true);
>+    } catch(e) { }
>+


Write some log?




>     function sendError(aError) {
>       aData.error = aError;
>       aMm.sendAsyncMessage("Webapps:CheckForUpdate:Return:KO", aData);
>     }
> 
>     function updatePackagedApp(aManifest) {
>       debug("updatePackagedApp");
>+      let manifest = new DOMApplicationManifest(aManifest, app.manifestURL);
>+      // A package is available: set downloadAvailable to fire the matching
>+      // event.
>+      app.downloadAvailable = true;
>+      app.downloadSize = manifest.size;
>+      aData.event = "downloadavailable";
>+      aData.app = {
>+        downloadAvailable: true,
>+        downloadSize: manifest.size
>+      }
>+      DOMApplicationRegistry._saveApps(function() {
>+        aMM.sendAsyncMessage("Webapps:CheckForUpdate:Return:OK", aData);
>+      });

do you mean aMm? 




>       }
>       // Copy the zip on disk.
>       let zipFile = FileUtils.getFile("TmpD",
>                                       ["webapps", id, "application.zip"], true);
>       let ostream = FileUtils.openSafeFileOutputStream(zipFile);
>+      debug("ok 1");

remove

>       NetUtil.asyncCopy(aInput, ostream, function (aResult) {
>         if (!Components.isSuccessCode(aResult)) {
>           // We failed to save the zip.
>           cleanup("DOWNLOAD_ERROR");
>           return;
>         }
>-
>+        debug("ok 2");

remove

>-            DOMApplicationRegistry.broadcastMessage("Webapps:PackageEvent",
>-                                                    { type: "installed",
>-                                                      manifestURL: aApp.manifestURL,
>-                                                      app: app,
>-                                                      manifest: manifest });
>-            delete DOMApplicationRegistry.downloads[aApp.manifestURL]
>-          });
>+          debug("ok 3");

remove
Attachment #666090 - Flags: review?(anygregor) → review+
(Assignee)

Comment 26

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/eae519534b01

Updated

5 years ago
Keywords: verifyme
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/25041824911b - one of the things in that push was causing

9351 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/dom/tests/mochitest/webapps/test_cross_origin.xul | Test timed out.
9354 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/dom/tests/mochitest/webapps/test_getNotInstalled.xul | Test timed out.
9358 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/dom/tests/mochitest/webapps/test_install_app.xul | Test timed out.
9364 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/dom/tests/mochitest/webapps/test_install_errors.xul | Test timed out.
9365 ERROR TEST-UNEXPECTED-FAIL | (SimpleTest/TestRunner.js) | 4 test timeouts, giving up.
9366 ERROR TEST-UNEXPECTED-FAIL | (SimpleTest/TestRunner.js) | Skipping 404 remaining tests.
9367 ERROR TEST-UNEXPECTED-FAIL | (SimpleTest/TestRunner.js) | chrome://mochitests/content/chrome/dom/tests/mochitest/webapps/test_install_errors.xul finished in a non-clean fashion, probably because it didn't call SimpleTest.finish()
(Assignee)

Comment 28

5 years ago
re-pushed since this was not the cause of test failure:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6676711a510b
https://hg.mozilla.org/mozilla-central/rev/6676711a510b
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
This doesn't look right:
>   startDownload: function cancelDownload(aManifestURL) {
https://hg.mozilla.org/mozilla-central/rev/6676711a510b#l2.91

Not sure if the function name matters there though?

Updated

5 years ago
Keywords: verifyme
Whiteboard: [LOE:M][WebAPI:P1] → [LOE:M][WebAPI:P1][qa-]

Updated

5 years ago
No longer blocks: 777048
You need to log in before you can comment on or make changes to this bug.