Closed Bug 876980 Opened 7 years ago Closed 6 years ago

Expose mozAlarms API to installed apps on Firefox desktop

Categories

(Core :: DOM: Device Interfaces, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: nsm, Assigned: marco)

References

Details

(Keywords: dev-doc-needed)

Attachments

(1 file, 2 obsolete files)

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

Gene, would you like to take this as a mentored bug?
Component: DOM → DOM: Apps
That does not belong in DOM:Apps, probably DOM:Devices instead.
Component: DOM: Apps → DOM: Device Interfaces
Hi Nikhil,

Marking you as the mentor if you don't mind. Thank you for addressing this topic!
Whiteboard: [good first bug][mentor=nikhil]
Whiteboard: [good first bug][mentor=nikhil] → [good first bug][mentor=nsm]
Hey I think I can take this on. Where do I start with this?
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.
Assignee: nobody → almasry.mina
Mina are you still working on this?
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.
Actually, I'll just reassign it to myself when I start working on it, if it's still available.
Assignee: almasry.mina → nobody
Attached patch webapi.patch (obsolete) — Splinter Review
Extremly trivial patch (only 3 lines added) enabling Alarms API in apps started with webappsrt.
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.
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.
Whiteboard: [good first bug][mentor=nsm]
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.
If you want we can start to support this basic case and then do a proper implementation in another bug.
Flags: needinfo?(nsm.nikhil)
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.
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.
Flags: needinfo?(nsm.nikhil)
Attached patch Patch (obsolete) — Splinter Review
Attachment #803139 - Attachment is obsolete: true
Attachment #8390603 - Flags: review?(nsm.nikhil)
Attachment #8390603 - Flags: review?(myk)
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.
Attachment #8390603 - Flags: review?(nsm.nikhil) → review+
(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 :(
Attachment #8390603 - Flags: feedback?(jonas)
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.
Attachment #8390603 - Flags: review?(myk) → review+
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.
Attachment #8390603 - Flags: feedback?(jonas)
Attached patch PatchSplinter Review
Assignee: k → mar.castelluccio
Attachment #8390603 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8395039 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/160b4dbded49
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.