Pending Privacy Policy update signoff.

Depends on D50209

Depends on D50210

Depends on D50211

Depends on D50212

Depends on D50210

So this now has all the required sign-offs and is pending reviews.

Dale, I just submitted the latest of my patches that implement the changes Mark requested and it's blowing test suite(s) up like it's nothing. Well, it's not as bad as my earlier try pushes, but still... It'd be great if you could take a look at the code and perhaps point out something I missed? Thanks!!

So as mentioned in search, these tests are failing due to the change in behaviour of maybeReloadEngines

https://searchfox.org/mozilla-central/source/toolkit/components/search/tests/xpcshell/test_geodefaults.js#18 should be |Services.search.init(true)|

However the main issue is that in the calls to |asyncReInit({ waitForRegionFetch: true })| the search service will explicity wait for the region fetch to complete, but then also schedule maybeReloadEngines to happen at some point later, at this point later the tests arent expecting us to run and we may be in a funny state.

We could solve this by passing through |awaitRegionFetch| to |_maybeReloadEngines|, this should fix most of the test usage, if you can avoid changing this behaviour we "may" want to look fixing it as part of a larger refactor

Comment on attachment 9103534 [details]
Bug 1613627 - Rename 'WifiGeo' to the more generic and appropriate 'NetworkGeo' inside the module. r?garvan!

Revision D50210 was moved to bug 1613627. Setting attachment 9103534 [details] to obsolete.

Comment on attachment 9103533 [details]
Bug 1613627 - Rename all 'geo.wifi' related preferences to match the 'geo.provider.' convention and move default values to all.js. r?garvan!

Revision D50209 was moved to bug 1613627. Setting attachment 9103533 [details] to obsolete.

Ok so I have all the tests passing except one memory leak that is introduced by this patch, (try run @ https://treeherder.mozilla.org/#/jobs?repo=try&revision=31093c62cd87b25e358b3799ae7d2240d5b84c20)

Mike, the memory leak is caused by the |Services.tm.dispatchToMainThread(() => this.sendLocationRequest(null));| in the startup() @ https://hg.mozilla.org/try/file/4a71e1592a5594f713131ff6678497113ad250f7/dom/system/NetworkGeolocationProvider.jsm#l310

Investigating why this codepath is being run since I wouldnt expect it to via dom/tests/mochitest/geolocation/test_manyCurrent which is where I see the memory leak, however either way its probably indicating something is wrong, one of the change I made from your version to mine was moving the dispatch to before |if (this.started)| because otherwise there was a bunch of tests waiting for geolocation requests that never got a response.

However given that change and this memory leak, any change you could give me a quick rundown about what the plan and implications of the changes were here so I have a bit more context for the fixes. I can probably make changes that gets the tests passing but want to make sure they are the right ones. Cheers

Mike answered on Matrix (cheerS)

We should probably re-title this bug for what we're actually landing as well.

Backed out changeset 5b5feaf8e1a0 (bug 1589618) for causing xpcshell failures on test_location_timeout.js

Backout revision https://hg.mozilla.org/integration/autoland/rev/86aa8af6ff8b77706d32e877f16d0d3b58091d87

Failed push https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=5b5feaf8e1a0986e4f3dbe0411ed794c8b49791f

Failures https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=linux64-tsan%2Copt%2Cxpcshell%2Ctests%2Ctest-linux64-tsan%2Fopt-xpcshell-e10s-1%2Cx%28x1%29&fromchange=71e376fd296d9b2ad1e3bca1ceab0df2a3eec093

Failure log https://treeherder.mozilla.org/logviewer.html#?job_id=290460025&repo=autoland

Dale can you please take a look?

ok found this, a timeout pref got rewritten with the wrong value, it was only failing on asan tests on taskcluster as it was a timeout that would only fail when running particularly slowly

https://hg.mozilla.org/mozilla-central/rev/eeda4bdcd130