Mike Kaply [:mkaply] Assignee
	Description • 7 years ago

Because of bug 1413652, we know people have incorrect geo values.

We're going to do a reset of these prefs with Firefox 57.

	Florian Quèze [:florian]
	Comment 4 • 7 years ago
mozreview-review

Comment on attachment 8925678 [details]
Bug 1414965 - Reset geo preferences for Firefox 57.

https://reviewboard.mozilla.org/r/196808/#review202036

r=me, let's get this into nightlies soon.

::: browser/components/nsBrowserGlue.js:2168
(Diff revision 3)
>              lwthemePrefs.setStringPref("usedThemes", JSON.stringify(usedThemes));
>            }
>          } catch (e) { /* Don't panic if this pref isn't what we expect it to be. */ }
>        }
>      }
> +    if (currentUIVersion < 58) {

Please add an empty line before this block to match the style of the rest of the method.

	Mike Kaply [:mkaply] Assignee
	Comment 6 • 7 years ago

[Tracking Requested - why for this release]: Needed for updating geos.

	Pulsebot
	Comment 7 • 7 years ago

Pushed by mozilla@kaply.com:
https://hg.mozilla.org/integration/autoland/rev/d44fe4ae02fe
Reset geo preferences for Firefox 57. r=florian

	Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)
	Comment 8 • 7 years ago

Backed out for unexpected connection in talos test ts_paint_heavy:

https://hg.mozilla.org/integration/autoland/rev/c6325a375c4c708e8d01c6c53ce974ace77aa06a

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=d44fe4ae02fe3dd94080347722cdde0c5d44be02&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=142576862&repo=autoland

	Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)
	Comment 9 • 7 years ago

I am told this is a change that *must* happen in 57, tracked as blocking.

	Mike Kaply [:mkaply] Assignee
	Comment 10 • 7 years ago

> Backed out for unexpected connection in talos test ts_paint_heavy.

The log you posted seems to be about a crash which is this:

https://bugzilla.mozilla.org/show_bug.cgi?id=1409685

or did I miss something? Where in the log does it talk about a connection?

ccing florian to see if he has any ideas when he gets in tomorrow.

	Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)
	Comment 11 • 7 years ago

Ok, this is a crash in [@ mozilla::net::nsSocketTransport::InitiateSocket] which is something we see as signature if there is an unexpected connection in test automation and when the process gets terminated. Here the text about the unexpected connection is missing.

	Florian Quèze [:florian]
	Comment 12 • 7 years ago

The crash indicates that we are attempting to do an external network request, which isn't fine for tests. This request is most likely the geoip lookup.

When the geoip code was introduced (bug 1109120), these 2 prefs were introduced in various pieces of test code to avoid the network request:
user_pref("browser.search.isUS", true);
user_pref("browser.search.countryCode", "US");

In the current talos code, this is done at http://searchfox.org/mozilla-central/rev/5a60492a53667fc61a62af1847d005a210b7a4f6/testing/talos/talos/config.py#136

The code we are landing in this bug clears these values in the migration code. For most tests this is fine because the migration code doesn't run for new profiles (profiles that don't have a user value for the browser.migration.version pref). ts_paint_heavy specifically uses an existing profile so it goes through the migration code; we'll need to find another way to disable the request for this test.

	Florian Quèze [:florian]
	Comment 13 • 7 years ago

Seems green on try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=06058ac6a07119c7d8201b4d6bd85e1bcf638e0e

	Joel Maher ( :jmaher ) (UTC -8)
	Comment 14 • 7 years ago

Comment on attachment 8925903 [details] [diff] [review]
Disable geoip lookups on Talos

Review of attachment 8925903 [details] [diff] [review]:
-----------------------------------------------------------------

I verified locally this fixes the crash, also I have seen it on try

	Pulsebot
	Comment 15 • 7 years ago

Pushed by florian@queze.net:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b7f758a76a87
Reset geo preferences for Firefox 57. r=florian
https://hg.mozilla.org/integration/mozilla-inbound/rev/bbfe61dcf41a
disable geoip lookups for talos tests, r=jmaher.

	Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)
	Comment 16 • 7 years ago

Hi Mike, please nominate for uplift to beta57.

	Mike Kaply [:mkaply] Assignee
	Comment 17 • 7 years ago

Comment on attachment 8925678 [details]
Bug 1414965 - Reset geo preferences for Firefox 57.

Approval Request Comment
[Feature/Bug causing the regression]: Reset geo due to bug 141495
[User impact if declined]: Bad geolocation
[Is this code covered by automated tests?]: No.
[Has the fix been verified in Nightly?]: yes.
[Needs manual test from QE? If yes, steps to reproduce]: Doc has been provided to QE.
[List of other uplifts needed for the feature/fix]: Talos fix in subsequent patch
[Is the change risky?]: Low
[Why is the change risky/not risky?]: Resets prefs to what they would have been in a new profile.
[String changes made/needed]:

	Florian Quèze [:florian]
	Comment 18 • 7 years ago

(In reply to Mike Kaply [:mkaply] from comment #17)

> Approval Request Comment

> [List of other uplifts needed for the feature/fix]: Talos fix in subsequent
> patch

We actually don't need the talos patch on 57, it's fixing a talos test that has been introduced in 58 (in bug 1407398).

	Eliza Balazs [:ebalazs_]
	Comment 19 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/b7f758a76a87
https://hg.mozilla.org/mozilla-central/rev/bbfe61dcf41a

	Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)
	Comment 20 • 7 years ago

Comment on attachment 8925678 [details]
Bug 1414965 - Reset geo preferences for Firefox 57.

Must fix, Beta57+

	Ryan VanderMeulen [:RyanVM]
	Comment 21 • 7 years ago
bugherder uplift

https://hg.mozilla.org/releases/mozilla-beta/rev/632401fa1b20 (FIREFOX_57b_RELBRANCH)
https://hg.mozilla.org/releases/mozilla-release/rev/e03bd534039e

	Andrei Vaida [:avaida]
	Comment 22 • 7 years ago

We verified the fix pushed for this issue on 58.0a1 (2017-11-08), using Windows 10 (64-bit), macOS 10.13 and Ubuntu 16.04 (64-bit) -- everything works as expected. 

Detailed test results are available here: https://goo.gl/kX1cAi.

Note: we plan to verify this on 57.0 as well, as soon as we have builds available.

	Andrei Vaida [:avaida]
	Comment 23 • 7 years ago

This fix has been verified on 57.0 [1] as well, using Windows 10 (64-bit), macOS 10.13 and Ubuntu 16.04 (64-bit) -- everything works as expected. 

Detailed test results are available here: https://goo.gl/kX1cAi.


[1] https://treeherder.mozilla.org/#/jobs?repo=mozilla-release&revision=ea14b85abeac942084437602be8486b410e4bfbd&filter-searchStr=tc(n)