883121 - Replace inefficient call to navigator.mozApps.mgmt.getAll(); on startup of a Webapp.

Status: RESOLVED WONTFIX

(Firefox for Android Graveyard :: Web Apps (PWAs), defect, P3)

Description

[James Hugman [:jhugman] [@jhugman]]

https://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/WebAppRT.js#60

This is prompted by this comment: https://bugzilla.mozilla.org/show_bug.cgi?id=860782#c7

Comment 1

[Marco Castelluccio [:marco]]

Is this really important? It's only triggered during the first startup. And probably the overhead is due to reading the registry file, that we should do anyway in order to get the application data (to "install" the permission).

Comment 2

[Marco Castelluccio [:marco]]

Note that even if you wanted to use directly DOMApplicationRegister, when you import Webapps.jsm you automatically trigger loading the webapps registry file and all the manifest files.

Fabrice what do you think?

Comment 3

[[:fabrice] Fabrice Desré]

GetAll() does a bunch of object copying to create the application objects. So I still think it's better to only get to the manifest in a more direct way, especially on mobile.

Comment 4

[Marco Castelluccio [:marco]]

Oh, actually the manifest files aren't read only during the first startup, but at every startup.

I think this is the main source of overhead, especially on mobile where file reading can be pretty slow.

Does DOMApplicationRegistry really need to read all the manifest files on startup? (http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.jsm#234)

I think to install permissions and to create a <browser> with the correct principal on Android and Desktop we don't need to read all the manifests. We'd just need to read the registry and the manifest of the app we're running.

Comment 5

[[:fabrice] Fabrice Desré]

Yes, we need to read all the manifest files at startup - at least on b2g - to register the system messages. It's not clear to me how these will be managed on desktop and android. Also, we are caching the manifests once they are read - this was a net gain on b2g because the homescreen is doing a getAll() anyway pretty early.

Comment 6

[Marco Castelluccio [:marco]]

(In reply to Fabrice Desré [:fabrice] from comment #5)
> Yes, we need to read all the manifest files at startup - at least on b2g -
> to register the system messages. It's not clear to me how these will be
> managed on desktop and android. Also, we are caching the manifests once they
> are read - this was a net gain on b2g because the homescreen is doing a
> getAll() anyway pretty early.

Yep, I was wondering if this was necessary on Android and Desktop. I don't know what system messages are, but they sounded b2g-specific.

Comment 7

[[:fabrice] Fabrice Desré]

(In reply to Marco Castelluccio [:marco] from comment #6)
> 
> Yep, I was wondering if this was necessary on Android and Desktop. I don't
> know what system messages are, but they sounded b2g-specific.

No they are not b2g-specific. Support is added for desktop because the push api relies on them for instance. Desktop and android need to catch up!

Comment 8

[Marco Castelluccio [:marco]]

(In reply to Fabrice Desré [:fabrice] from comment #7)
> No they are not b2g-specific. Support is added for desktop because the push
> api relies on them for instance. Desktop and android need to catch up!

Thank you for the clarification!

Comment 9

[Myk Melez [:myk] [@mykmelez]]

Per bug 1235869, we're going to disable the Android web runtime, so we won't fix this bug in it.

(This is part of a bulk resolution of bugs in the Firefox for Android::Web Apps component, from which I attempted to exclude bugs that are not specific to the runtime, but it's possible that I included one accidentally.  If so, I'm sorry, and please reopen the bug!)