Closed
Bug 1026669
Opened 10 years ago
Closed 9 years ago
Limit SSDP polling to when Firefox is in the foreground
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox43 fixed, firefox44 fixed)
RESOLVED
FIXED
Firefox 44
People
(Reporter: guigs, Assigned: mfinkle)
References
Details
(Whiteboard: [Power:P2])
Attachments
(1 file)
3.64 KB,
patch
|
Margaret
:
review+
lizzard
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
A user has performed a few experiments with Firefox for Android on a Droid X on version 29 and 30.
https://support.mozilla.org/en-US/questions/988326
from: https://support.mozilla.org/en-US/questions/988326#answer-591449
"Basically I have performed the following tests with only FF 29 opened (I have also performed some tests with FF30 with the same behavior):
- Duration 19h for each test
- Open 10 tab on the site http://www.developpez.com/ Results by opening the Battery system application: Network: 31%, Display: 16%, OS:10%, FF:8%, Battery decrease: 19%
- Open 10 tab on the site http://mobile.clubic.com/ Results by opening the Battery system application:
Network: 21%, Display: 15%, OS:13%, FF:13%, Battery
decrease: 58% :-(
- Open 10 tab on the site http://www.lesnumeriques.com/
Results by opening the Battery system application: Network: 28%, Display: 21%, OS: 9%, FF:5%,
Battery decrease: 19%
So it seems that there is an issue with the site mobile.clubic.com at least :-( "
Comment 1•10 years ago
|
||
We've done some power consumption experiments, and there are certainly improvements to be made.
Component: Core → General
Depends on: powah
OS: Mac OS X → Android
Product: Android Background Services → Firefox for Android
Hardware: x86 → ARM
Comment 2•10 years ago
|
||
I would be weary of any testing without running the test multiple times and by not using dynamic content online which is suspect to change
Comment 3•10 years ago
|
||
I ran a multi-minute profile with Clubic open.
99.98% of the app runtime was in nsAppShell::PNNE:Wait.
0.1% of the time was js::RunScript, mostly handling a number of SimpleServiceDiscovery.onPacketReceived() calls.
I also see about the same number of SSD _search() calls from a timer.
I also see regular requests -- every two minutes -- for device-desc.xml, dd.xml, and / on three devices on my home network.
SSD landed in Fx29 (Bug 938571), so it corresponds with the user's observed regression range.
I wouldn't normally imagine SSD as a problem, because the user reports that they're on 3G, not wifi. But I am gently apprehensive that we're keeping the device (and the radios) awake by constantly polling for services.
mfinkle, thoughts on that?
Flags: needinfo?(mark.finkle)
Assignee | ||
Comment 4•10 years ago
|
||
SSDP should only be enabled on Fx29 & Fx30 & Fx31 if the "browser.casting.enabled" pref is true. Otherwise, the code should never try to search for anything.
If the code is enabled, it should only search (networking) if on Wifi or ethernet:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/modules/SimpleServiceDiscovery.jsm#135
Otherwise, the timer can fire but we shouldn't trigger any networking.
For the OP case, it's unlikely that SSDP is the cause, but in Richard's case it should be expected if he is testing Nightly or Aurora (disabled on Beta and Release) and on Wifi.
I'm not opposed to looking for ways to pause the SSDP polling. Maybe we could hook CastingApps (which controls the service for now) to pause polling when we go into the background?
Flags: needinfo?(mark.finkle)
Updated•9 years ago
|
Whiteboard: [Power]
Updated•9 years ago
|
Whiteboard: [Power] → [Power:P2]
Assignee | ||
Comment 5•9 years ago
|
||
This turned out to be kinda simple. I verified via debug logging that the notifications are fired at the right times and the search is stopped and started as expected.
Assignee: nobody → mark.finkle
Attachment #8669880 -
Flags: review?(margaret.leibovic)
Assignee | ||
Comment 6•9 years ago
|
||
The patch and most of the comments are about limiting the SSDP polling. I think we should re-subject this bug to make it specific to that purpose.
Summary: Battery performance Droid X Firefox for Android or site compatibility → Limit SSDP polling to when Firefox is in the foreground
Comment 7•9 years ago
|
||
Comment on attachment 8669880 [details] [diff] [review]
casting-lowpower v0.1
Review of attachment 8669880 [details] [diff] [review]:
-----------------------------------------------------------------
This is a pattern we should make sure people know about, since this is a pattern we've used in the past (I'm thinking about the devicelight listener for reader mode).
Attachment #8669880 -
Flags: review?(margaret.leibovic) → review+
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox44:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44
Assignee | ||
Comment 11•9 years ago
|
||
Comment on attachment 8669880 [details] [diff] [review]
casting-lowpower v0.1
Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]: Using power when in background or (more importantly) when device is asleep
[Describe test coverage new/current, TreeHerder]: running on Nightly
[Risks and why]: Low risks due to the small change, but also it's a low risk feature (video casting), but the power usage is a big concern
[String/UUID change made/needed]: none
This seems to have a noticeable affect on power usage, so I want to push it forward a little.
Attachment #8669880 -
Flags: approval-mozilla-aurora?
Comment 12•9 years ago
|
||
Comment on attachment 8669880 [details] [diff] [review]
casting-lowpower v0.1
On m-c for a while, risk probably limited to single feature.
Let's improve battery life, ok to uplift to aurora.
Attachment #8669880 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 13•9 years ago
|
||
status-firefox43:
--- → fixed
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•