Closed
Bug 772364
Opened 12 years ago
Closed 12 years ago
Implement update mechanism for packaged apps
Categories
(Core Graveyard :: DOM: Apps, defect)
Core Graveyard
DOM: Apps
Tracking
(blocking-kilimanjaro:+, blocking-basecamp:+)
RESOLVED
FIXED
mozilla18
People
(Reporter: sicking, Assigned: fabrice)
References
Details
(Keywords: feature, Whiteboard: [LOE:M][WebAPI:P1][qa-])
Attachments
(1 file, 5 obsolete files)
18.81 KB,
patch
|
gwagner
:
review+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Updated•12 years ago
|
Blocks: b2g-app-updates
Updated•12 years ago
|
blocking-basecamp: --- → +
blocking-kilimanjaro: --- → +
OS: Mac OS X → All
Hardware: x86 → All
Assignee | ||
Comment 1•12 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?
Comment 2•12 years ago
|
||
(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.
Comment 3•12 years ago
|
||
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•12 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.
Comment 5•12 years ago
|
||
(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.
Updated•12 years ago
|
Blocks: market-packaged-apps
Assignee | ||
Comment 6•12 years ago
|
||
Assignee | ||
Comment 7•12 years ago
|
||
Assignee: nobody → fabrice
Assignee | ||
Comment 8•12 years ago
|
||
Assignee | ||
Comment 9•12 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.
Comment 10•12 years ago
|
||
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•12 years ago
|
Component: General → DOM: Mozilla Extensions
Product: Web Apps → Core
Comment 11•12 years ago
|
||
(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•12 years ago
|
||
Using Etag instead of If-Modified-Since
Attachment #645699 -
Attachment is obsolete: true
Assignee | ||
Comment 13•12 years ago
|
||
Also updated to use Etag.
Attachment #645700 -
Attachment is obsolete: true
Updated•12 years ago
|
Component: DOM: Mozilla Extensions → DOM: Apps
Comment 14•12 years ago
|
||
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.
Comment 15•12 years ago
|
||
Is there any test case?
Updated•12 years ago
|
Blocks: basecamp-updates
Assignee | ||
Updated•12 years ago
|
Whiteboard: [LEO:M]
Assignee | ||
Updated•12 years ago
|
Whiteboard: [LEO:M] → [LOE:M]
Updated•12 years ago
|
QA Contact: jsmith
Whiteboard: [LOE:M] → [LOE:M], [qa+]
Updated•12 years ago
|
No longer blocks: basecamp-updates
Updated•12 years ago
|
Whiteboard: [LOE:M], [qa+] → [LOE:M]
Updated•12 years ago
|
Whiteboard: [LOE:M] → [LOE:M][WebAPI:P1]
Comment 17•12 years ago
|
||
Ping.. this seems to have stalled out?
Assignee | ||
Comment 18•12 years ago
|
||
That will start again now that we have the API we need to implement designed in bug 790558.
Reporter | ||
Updated•12 years ago
|
Reporter | ||
Comment 19•12 years ago
|
||
Actually, i'm going to close this bug in favor of bug 790558. It tracks the platform work needed to implement update.
Reporter | ||
Comment 20•12 years ago
|
||
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.
Reporter | ||
Comment 21•12 years ago
|
||
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.
Comment 22•12 years ago
|
||
Fabrice told me he was working/will work on this. Updating the assignee field to make this clearer.
Assignee: mounir → fabrice
Reporter | ||
Updated•12 years ago
|
Attachment #645701 -
Attachment is obsolete: true
Reporter | ||
Updated•12 years ago
|
Attachment #645779 -
Attachment is obsolete: true
Reporter | ||
Comment 23•12 years ago
|
||
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•12 years ago
|
||
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 25•12 years ago
|
||
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•12 years ago
|
||
Comment 27•12 years ago
|
||
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•12 years ago
|
||
re-pushed since this was not the cause of test failure:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6676711a510b
Comment 29•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
Comment 30•12 years ago
|
||
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•12 years ago
|
No longer blocks: market-packaged-apps
Updated•7 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•