Closed
Bug 791039
Opened 12 years ago
Closed 12 years ago
Add support for 3rd party preinstalled apps to build system
Categories
(Firefox OS Graveyard :: GonkIntegration, defect, P1)
Tracking
(blocking-basecamp:+, firefox18 fixed, firefox19 fixed)
RESOLVED
FIXED
blocking-basecamp | + |
People
(Reporter: sicking, Assigned: ochameau)
References
Details
Attachments
(4 files, 3 obsolete files)
(deleted),
text/html
|
vingtetun
:
review+
|
Details |
(deleted),
patch
|
vingtetun
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
vingtetun
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
vingtetun
:
review+
|
Details | Diff | Splinter Review |
Right now our buildsystem can only create preinstalled apps that are uninstallable and which are stored in the /system directory.
We need to enable it to create preinstalled 3rd party apps which can be uninstalled by the user. Ideally these apps would be built to the /data directory, though technically that isn't required as long as they can be uninstalled, and they don't reappear on system update.
I suspect that we don't need a whole lot of platform work here, mostly just support for the buildsystem to preinstall these apps at build time.
Comment 1•12 years ago
|
||
The platform already has support for loading apps from /system and from /data. All we need here is on the gaia build system side.
Updated•12 years ago
|
OS: Mac OS X → Gonk
Hardware: x86 → ARM
Comment 2•12 years ago
|
||
Fabrice says we should close this and open a Gaia bug instead. I did that with https://github.com/mozilla-b2g/gaia/issues/4748.
Status: NEW → RESOLVED
Closed: 12 years ago
OS: Gonk → Mac OS X
Hardware: ARM → x86
Resolution: --- → INVALID
Comment 3•12 years ago
|
||
We have the following requirements for preloaded 3rd party apps:
- These apps come preloaded, optionally with an app-cache. They are
considered as hosted apps.
- They can be upgraded independently of gecko+gaia updates.
- On a factory reset, they should survive (and ideally their app-cache
also).
- On a gaia+gecko update, these apps may be downgraded if the version in
the gecko+gaia update is not the latest one.
We must first change the build system to flash these apps in
/system/b2g/external_apps, with a directory structure such as:
/system/b2g/external_apps/webapps.json (includes the origin, etc.)
/marketplace/manifest.webapp
/marketplace/cache.manifest
/marketplace/cache/index.html
/marketplace/cache/index.css
At first run (including a factory reset) or after gaia+gecko update, we
run the following steps for each 3rd party app:
- copy $APP/manifest.webapp to /data/local/webapps/$APP/manifest.webapp
- add the entry for this app from webapps.json to the app registry.
- set the permissions for this app.
- load the app-cache if any, using the ressources in the cache
subdirectory (eg. reusing the code currently used in the gaia build system).
Updated•12 years ago
|
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Updated•12 years ago
|
Assignee: nobody → fabrice
OS: Mac OS X → Gonk
Hardware: x86 → All
Updated•12 years ago
|
Assignee: fabrice → poirot.alex
Comment 4•12 years ago
|
||
(In reply to Fabrice Desré [:fabrice] from comment #3)
> We have the following requirements for preloaded 3rd party apps:
>
> - These apps come preloaded, optionally with an app-cache. They are
> considered as hosted apps.
> - They can be upgraded independently of gecko+gaia updates.
> - On a factory reset, they should survive (and ideally their app-cache
> also).
> - On a gaia+gecko update, these apps may be downgraded if the version in
> the gecko+gaia update is not the latest one.
>
I believe this is a strong requirement? This create the needs for the external-apps directory in system which is not really elegant.
> We must first change the build system to flash these apps in
> /system/b2g/external_apps, with a directory structure such as:
>
> /system/b2g/external_apps/webapps.json (includes the origin, etc.)
> /marketplace/manifest.webapp
> /marketplace/cache.manifest
> /marketplace/cache/index.html
> /marketplace/cache/index.css
>
> At first run (including a factory reset) or after gaia+gecko update, we
> run the following steps for each 3rd party app:
> - copy $APP/manifest.webapp to /data/local/webapps/$APP/manifest.webapp
This step can be done by Gaia. The manifest.webapp file will live in the userdata.img and will be by default on firstrun or every factory reset.
> - add the entry for this app from webapps.json to the app registry.
> - set the permissions for this app.
> - load the app-cache if any, using the ressources in the cache
> subdirectory (eg. reusing the code currently used in the gaia build system).
I wish those files could have live in /data/local/webapps/$APP/ instead of /system/b2g/externall-apps/ and break the ability to update them with Gecko (or downgrade them). But this is not an option right?
Comment 5•12 years ago
|
||
We can't flash these apps to /data/local since this partition is wiped when doing a factory reset. We should not flash ANYTHING on /data actually.
Assignee | ||
Comment 6•12 years ago
|
||
What vivien meant is to put them in userdata.img. That's what is currently done when not using PRODUCTION flag. This image file is used during factory reset in order to reset /data partition with its content.
Updated•12 years ago
|
Priority: -- → P1
Assignee | ||
Comment 7•12 years ago
|
||
Assignee | ||
Comment 8•12 years ago
|
||
Assignee | ||
Comment 9•12 years ago
|
||
So. In this Gaia pull request, I ship the `cache` folder into the webapp device directory, next to its manifest.webapp file.
And in the two previous platform patches, I read this cache folder, search for the /cache/manifest.appcache file, parse it and automatically inject all cache entry into the appcache service.
It is slightly different from what has been described in comment 3.
The cache folder can contain cache entry for multiple domains.
So that the layout will look like this:
../apps/webapps.json (includes the origin, etc.)
/marketplace/manifest.webapp
/marketplace/cache/manifest.appcache
/marketplace/cache/marketplace.mozilla.org/index.html
/marketplace/cache/marketplace.mozilla.org/index.css
/marketplace/cache/persona.mozilla.org/include.js
The other sensible different is that I haven't created a dedicated folder for external apps on device (yet). If it really is usefull, I'd prefer doing it in a followup patch.
Having said that I only have weak opinion on what we should do. I'm focused on getting stuff done. So suggestions are welcomed.
Assignee | ||
Updated•12 years ago
|
Attachment #672840 -
Flags: review?(21)
Assignee | ||
Updated•12 years ago
|
Attachment #672841 -
Flags: review?(21)
Assignee | ||
Updated•12 years ago
|
Attachment #672847 -
Flags: review?(21)
Assignee | ||
Comment 10•12 years ago
|
||
Carry over r+ from bug Bug 803071.
Attachment #673018 -
Flags: review+
Assignee | ||
Updated•12 years ago
|
Attachment #672840 -
Attachment description: Factorize code executed after system app install. (`onAppsLoaded` function) → Part 1 - Factorize code executed after system app install. (`onAppsLoaded` function)
Assignee | ||
Updated•12 years ago
|
Attachment #672841 -
Attachment description: Bug 791039: Fill webapp appcache from its local `cache` folder. → Part 2 - Fill webapp appcache from its local `cache` folder.
Comment 11•12 years ago
|
||
Comment on attachment 672840 [details] [diff] [review]
Part 1 - Factorize code executed after system app install. (`onAppsLoaded` function)
Review of attachment 672840 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/apps/src/Webapps.jsm
@@ +177,5 @@
> + try {
> + file = FileUtils.getFile("coreAppsDir", ["webapps", "webapps.json"], false);
> + } catch(e) { }
> +
> + if (file && file.exists) {
Don't forget to merge that with you typo fix.
Attachment #672840 -
Flags: review?(21) → review+
Comment 12•12 years ago
|
||
Comment on attachment 672841 [details] [diff] [review]
Part 2 - Fill webapp appcache from its local `cache` folder.
Review of attachment 672841 [details] [diff] [review]:
-----------------------------------------------------------------
R+ with nits fixed.
::: dom/apps/src/OfflineCacheInstaller.jsm
@@ +6,5 @@
> +
> +const Cu = Components.utils;
> +const Cc = Components.classes;
> +const Ci = Components.interfaces;
> +const Cr = Components.results;
nit: Cr is unused.
@@ +98,5 @@
> +const OfflineCacheInstaller = {
> + installCache: function installCache(app) {
> + let cacheDir = Cc["@mozilla.org/file/local;1"]
> + .createInstance(Ci.nsIFile);
> + cacheDir.initWithPath(app.basePath);
You may want to create a new File constructor here with CC.
@@ +102,5 @@
> + cacheDir.initWithPath(app.basePath);
> + cacheDir.append(app.appId);
> + cacheDir.append("cache");
> + if (!cacheDir.exists())
> + return;
nit: add a line break
@@ +118,5 @@
> + // a browser element, but a mozbrowser iframe.
> + // See netwerk/cache/nsDiskCacheDeviceSQL.cpp: AppendJARIdentifier
> + let groupID = appcacheURL + '#' + app.localId+ '+f';
> + let applicationCache = applicationCacheService.createApplicationCache(groupID);
> + applicationCache.activate();
nit: indets seems broken here :)
@@ +146,5 @@
> + else if (line.substr(0, 4) == 'http')
> + urls.push(line);
> + else
> + throw new Error('Invalid line in appcache manifest:\n' + line +
> + '\nFrom: ' + cacheManifest.path);
Use brackets for if/else if/else. Mozilla guidelines are to not use them only if this is a if one liner.
Attachment #672841 -
Flags: review?(21) → review+
Comment 13•12 years ago
|
||
Comment on attachment 672847 [details]
Pull request 5885
r+ with nits fixed.
Attachment #672847 -
Flags: review?(21) → review+
Comment 14•12 years ago
|
||
Comment on attachment 672841 [details] [diff] [review]
Part 2 - Fill webapp appcache from its local `cache` folder.
Review of attachment 672841 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/apps/src/OfflineCacheInstaller.jsm
@@ +35,5 @@
> + let originURI = Services.io.newURI(origin, null, null);
> + let principal = Services.scriptSecurityManager.getAppCodebasePrincipal(
> + originURI, appId, false);
> + Services.perms.addFromPrincipal(principal, 'offline-app',
> + Ci.nsIPermissionManager.ALLOW_ACTION);
You also need to add the "pin-app" permission to prevent these caches to be evicted.
@@ +47,5 @@
> + onCacheEntryAvailable: function (cacheEntry, accessGranted, status) {
> + cacheEntry.setMetaDataElement('request-method', 'GET');
> + cacheEntry.setMetaDataElement('response-head', 'HTTP/1.1 200 OK\r\n');
> + // Force an update. the default expiration time is way too far in the future:
> + //cacheEntry.setExpirationTime(0);
Will this line stay commented?
Comment 15•12 years ago
|
||
(In reply to Fabrice Desré [:fabrice] from comment #14)
> Comment on attachment 672841 [details] [diff] [review]
> Part 2 - Fill webapp appcache from its local `cache` folder.
>
> Review of attachment 672841 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> ::: dom/apps/src/OfflineCacheInstaller.jsm
> @@ +35,5 @@
> > + let originURI = Services.io.newURI(origin, null, null);
> > + let principal = Services.scriptSecurityManager.getAppCodebasePrincipal(
> > + originURI, appId, false);
> > + Services.perms.addFromPrincipal(principal, 'offline-app',
> > + Ci.nsIPermissionManager.ALLOW_ACTION);
>
> You also need to add the "pin-app" permission to prevent these caches to be
> evicted.
Oups. I forgot that. Thanks for pointing it out.
Assignee | ||
Comment 16•12 years ago
|
||
Assignee | ||
Comment 17•12 years ago
|
||
Assignee | ||
Comment 18•12 years ago
|
||
Attachment #673908 -
Flags: review+
Attachment #673907 -
Flags: review+
Attachment #673905 -
Flags: review+
Attachment #673018 -
Attachment is obsolete: true
Attachment #672841 -
Attachment is obsolete: true
Attachment #672840 -
Attachment is obsolete: true
Assignee | ||
Comment 19•12 years ago
|
||
Assignee | ||
Comment 20•12 years ago
|
||
Try seems mostly green, I don't think oranges are related to this change?
Keywords: checkin-needed
Comment 21•12 years ago
|
||
(In reply to Alexandre Poirot (:ochameau) from comment #20)
> Try seems mostly green, I don't think oranges are related to this change?
Yep. Note that you can retrigger failing tests if you want to be extra sure (not needed here, let's save some IT resources :) ).
Comment 22•12 years ago
|
||
Updated•12 years ago
|
Keywords: checkin-needed
Comment 23•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/fb90e14a89f7
https://hg.mozilla.org/mozilla-central/rev/04176aaf8ee4
https://hg.mozilla.org/mozilla-central/rev/5cce74c60214
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Comment 24•12 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/30d6183f4dea
https://hg.mozilla.org/releases/mozilla-aurora/rev/c210cb087be3
https://hg.mozilla.org/releases/mozilla-aurora/rev/13a355eeeff8
status-firefox18:
--- → fixed
status-firefox19:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•