Fernando Jiménez Moreno [:ferjm] Assignee
	Description • 12 years ago

Currently, we have to start downloading the app and wait for it to fail due to insufficient device storage before triggering the 'ondownloaderror' event. We probably want to check that there is enough device storage *before* starting to download any data.

	Dietrich Ayala (:dietrich)
	Comment 1 • 12 years ago

Should cover the install and update cases.

Blocking+ for required feature work:

AppUpdates-005 "As a user, I want the system to check that there is enough storage before beginning to download apps, so that I can avoid "not enough storage" errors."

	Fernando Jiménez Moreno [:ferjm] Assignee
	Comment 2 • 12 years ago

With this patch the platform checks if there is enough free device storage before starting to download a package and triggers a 'ondownloaderror' in case that there is not enough space.

I've made the patch to simply trigger the 'ondownloaderror' because that is what we agreed to do, but this way the UI won't know if the download failed because of insufficient device storage, a connection error or whatever. If we want to show the specific cause of the download failure we might want to either add more information to 'ondownloaderror' or trigger a new type of event. What do you think?

	Fernando Jiménez Moreno [:ferjm] Assignee
	Comment 4 • 12 years ago

Pointer to Github pull-request

	Fernando Jiménez Moreno [:ferjm] Assignee
	Comment 5 • 12 years ago

Comment on attachment 681457 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/6401

This patch allows the system app to have permissions for accessing apps device storage.

	Fernando Jiménez Moreno [:ferjm] Assignee
	Comment 6 • 12 years ago

(In reply to Fernando Jiménez Moreno [:ferjm] from comment #2)
> Created attachment 681440 [details] [diff] [review]
> v1
> I've made the patch to simply trigger the 'ondownloaderror' because that is
> what we agreed to do, but this way the UI won't know if the download failed
> because of insufficient device storage, a connection error or whatever. If
> we want to show the specific cause of the download failure we might want to
> either add more information to 'ondownloaderror' or trigger a new type of
> event. What do you think?

Please, ignore this comment. I've just realized that the error details are exposed via 'mozIDOMApplication.downloadError' :)

	Fernando Jiménez Moreno [:ferjm] Assignee
	Comment 7 • 12 years ago

Added the fix to include the app manifest along with the 'ondownloaderror' event.

	[:fabrice] Fabrice Desré
	Comment 8 • 12 years ago

Comment on attachment 681555 [details] [diff] [review]
v2

Review of attachment 681555 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/apps/src/Webapps.jsm
@@ +1360,5 @@
> +        onProgress: function notifProgress(aRequest, aContext,
> +                                           aProgress, aProgressMax) {
> +          debug("onProgress: " + aProgress + "/" + aProgressMax);
> +          app.progress = aProgress;
> +          DOMApplicationRegistry.broadcastMessage("Webapps:PackageEvent",

Nit: now that you have "self", you can use it there also.

	Fernando Jiménez Moreno [:ferjm] Assignee
	Comment 9 • 12 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/d95d4e953df5

	Jason Smith [:jsmith]
	Comment 10 • 12 years ago

Marking leave-open until the PR for Gaia lands after the gecko pieces land.

	Ed Morley [:emorley]
	Comment 11 • 12 years ago

https://hg.mozilla.org/mozilla-central/rev/d95d4e953df5

	Josh Carpenter [:jcarpenter]
	Comment 12 • 12 years ago

If storage space check fails during app install, the user needs to be prompted. I've created a bug for that here: https://bugzilla.mozilla.org/show_bug.cgi?id=812369

This is per the Oct 11 spec, page 8, here: https://www.dropbox.com/sh/b0kyykhzcfkpm8b/ReFTX_E72X

	[:fabrice] Fabrice Desré
	Comment 13 • 12 years ago

Comment on attachment 681457 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/6401

Hm, I really don't like that we have to give a content permission to make some chrome content work. I understand that this because you used browser.contentWindow.navigator, but we need to fix that.

Can we just use wm.getMostRecentWindow("...").navigator instead ?

	Fernando Jiménez Moreno [:ferjm] Assignee
	Comment 14 • 12 years ago

You are absolutely right. Thanks!

	Fernando Jiménez Moreno [:ferjm] Assignee
	Comment 15 • 12 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/d0562b40fc01

	Ryan VanderMeulen [:RyanVM]
	Comment 16 • 12 years ago

https://hg.mozilla.org/mozilla-central/rev/d0562b40fc01

	Fernando Jiménez Moreno [:ferjm] Assignee
	Comment 17 • 12 years ago

With the follow up landed there is no need for any other Gaia patch, so I am removing the [leave-open] flag and closing as fixed.

	Ryan VanderMeulen [:RyanVM]
	Comment 18 • 12 years ago

https://hg.mozilla.org/releases/mozilla-beta/rev/7332ccca94de
https://hg.mozilla.org/releases/mozilla-beta/rev/3bac8a2ea386