876980 - Expose mozAlarms API to installed apps on Firefox desktop

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Should be a simple matter of flipping the pref and other minor changes.

Gene, would you like to take this as a mentored bug?

Comment 1

[[:fabrice] Fabrice Desré]

That does not belong in DOM:Apps, probably DOM:Devices instead.

Comment 2

[Gene Lian (I already quit Mozilla)]

Hi Nikhil,

Marking you as the mentor if you don't mind. Thank you for addressing this topic!

Comment 3

[Mina Almasry]

Hey I think I can take this on. Where do I start with this?

Comment 4

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Mina,

You'll want to edit the browser preferences in browser/app/profile/firefox.js similar to b2g/app/b2g.js so that alarms can be enabled. After that you need to write (or use one on the internet) a simple application that can be installed as an app on firefox desktop to check if alarms are working.

The code for alarms is in dom/alarm while that of system messages (which are used to notify apps when an alarm fires) is in dom/messages.

Comment 5

[Marco Castelluccio [:marco]]

Mina are you still working on this?

Comment 6

[Mina Almasry]

I'm planning on working on it more in a week or so when I have less on my plate, but if you'd like to work on it yourself feel free to take it.

Comment 7

[Mina Almasry]

Actually, I'll just reassign it to myself when I start working on it, if it's still available.

Comment 8

[k]

Attachment: webapi.patch

Extremly trivial patch (only 3 lines added) enabling Alarms API in apps started with webappsrt.

Comment 9

[k]

There are two problems with above patch that should be addresed:

1. Alarms are not preserved between webapp restarts (they are lost after user closes the app). I'm currently checking how to address this issue. It is posibly related to the next one.

2. There is no way an app can be started by alarm after it's closed. AlarmService is attached to webapprt process and this process is killed when user closes the app. Alternative solutions:
  2.1 Attach AlarmService to main browser window which will become something like "master" for all the web apps. In this case apps wont be started if user closes main browser or when they are started standalone (and they are supposed to be working this way). 
  2.2 Create a dedicated daemon process that is responsible for scheduling alarms. 
  2.3 Attach AlarmService to all firefox windows, if at least one window is alive, it should be resposible for scheduling alarms and starting apps.

All propositions are nontrivial so I need some comments on what would be the best approach for this.

Comment 10

[Marco Castelluccio [:marco]]

I think this is quite complicated and not a good first bug. I think the proper way to support this API (and other APIs, like the Push API, that need to work while apps aren't running) is to create a deamon.

Comment 11

[k]

After further investigation, it seems I was wrong about first issue. Alarms are properly stored in database and preserved between restarts. The only issue is the 2nd one.

Comment 12

[Marco Castelluccio [:marco]]

If you want we can start to support this basic case and then do a proper implementation in another bug.

Comment 13

[k]

I believe this is good idea since creating proper solution may take a lot of time - the deamon have to be started by operating system so this is quite a big change and since many users wont be running webapps anyways, not all distributions/users will be willing to enable it by default. Since it is possible to run different versions of Firefox concurently in the system, it should be possible to run multiple versions of such daemon too. And update mechanism should be able to force restarting such daemon when needed which is another concern.. And there's probably even more problems with this.

Providing partial solution means that at least some apps may work unchanged in both desktop and b2g.

Comment 14

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

I agree, the daemon thing is something that'll have to first pass muster from various module owners since that is a pretty invasive system change. Let's land this so that webapps can at least use alarms while they are running. This will also allow alarms to eventually be used by pages/services.

Comment 15

[Marco Castelluccio [:marco]]

Attachment: Patch

Comment 16

[Nikhil Marathe [:nsm] (No longer reading bugmail, please needinfo?)]

Comment on attachment 8390603 [details] [diff] [review]
Patch

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

This looks good. Please post a try run before landing though.

::: webapprt/prefs.js
@@ +52,5 @@
> +// System messages
> +pref("dom.sysmsg.enabled", true);
> +
> +// Alarm API
> +pref("dom.mozAlarms.enabled", true);

Both of these will need review by a DOM peer [1]
Our new policy is not to have prefixed APIs exposed to webpages. I'm not sure about the policy for apps. I guess it is acceptable. Please ask :sicking or someone.

[1]: https://wiki.mozilla.org/Modules/Core

::: webapprt/test/chrome/alarm.html
@@ +4,5 @@
> +    <meta charset="utf-8">
> +  </head>
> +  <body>
> +    <script>
> +    navigator.mozSetMessageHandler("alarm", function (message) { 

Nit: Trailing whitespace.

Comment 17

[Marco Castelluccio [:marco]]

(In reply to Nikhil Marathe [:nsm] (needinfo? please) from comment #16)
> This looks good. Please post a try run before landing though.

It's sadly useless at the moment, because we aren't running webapprt tests on try :(

Comment 18

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

Comment on attachment 8390603 [details] [diff] [review]
Patch

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

::: webapprt/prefs.js
@@ +52,5 @@
> +// System messages
> +pref("dom.sysmsg.enabled", true);
> +
> +// Alarm API
> +pref("dom.mozAlarms.enabled", true);

It should be acceptable for apps, to which we already expose these APIs on other runtimes.

Comment 19

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 8390603 [details] [diff] [review]
Patch

I don't think I need to look at this as long as it's properly reviewed.

Comment 20

[Marco Castelluccio [:marco]]

Attachment: Patch

Comment 21

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/160b4dbded49

Comment 22

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/160b4dbded49