Closed Bug 907028 Opened 11 years ago Closed 10 years ago

[email] Best approach for wifi wake lock

Categories

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

Product:
Firefox OS Graveyard
Component:
Gaia::E-Mail
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(b2g1828+)

Status:
RESOLVED DUPLICATE of bug 1098289
Tracking Flags:
Tracking Status
b2g18 28+ ---

People

(Reporter: jrburke, Unassigned)

References

Details

As part of bug 892519, there was a discussion about if and when to acquire a wifi wake lock for a background email sync. It was decided to break that off into a new bug (this one). Previous discussion about the issues involved, from :asuth in the pull request discussion for 892519:

Right now, if we acquire the 'wifi' wake-lock from our background sync and wi-fi was off (because we were on battery), this will cause the  wi-fi to turn back on.  But our logic isn't waiting for the wi-fi  connection to be established.  It's possible that by the time wi-fi  comes up we will have already completed our sync.  If we wait for wi-fi to come up, it's possible that we aren't in range of a good wi-fi  connection and that's wasteful. Or maybe the resumed wi-fi connection  will find us unable to actually reach the internet because we're on a wi-fi connection that requires user action to connect (ex: enter your hotel room number and name, code from the receipt, etc.)

The ideal goal would be to know if wi-fi was working recently and only  turn it on in that case.  And/or be informed by some limited geofencing  to know if there should now be different wi-fi networks in range that we should try and use.  The system wifi.js seems to set a settings flag for its own benefit that would tell us if wi-fi got powered down for wake-lock reasons so we could know whether we even bothered wanting to turn on the wi-fi.  A super built-out implementation would then know whether it wants to acquire the wake-lock, wait for something to happen, then store the results of that with the current cell tower id in use for geo-fencing purposes so next time around we know whether to try acquiring the wi-fi wake-lock again.

So the current cop-out (no wifi wake lock) lets us avoid the issue.  If the device is plugged in, wifi.js won't turn off the wi-fi, so we'll just be using wi-fi.  If the device is not plugged in, we'll just use the data connection.  This is acceptable for dogfooding by people with unlimited data plans, but is not a sufficient long term solution without more discussion.

We probably also need to think if we need to talk with UX or the settings app about stuff like this.  Such as adding a setting to ourselves like 'only sync over wi-fi'.
triage: need some solution here for effective notifications
blocking-b2g: --- → koi+
Started a discussion on dev-gaia here:
https://groups.google.com/forum/#!searchin/mozilla.dev.gaia/wifi$20wake$20lock/mozilla.dev.gaia/sMiGF-uP3mQ/gKJYeGlgMZAJ
1. We should always target "least cost routing" when making any connection.  
2. Conserving data (hard cost) is paramount to conserving battery (soft cost). 
3. The user can override the previous two rules.

If we're connected to WiFi, we should be trying our best to use that connection. I believe an async WiFi wait lock that would block until the WiFi turned on is the best approach.

I like Guillermo López's [willyaranda] idea of a system message that indicates 'working' data connection

"One of the ideas we had with SimplePush (and I think we never ended up filling a bug) was to had a System Message that says that there is a active data connection being used (wifi or 3g… but there has been transmited some data in the last 2-3 seconds)."

but I expect this may require more significant effort.

In the near term, can we:

1) Create an async WiFi wake-lock
2) When syncing messages, assume if WiFi is connected it's working and default to WiFi anytime it's connected
3) If the sync over WiFi fails (bad network, not logged into a hotspot, etc.) this can be taken to indicate that we're not actually connected to Wifi so we should retry over wireless.

thoughts? comments?
:arog: I am hoping to get to "1) Create an async WiFi wake-lock", but trying to work that out in that dev-gaia thread, and hopefully that mechanism will try for a wifi connection but if not just proceed with whatever network connection we have.

I am not sure if the "3) detect wifi failure, try cell" is possible right now. IIRC, we can know if we are online or offline, but not force a type of connection.
The updater uses the registers an observer on the network:offline-status-changed notification.

If the updater "time-to-check" time comes up and the network is unavailable, then it registers for the above event and tries again when the network becomes available.

https://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/update/nsUpdateService.js#2415
Dylan,

Please help assign an owner for this bug as it is a koi+
Flags: needinfo?(doliver)
Assignee: nobody → jrburke
Flags: needinfo?(doliver)
Too late for 1.2. Discussions are underway for the RIL work required to support this -- bumping this forward for consideration in 1.3.
blocking-b2g: koi+ → 1.3?
Depends on: 930969
Waiting on dependent work in bug 930969 -- moving this to 1.4.
blocking-b2g: 1.3? → 1.4?
tracking-b2g18: --- → 28+
Still on hold. Backlogging until we get unblocked.
blocking-b2g: 1.4? → backlog
Talking with Ken Chang, this is how we will address the issues around network connection and when to ask for a wakelock:

1) First, bug 930972 will allow the user to use wifi when the phone is in sleep mode. This will allow more of our background syncs to use the less user-expensive connection, if they so choose.

2) Bug 960426 will provide a better network connection API, so we can save some battery if we know there is no connection. I filed bug 971166 to track email changes to use that API when it becomes available.

3) Ken said we should always just ask for a wifi wake lock when we will be doing network, and it is fine to do even if wifi is not the actual connection type that will be used. He was thinking of providing a 'connection' wake lock type for people that just want to indicate they just want whatever connection still workable. However, no concrete plans for that have been set yet.

Items 1 and 2 have bugs to track those changes, and I just filed bug 971183 to track adding a wake lock request as part of sync. So if :arog is satisfied, I think we could close out this bug with the approach having been identified, with bugs for the individual pieces.
:jrburke, I'm going to restate for my understanding.  Please sanity check me.

The bad situations are:
* Screen is off, user has data-connection.
* We wake up, we take wi-fi wakelock, this causes wi-fi to spin up, BUT
* One Of:
 * We are so fast and nimble we establish our synchronization connections using data, so we just wasted the wi-fi spin-up and burnt power for no reason.
 * We are so fast and nimble we manage to fail to connect and give up before the wi-fi connection spins up.

The good situation is that:
* We take wi-fi wakelock.
* Wi-fi spins up before we connect (or wi-fi was already connected)
* We use wi-fi to connect
* Hooray


I'll break out the cases here for when sync triggers using +/- to indicate the state when we start:
* +wifi, +data: works, we use wi-fi.
* +wifi, -data: works, we use wi-fi
* -wifi, +data: works, we may or may not use wi-fi, we may waste power
* -wifi, -data: may or may not work, we may or may not use wi-fi, we may waste power

Here is how the bugs you list impact us:
* Bug 930972.  This bug currently is very unclear about what is planned for v1.4, and it looks slip-prone.  There are a few interpretations of what could happen:
 * +wifi always.  User chooses to just burn power all the time.  We always win!
 * +wifi always by way of magic low-power wifi.  We always win!
 * +wifi when charging.  We always win when charging, same situation when not charging
 * +wifi on some interval.  Unless we get a notification, same situation as now but sometimes we'll randomly win and the device periodically burns power for the hell of it.

* Bug 960426.  This seems like we just never turn on wi-fi so we never race wi-fi, and we can determine whether to take a wifi wake-lock if we're on wi-fi or a cpu wake-lock if we're on CPU.  We can also already do this by just using the certified wi-fi API to see if there's a live wi-fi connection.  Works for us, the security team may hate us for increasing our surface area, and other e-mail clients that can only be privileged may jealous of us.


Based on these bugs, it sounds like the right course of action for us would be to:

* Use the wi-fi API.  Check navigator.onLine when we start and check the wi-fi status.  If wi-fi says we are connected, assume +wifi and take the wake-lock.  If wi-fi is no but navigator.onLine is true, assume data and just take a CPU wake-lock.  Otherwise, do the bug 971166 fast-path abort.

* If/when bug 960426 lands and is able to tell us we are on wi-fi, stop using the certified-only wi-fi API and use that instead.

* If/when we get a 'connection' wake-lock, just use that and navigator.onLine.


From a user perspective what this means is that:

* Periodic sync requires a data connection and will use your data-connection unless bug 930972 / friends allow wi-fi while we are sleeping.

This sorta suggests UI enhancements to add warnings to our settings UI to explain that if we don't have a data connection that wi-fi is not going to work.
Flags: needinfo?(jrburke)
navigator.onLine has had a history of not working well -- just tells if if user explicitly turned off networking, similar to how desktop only returns onLine === false if the "work offline" menu was used. Likely just helps if the user turns on airplane mode, that's it. Not if network connection was not available.

So we could use that now to help save battery in the airplane mode case.

I would prefer to not use the certified wifi API though, as I do not think it buys much given what I understood happened when a wifi wakeLock was requested, (question for Ken below), and that it does not help us know if there is a cell connection. We would still want to try making a connection. 

My understanding from Ken was that doing `navigator.requestWakeLock('wifi')` did not trigger wifi to turn on, but just keeps it on if it was already on. So it was fine to always just ask for the wakeLock. Ken, is this true?
Flags: needinfo?(jrburke) → needinfo?(kchang)
Yes, it's my through. But chuck is designing this now. Maybe, he can provide more information for this after we discuss the design today.
Flags: needinfo?(kchang) → needinfo?(chulee)
Current API draft is:

enum WifiLockType {
  "full",
  "powersave"
};

interface nsIDOMWifiManager
{
  ...
  nsIDOMDOMRequest setWifiLocked(in WifiLockType lockType, in boolean locked);
  nsIDOMDOMRequest getCurrentWifiLock();
  attribute nsIDOMEventListener onWifiLockChange;
}

app can use MozWifiManager.setWifiLocked() to acquire/release wifi lock. System app use callback MozWifiManager.onWifiLockChange to handle wifi status change.

Current design is due to the fact(limitation) that gecko don't get any event about screen status change, so turning off wifi after screen off is completely handled by system app now.
So WifiManager is playing the role of managing lock state, and provide lock state to system app for controlling wifi.

Holding wifi lock won't hold cpu lock automatically in current design, I am not sure we should or shouldn't.

Opinions are welcome!
Flags: needinfo?(chulee)
After discussion with Alive and more study, it seems we can accomplish this by |navigator.requestWakeLock('wifi')|.
Changing wakelock state will be received by system app and change the wifi state. But it is impossible to get wifi up and connect right away after it is disabled. To prevent using data connection while wifi getting up, the best way should be always hold the wifi lock.

Also keep wifi in full power mode takes much power, we can make an adjustment to the wifi control logic in system app[1], make wifi enter power save mode while screen is off and wifi lock is held.

[1] http://git.mozilla.org/?p=releases/gaia.git;a=blob;f=apps/system/js/wifi.js;h=59f0519a2a949b90d3a29631a2d1ba4f88ec18c0;hb=refs/heads/master#l134
:chucklee: for the email synchronizing email case, the email app is very likely not running in the background, but instead, started up as part of a mozAlarm wake up, and the sync happens right after that, and once the sync completes, email closes itself down, kills itself. The alarm could fire when the screen is off.

So I do not think we could hold on to the wifi lock for those cases.

I think what is needed longer term is a different kind of API, where the app could ask the system, "call me back or start me up if network becomes available, I have work to do". And then if the phone, while doing any periodic network wakeup to do tasks like simple push, could then also give apps a chance to run. Although, I expect that API to be hard to work out correctly.

In the meantime, it still seems like the things we can do today for this is:

* use navigator.onLine to short circuit startup and shut down in a sync if it reports false.

* Always ask for a wifi wake lock when we will be doing network activity. While there are edge cases, like sync, where it may not be helpful if the wifi was in an off state, and may not start up fast enough, it could help cases where the sync starts, before wifi is turned off.

This all assumes though that a network connection established via the cell network connection stays active and alive when wifi does come back on. If it does not, and the connection through the cell network is killed when wifi comes on, that would cause a problem for email syncing, since email syncing uses longer lived TCP connections for the email protocols.
(In reply to James Burke [:jrburke] from comment #18)
> :chucklee: for the email synchronizing email case, the email app is very
> likely not running in the background, but instead, started up as part of a
> mozAlarm wake up, and the sync happens right after that, and once the sync
> completes, email closes itself down, kills itself. The alarm could fire when
> the screen is off.
> 
> So I do not think we could hold on to the wifi lock for those cases.

If the app try to use wifi, following flow could work:
Alarm wake -> hold CPU lock -> Hold wifi lock -> listen to event "wifi-enabled" and "wifi-statechange" from system app, with timeout -> sync if connected -> release both locks.

But I don't see much need to do so, instead of just wake up and sync only if network is connected:
Alarm wake -> hold CPU lock -> sync if connected -> release CPU lock.

> 
> I think what is needed longer term is a different kind of API, where the app
> could ask the system, "call me back or start me up if network becomes
> available, I have work to do". And then if the phone, while doing any
> periodic network wakeup to do tasks like simple push, could then also give
> apps a chance to run. Although, I expect that API to be hard to work out
> correctly.
> 

What this API does might like set a periodic alarm doing:
Wake -> hold CPU lock -> check connection status -> notify -> release CPU lock.
I think this can be done in app with current API, and doing work directly instead of go through the notify path.

> 
> This all assumes though that a network connection established via the cell
> network connection stays active and alive when wifi does come back on. If it
> does not, and the connection through the cell network is killed when wifi
> comes on, that would cause a problem for email syncing, since email syncing
> uses longer lived TCP connections for the email protocols.

Data connection will be killed while Wifi is connected. Also, connecting to different AP will break TCP connection even wifi lock is held.
These locks only keeps the targets up, but can not keep them stay in the same state.
So app should not assume the it will have same TCP connection even it's connected to a network while waken up every time.
For checking connection status, you can check this bug, bug 960426.
This should be solved by using the new RequestSync API, which will be tracked in bug 1098289, so will dupe this bug to that bug, even though this one came first. The newer bug has a specific implementation pathway. This bug was more about trying to figure out what API should be tracked, and now there is one.
Assignee: jrburke → nobody
Status: NEW → RESOLVED
blocking-b2g: backlog → ---
Closed: 10 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.