Closed
Bug 1166773
Opened 9 years ago
Closed 9 years ago
Disable Windows / OSX Core location provider for a three week period
Categories
(Core :: DOM: Geolocation, defect)
Core
DOM: Geolocation
Tracking
()
RESOLVED
FIXED
mozilla41
People
(Reporter: hschlichting, Assigned: garvan)
References
Details
Attachments
(1 file)
1.25 KB,
patch
|
cpeterson
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Today we have the Windows and OSX Core location providers enabled in aurora/nightly, using MLS as a fallback if one of those cannot find a result. As of last week we have also added support for querying our combain.com partner from MLS, using them if MLS cannot provide an answer based on its own dataset. In order to better judge the quality of the MLS/Combain combination, I'd like to see us disable the native Windows / OSX Core location providers for a three week period in nightly and aurora, so we'd get all the geolocation requests from these to MLS for some time. Right now we only get the "leftovers" from OS X / Windows 8+ nightly/aurora users, which results in a too small sample to be representative of the beta/release audience. Nightly also tends to be too small of a sample size to provide meaningful data for this use-case, hence the ask to do this time-limited switch on aurora as well.
Previously, the Mac and Windows native providers we're only turned on in nightly and Aurora. This patch turns off the native providers so that we can test MLS+Combain.
Attachment #8608206 -
Flags: review?(cpeterson)
Comment 2•9 years ago
|
||
Comment on attachment 8608206 [details] [diff] [review] bug1166773_combain.diff Review of attachment 8608206 [details] [diff] [review]: ----------------------------------------------------------------- LGTM. Do you plan to uplift this to Aurora 40? We should probably file a follow-up bug with tracking-firefox40=+ so we remember to back out this change before it hits the Beta channel. :)
Attachment #8608206 -
Flags: review?(cpeterson) → review+
Comment on attachment 8608206 [details] [diff] [review] bug1166773_combain.diff Approval Request Comment [Feature/regressing bug #]: relates to bug 1122393, this part of the experimentation required to certify Mozilla Location Services is working as expected. [User impact if declined]: None. [Describe test coverage new/current, TreeHerder]: None. (all testing is server side). [Risks and why]: Potentially worse geolocation results for Aurora+Nightly users, for a minority of users. Worst case result is a GeoIP-based result, which is still a usable result. A small number of users may get annoyed, however we need a user base to experiment on. Coding/stability risk is zero, this patch just toggles a pref. [String/UUID change made/needed]: None.
Attachment #8608206 -
Flags: approval-mozilla-aurora?
> LGTM. Do you plan to uplift this to Aurora 40? We should probably file a
> follow-up bug with tracking-firefox40=+ so we remember to back out this
> change before it hits the Beta channel. :)
Thanks for the r+, yes to uplifting to A40. Except we don't need to back it out for Beta, as Beta and Release don't use the native providers, they use GLS (that is a separate pref, next to this pref in firefox.js) . Aurora+Nightly were being used to test the native providers, with a fallback to MLS, this patch changes the behaviour to always use MLS on those, as we don't get enough MLS usage data with the native providers turned on.
Comment 5•9 years ago
|
||
Shouldn't we run things like that with the Telemetry Experiments infrastructure instead? It would run via a add-on-driven infrastructure instead of product patches and could also be adjusted or turned off easily at any point if there are issues.
Thanks for the suggestion Kairo, it is the first I have heard of this, I'll research it before progressing with this bug.
Just finished reading about Telemetry Experiments. Pretty cool. Looks like overkill though, in that it requires greater time commitment, involves more people in the landing process, and the volume of code and steps required mean there is a greater risk of error. It changes the scope of work for this bug significantly. The benefit that I see of the TE approach is the ability to turn things on and off easily. There is no concern that I am aware of with toggling these prefs, that would require that. Of course, bug readers please chime in if you feel I should be investigating TE further.
Comment 8•9 years ago
|
||
Before we approve this uplift, could you report a new bug to enable back Windows / OSX Core location provider with the opposite patch and request for tracking in 40? Chris, you confirm that you don't want to use Telemetry experiments for this?
Flags: needinfo?(cpeterson)
Added 1167614 to revert this experiment. To be clear, none of this affects Beta/Release in any way, so IMHO, we are experimenting in an area of code that is already experimental.
Comment 10•9 years ago
|
||
(In reply to Sylvestre Ledru [:sylvestre] from comment #8) > Chris, you confirm that you don't want to use Telemetry experiments for this? Correct. Garvan says they are overkill for this short test.
Flags: needinfo?(cpeterson)
Updated•9 years ago
|
status-firefox40:
--- → affected
Comment 11•9 years ago
|
||
Comment on attachment 8608206 [details] [diff] [review] bug1166773_combain.diff OK, let's do it then.
Attachment #8608206 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 13•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/a41e4d946c50
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in
before you can comment on or make changes to this bug.
Description
•