Closed Bug 817982 Opened 8 years ago Closed 8 years ago

Add support for preinstalled packaged apps in webapps.jsm

Categories

(Firefox OS Graveyard :: General, defect, P1)

defect

Tracking

(blocking-basecamp:+, firefox19 fixed, b2g18 fixed)

RESOLVED FIXED
B2G C2 (20nov-10dec)
blocking-basecamp +
Tracking Status
firefox19 --- fixed
b2g18 --- fixed

People

(Reporter: ochameau, Assigned: fabrice)

References

Details

(Whiteboard: [qa-])

Attachments

(2 files, 2 obsolete files)

Companion of bug 812198, but for the platform side.
We need to support pre-installing zip files dropped into webapps folder by gaia build system.
blocking-basecamp: --- → ?
I started working on that.
Assignee: nobody → fabrice
Thanks Fabrice, let me know if you need any help on this.
blocking-basecamp: ? → +
Can we get a priority and target milestone for this>
(In reply to Damon Sicore (:damons) from comment #3)
> Can we get a priority and target milestone for this>

It blocks a P1 C2. So I would say it is P1 C2.
Priority: -- → P1
Target Milestone: --- → B2G C2 (20nov-10dec)
Attached patch patch (obsolete) — Splinter Review
Attachment #689329 - Flags: review?(poirot.alex)
Comment on attachment 689329 [details] [diff] [review]
patch

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

Looks good but I wasn't able to have a working example.

I used the following app:
https://github.com/ochameau/gaia/tree/prepackaged/external-apps/packaged
When I open it, I'm seing the firefox logo, which tend to say that it can't load index.html.
I've added various debug and it tries to load app://package/index.html, which is mapped by the app protocol to: file:///data/local/webapps/packaged/application.zip!/index.html
And /data/local/webapps/packaged/application.zip exists and contains an index.html file at root folder.
So everything looks good so far but still doesn't work :(

::: dom/apps/src/Webapps.jsm
@@ +242,5 @@
> +      }
> +      let manifestFile = destDir.clone();
> +      manifestFile.append("manifest.webapp");
> +      zipReader.extract("manifest.webapp", manifestFile);
> +      manifestFile.permissions = FileUtils.PERMS_FILE;

You should close the zip file, ideally in a finally statement:
  zipReader.close();

@@ +355,3 @@
>          for (let id in this.webapps) {
> +          this.updateOfflineCacheForApp(id);
> +          this.installPreinstalledPackage(id);

We should call installPreinstalledPackage first and break if this.webapps[id] has been deleted.
(In reply to Alexandre Poirot (:ochameau) from comment #6)

> And /data/local/webapps/packaged/application.zip exists and contains an
> index.html file at root folder.
> So everything looks good so far but still doesn't work :(

You're probably seeing bug 813468. Try with the follow-up patch applied.

> ::: dom/apps/src/Webapps.jsm
> @@ +242,5 @@
> > +      }
> > +      let manifestFile = destDir.clone();
> > +      manifestFile.append("manifest.webapp");
> > +      zipReader.extract("manifest.webapp", manifestFile);
> > +      manifestFile.permissions = FileUtils.PERMS_FILE;
> 
> You should close the zip file, ideally in a finally statement:
>   zipReader.close();

right

> @@ +355,3 @@
> >          for (let id in this.webapps) {
> > +          this.updateOfflineCacheForApp(id);
> > +          this.installPreinstalledPackage(id);
> 
> We should call installPreinstalledPackage first and break if
> this.webapps[id] has been deleted.

yes, I'll update with that.
Attached patch patch v2 (obsolete) — Splinter Review
Addressing comments.
Attachment #689329 - Attachment is obsolete: true
Attachment #689329 - Flags: review?(poirot.alex)
Attachment #689578 - Flags: review?(poirot.alex)
Comment on attachment 689578 [details] [diff] [review]
patch v2

Tested against following test packaged app:
  https://github.com/ochameau/gaia/tree/prepackaged/external-apps/packaged

Works fine when applying attachment 687621 [details] [diff] [review] from bug 813468 on user build.
But I'm seeing following error on eng build, without any additional app:

E/GeckoConsole(  110): [JavaScript Error: "DOMApplicationRegistry: Could not parse JSON: /data/local/webapps/webapps.json [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsILocalFile.create]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/FileUtils.jsm :: FileUtils_getDir :: line 67"  data: no]
E/GeckoConsole(  110): undefined" {file: "resource://gre/modules/Webapps.jsm" line: 605}]
E/GeckoConsole(  110): [JavaScript Error: "[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsILocalFile.create]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/FileUtils.jsm :: FileUtils_getDir :: line 67"  data: no]" {file: "resource://gre/modules/FileUtils.jsm" line: 67}]
I/Gecko   (  110): ###################################### forms.js loaded
E/GeckoConsole(  110): [JavaScript Error: "NO SETTINGS PERMISSION FOR: app://system.gaiamobile.org
E/GeckoConsole(  110): " {file: "jar:file:///system/b2g/omni.ja!/components/SettingsManager.js" line: 344}]
E/GeckoConsole(  110): [JavaScript Warning: "Expected end of value but found 'border'.  Error in parsing value for 'background-color'.  Declaration dropped." {file: "app://system.gaiamobile.org/style/lockscreen/lockscreen.css" line: 240}]
E/GeckoConsole(  110): [JavaScript Error: "NS_ERROR_FAILURE: Denied" {file: "jar:file:///system/b2g/omni.ja!/components/Webapps.js" line: 701}]
E/GeckoConsole(  110): [JavaScript Error: "NS_ERROR_FAILURE: Denied" {file: "jar:file:///system/b2g/omni.ja!/components/DOMWifiManager.js" line: 380}]
E/GeckoConsole(  110): [JavaScript Error: "SecurityError: The operation is insecure." {file: "app://system.gaiamobile.org/shared/js/idletimer.js" line: 101}]
E/GeckoConsole(  110): [JavaScript Error: "TypeError: settings is null" {file: "app://system.gaiamobile.org/shared/js/settings_listener.js" line: 34}]
E/GeckoConsole(  343): [JavaScript Warning: "Unknown property 'align-self'.  Declaration dropped." {file: "resource://gre-resources/ua.css" line: 45}]
E/GeckoConsole(  343): [JavaScript Warning: "Unknown property 'order'.  Declaration dropped." {file: "resource://gre-resources/ua.css" line: 46}]
E/GeckoConsole(  110): [JavaScript Error: "TypeError: settings is null" {file: "app://system.gaiamobile.org/shared/js/settings_listener.js" line: 34}]
E/GeckoConsole(  110): Content JS WARN at app://system.gaiamobile.org/js/wifi.js:119 in wifi_maybeToggleWifi: Turning off wifi without sleep timer because Alarm API is not available
E/GeckoConsole(  110): [JavaScript Error: "NS_ERROR_FAILURE: Denied" {file: "jar:file:///system/b2g/omni.ja!/components/DOMWifiManager.js" line: 359}]


Can you confirm this issue ?
Attachment #689578 - Flags: review?(poirot.alex)
Attached patch patch v3Splinter Review
This patch fixes the breakage of ENG builds.
Attachment #689578 - Attachment is obsolete: true
Attachment #689972 - Flags: review?(poirot.alex)
Comment on attachment 689972 [details] [diff] [review]
patch v3

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

On eng build, I'm still having an exception here:
  http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.jsm#1365
'maps' app has a `removable: false` in webapps.json, so that baseDir is 'coreAppsDir', i.e. /system/b2g/webapps; and next call to getFile throws as it calls getDir with shouldCreate=true.
I immediatly hit that issue when trying to build patch for this bug.
I ended up always using removable=undefined in gaia build script, here:
  https://github.com/mozilla-b2g/gaia/blob/master/build/webapp-manifests.js#L87
But if I do that with your gecko patch, it looks like packaged apps in eng build are considered as non-packaged one.
When I open my test package app, it tells me that network is unavailable and can't load the app.

I'm fine landing this patch with the gaia patch I'm going to attach (fix broken eng builds),
and open a followup to address packaged app in eng builds.
Attachment #689972 - Flags: review?(poirot.alex) → review+
(In reply to Alexandre Poirot (:ochameau) from comment #11)
> Comment on attachment 689972 [details] [diff] [review]

> https://github.com/mozilla-b2g/gaia/blob/master/build/webapp-manifests.js#L87
> But if I do that with your gecko patch, it looks like packaged apps in eng
> build are considered as non-packaged one.
> When I open my test package app, it tells me that network is unavailable and
> can't load the app.

That's because we don't rewrite the origin in this case. Setting it to app://APP_ID in metadata.json should work.
https://hg.mozilla.org/mozilla-central/rev/efaa95eba566
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: [qa-]
Whiteboard: [qa-][status-b2g18:fixed] → [qa-]
No longer blocks: packaged-apps
You need to log in before you can comment on or make changes to this bug.