Closed Bug 1278716 Opened 3 years ago Closed 3 years ago

Remove wakelocks in geo

Categories

(Core :: DOM: Geolocation, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: mds, Assigned: mds)

References

(Blocks 1 open bug)

Details

(Whiteboard: btpp-active)

Attachments

(1 file)

No description provided.
Assignee: nobody → michelangelo
This is fine -- but maybe lets not remove the visibility test.  What we should do is collect telemetry when Geo is used in the background.  iirc, that was being done in another patch?
(In reply to Doug Turner (:dougt) from comment #2)

> This is fine -- but maybe lets not remove the visibility test.  What we
> should do is collect telemetry when Geo is used in the background.  iirc,
> that was being done in another patch?

Yes, we are already accumulating that: https://dxr.mozilla.org/mozilla-central/source/dom/geolocation/nsGeolocation.cpp?from=nsGeolocation.cpp%3A480#480
P
Whiteboard: btpp-active
I am asking a different question. I am suggesting that we not remove the code that tracks visibility changes and use this as an opportunity to understand how Geo is used in background tabs.
(In reply to Doug Turner (:dougt) from comment #4)

> I am asking a different question. I am suggesting that we not remove the
> code that tracks visibility changes and use this as an opportunity to
> understand how Geo is used in background tabs.

Makes sense.
If you don't mind I'm gonna go over that in a different bug.
Comment on attachment 8760990 [details]
Bug 1278716 - Remove wakelocks from geo.

clearing flag -- if the gonk stuff is removed, this will be lgtm.
Attachment #8760990 - Flags: review?(doug.turner)
Comment on attachment 8760990 [details]
Bug 1278716 - Remove wakelocks from geo.

https://reviewboard.mozilla.org/r/58368/#review82930

It took me longer than I anticipated to figure out why removing the check for ContainsRequest is correct, but I've convinced myself that now requests will only have Allow called on them once, while previously they could have been suspended by the wakelock after Allow was called.
Attachment #8760990 - Flags: review?(josh) → review+
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/503f80f3f2c2
Remove wakelocks from geo. r=jdm
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/503f80f3f2c2
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
No longer blocks: 1369194
You need to log in before you can comment on or make changes to this bug.