Closed
Bug 790870
Opened 12 years ago
Closed 12 years ago
Implement install/update API during installation of hosted apps
Categories
(Core Graveyard :: DOM: Apps, defect)
Core Graveyard
DOM: Apps
Tracking
(blocking-basecamp:+)
People
(Reporter: sicking, Assigned: fabrice)
References
(Depends on 1 open bug, Blocks 1 open bug)
Details
(Keywords: dev-doc-needed, Whiteboard: [LOE:S][qa-])
Attachments
(2 files, 4 obsolete files)
4.57 KB,
patch
|
sicking
:
superreview+
|
Details | Diff | Splinter Review |
30.61 KB,
patch
|
gwagner
:
review+
|
Details | Diff | Splinter Review |
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
Updated•12 years ago
|
blocking-basecamp: --- → ?
Comment 1•12 years ago
|
||
Updated•12 years ago
|
blocking-basecamp: ? → +
Updated•12 years ago
|
Assignee: nobody → fabrice
Assignee | ||
Updated•12 years ago
|
Whiteboard: [LOE:S]
Assignee | ||
Comment 2•12 years ago
|
||
The only change I made is to use a long for the lastUpdateCheck since this is already what we use for installTime.
Attachment #662748 -
Flags: superreview?(jonas)
Assignee | ||
Comment 3•12 years ago
|
||
wip, pushed to try at https://tbpl.mozilla.org/?tree=Try&rev=52afca36060f
Assignee | ||
Comment 4•12 years ago
|
||
Myk, would you have some time to help us by writing proper tests for these changes?
OS: Mac OS X → All
Hardware: x86 → All
Assignee | ||
Comment 5•12 years ago
|
||
test failures on the try push, I'm looking into that.
Assignee | ||
Comment 6•12 years ago
|
||
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.
Attachment #662749 -
Attachment is obsolete: true
Reporter | ||
Updated•12 years ago
|
Attachment #662748 -
Flags: superreview?(jonas) → superreview+
Updated•12 years ago
|
Blocks: Apps-Dev-Doc-Needed
Keywords: dev-doc-needed
Reporter | ||
Comment 7•12 years ago
|
||
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.
Assignee | ||
Comment 8•12 years ago
|
||
I removed the builtinclass, and now test are passing locally. I pushed to try at https://tbpl.mozilla.org/?tree=Try&rev=f16a8bd51959
Attachment #663105 -
Attachment is obsolete: true
Assignee | ||
Comment 9•12 years ago
|
||
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"
Attachment #663473 -
Attachment is obsolete: true
Attachment #664229 -
Flags: review?(anygregor)
Assignee | ||
Comment 11•12 years ago
|
||
Rebased to current tree.
Attachment #664229 -
Attachment is obsolete: true
Attachment #664229 -
Flags: review?(anygregor)
Attachment #664735 -
Flags: review?(anygregor)
Comment 12•12 years ago
|
||
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?
Attachment #664735 -
Flags: review?(anygregor) → review+
Assignee | ||
Comment 13•12 years ago
|
||
(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!
Assignee | ||
Comment 14•12 years ago
|
||
Comment 15•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/2649ac9b652f
https://hg.mozilla.org/mozilla-central/rev/54fcc1121824
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
Updated•12 years ago
|
Whiteboard: [LOE:S] → [LOE:S][qa-]
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
•