1053403 - Use Promises to manage wakelocks

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: FindMyDevice, defect)

Description

[Guilherme Gonçalves [:ggp]]

We could do a much better job of acquiring/releasing wakelocks if we wrapped the client's logic and commands in promises. That way we could ensure wakelocks are acquired at the beginning of any async operation (say, registration), and released at the end or if anything goes wrong.

Taking this one step further, I'd like to consider refactoring the FMD app so that it acts like a single-threaded server, servicing requests made by the System and Settings apps through a well-defined IAC API.
 
Right now we're basically using IAC to launch the FMD app and notify it of interesting events, which causes it to start doing stuff in the background (for example, re-registering when the 'geolocation disabled' event happens). We could formalize these async operations (register, update client id etc.), exposing some (all?) of them through IAC, and have each incoming IAC request be associated with a promise that is put in a request queue inside the FMD app. The FMD app would then de-queue and run only one operation at a time, holding a wakelock during the entire time, reducing complexity of the client state machine and potentially mitigating a class of bugs caused by intermingling async actions (such as trying to disable FMD while registering, see bug 1041659).

The above description contains a lot of hand-waving, but I think it's an interesting general direction moving forward.

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] guilherme-pg:fmd-promises > mozilla-b2g:master

Comment 2

[Guilherme Gonçalves [:ggp]]

I think I have something to show here. Please ignore most of comment 1 (from 'Taking this...' on), that's not the design I'm using.

This PR converts the most important methods in FindMyDevice to use Promises, which allows us to express locking a lot more concisely [1,2].

It also introduces a helper class that implements the locking semantics we need for settings [3], and as a result, we'll use a separate lock for each setting we're modifying. This improves on what we currently do, which is to try to guard all settings changes and network accesses with a single lock, making lock management very awkward and error-prone [4].

This is still a work-in-progress as I'd like to write more tests, and there are some old tests that I've broken and don't know how to fix (yet!). But I'd love to get some input as an informal review.

1- https://github.com/guilherme-pg/gaia/blob/e2c200479174441460fb938540679d7ef1455756/apps/findmydevice/js/findmydevice.js#L240,L256
2- https://github.com/guilherme-pg/gaia/blob/e2c200479174441460fb938540679d7ef1455756/apps/findmydevice/js/findmydevice.js#L510,L528
3- https://github.com/guilherme-pg/gaia/blob/e2c200479174441460fb938540679d7ef1455756/apps/findmydevice/js/findmydevice.js#L21
4- https://github.com/guilherme-pg/gaia/blob/1e54803f8a79c65a344093c5e8da462bba6d3fa0/apps/findmydevice/js/findmydevice.js#L443

Comment 3

[:gerard-majax]

Comment on attachment 8593361 [details] [review]
[gaia] guilherme-pg:fmd-promises > mozilla-b2g:master

I've left a few comments, but it looks good for now.

Comment 4

[Mark Goodwin [:mgoodwin]]

Comment on attachment 8593361 [details] [review]
[gaia] guilherme-pg:fmd-promises > mozilla-b2g:master

LGTM

Comment 5

[:gerard-majax]

Sorry for the delay, I'm not forgetting this, just being quite busy.

Comment 6

[:gerard-majax]

Comment on attachment 8593361 [details] [review]
[gaia] guilherme-pg:fmd-promises > mozilla-b2g:master

Sorry for the delay. This looks pretty good to me. I have left some questions on the pull request, and maybe we would like to rebase and have a set of retry on current codebase.

But thanks for this work, this is really valuable and the code is getting easier to understand !

Comment 7

[BMO Automation]

Firefox OS is not being worked on