1532730 - New profiles on nightly aren't ever getting the proper US search code

Mike Kaply [:mkaply]

Reporter

Description

•

5 years ago

If you create a new profile on nightly and do a search, you won't see the -1 on the search.

If you restart, you still don't get it.

If you delete search.json.mozlz4, it is there.

I've tried to use mozregression to determine when this started, but it's given me two completely different results that were wrong (Jan 17 and Jan 29).

So it appears we've done something to mess up our timing. I definitely can't recreate on beta.

I put in some logging and it seems to be related to jarNames not having the google-b-1-d engines for some reason...

Mike Kaply [:mkaply]

Reporter

Comment 1

•

5 years ago

I put in some logging and it seems to be related to jarNames not having the google-b-1-d engines for some reason...

red herring, separate issue.

I'm not sure what's going on. I expect it to have no -1 on first start because we don't have a region, but after restart we have a region, and the list of engineNames contains the new engine (google-b-1-d,amazondotcom,bing,ddg,ebay,twitter,wikipedia) but it's not actually being used.

Mike Kaply [:mkaply]

Reporter

Comment 2

•

5 years ago

So it looks like the right things are happening to do the switch, but replacing the engines is failing. Here's the log:

google-b-1-d,amazondotcom,bing,ddg,ebay,twitter,wikipedia SearchService.jsm:3354
google-b-d,amazondotcom,bing,ddg,ebay,twitter,wikipedia SearchService.jsm:2813
google-b-1-d,amazondotcom,bing,ddg,ebay,twitter,wikipedia SearchService.jsm:2814
rebuildingcachh=true SearchService.jsm:2823
_loadEngines: Absent or outdated cache. Loading engines from disk.
_loadFromChromeURLs: loading engine from chrome url: resource://search-plugins/google-b-1-d.xml
_initFromURI: Loading engine from: "resource://search-plugins/google-b-1-d.xml".
_init: Initing search plugin from [app]/chrome/browser/searchplugins/google-b-1-d.xml
_parseImage: Image textContent: "resource://search-plugins/images/google.ico"
_setIcon: Setting icon url "resource://search-plugins/images/google.ico" for engine "Google".
NOTIFY: Engine: "Google"; Verb: "engine-changed"
SearchService.init
_loadFromChromeURLs: loading engine from chrome url: resource://search-plugins/amazondotcom.xml
_initFromURI: Loading engine from: "resource://search-plugins/amazondotcom.xml".
getVisibleEngines: getting all visible engines
SearchService.init
getVisibleEngines: getting all visible engines
_init: Initing search plugin from [app]/chrome/browser/searchplugins/amazondotcom.xml
_parseImage: Image textContent: "resource://search-plugins/images/amazon.ico"
_setIcon: Setting icon url "resource://search-plugins/images/amazon.ico" for engine "Amazon.com".
NOTIFY: Engine: "Amazon.com"; Verb: "engine-changed"
SearchService.init
_loadFromChromeURLs: loading engine from chrome url: resource://search-plugins/bing.xml
_initFromURI: Loading engine from: "resource://search-plugins/bing.xml".
getVisibleEngines: getting all visible engines
_init: Initing search plugin from [app]/chrome/browser/searchplugins/bing.xml
_parseImage: Image textContent: "data:image/x-icon;base64,AAABAAIAICAAAAEACACoCAAAJgAAABAQAAABAAgAaAUAAM4IAAAoAAAAIAAAAEAAAAABAAgAAAAAAIAEAAAAAAAAAAAAAAABAAAAAAAAhIQMAI+PIwC..."
_setIcon: Setting icon url "data:image/x-icon;base64,AAABAAIAICAAAAEACACoCAAAJgAAABAQAAABAAgAaAUAAM4IAAAoAAAAIAAAAEAAAAABAAgAAAAAAIAEAAAAAAAAAAAAAAABAAAAAAAAhIQMAI+PIwC..." for engine "Bing".
NOTIFY: Engine: "Bing"; Verb: "engine-changed"
SearchService.init
_loadFromChromeURLs: loading engine from chrome url: resource://search-plugins/ddg.xml
_initFromURI: Loading engine from: "resource://search-plugins/ddg.xml".
getVisibleEngines: getting all visible engines
_init: Initing search plugin from [app]/chrome/browser/searchplugins/ddg.xml
_parseImage: Image textContent: "data:image/icon;base64,AAABAAIAEBAAAAEAIABoBAAAJgAAACAgAAABACAAqBAAAI4EAAAoAAAAEAAAACAAAAABACAAAAAAAAAEAAATCwAAEwsAAAAAAAAAAAAAAAAAAAAAAAAAA..."
_setIcon: Setting icon url "data:image/icon;base64,AAABAAIAEBAAAAEAIABoBAAAJgAAACAgAAABACAAqBAAAI4EAAAoAAAAEAAAACAAAAABACAAAAAAAAAEAAATCwAAEwsAAAAAAAAAAAAAAAAAAAAAAAAAA..." for engine "DuckDuckGo".
NOTIFY: Engine: "DuckDuckGo"; Verb: "engine-changed"
SearchService.init
_loadFromChromeURLs: loading engine from chrome url: resource://search-plugins/ebay.xml
_initFromURI: Loading engine from: "resource://search-plugins/ebay.xml".
getVisibleEngines: getting all visible engines
SearchService.init
getVisibleEngines: getting all visible engines
_init: Initing search plugin from [app]/chrome/browser/searchplugins/ebay.xml
_parseImage: Image textContent: "resource://search-plugins/images/ebay.ico"
_setIcon: Setting icon url "resource://search-plugins/images/ebay.ico" for engine "eBay".
NOTIFY: Engine: "eBay"; Verb: "engine-changed"
SearchService.init
_loadFromChromeURLs: loading engine from chrome url: resource://search-plugins/twitter.xml
_initFromURI: Loading engine from: "resource://search-plugins/twitter.xml".
getVisibleEngines: getting all visible engines
_init: Initing search plugin from [app]/chrome/browser/searchplugins/twitter.xml
_parseImage: Image textContent: "data:image/x-icon;base64,AAABAAIAEBAAAAAAAAALAgAAJgAAACAgAAAAAAAAQQQAADECAACJUE5HDQoaCgAAAA1JSERSAAAAEAAAABAIBgAAAB/z/2EAAAHSSURBVDjLfVO/axR..."
_setIcon: Setting icon url "data:image/x-icon;base64,AAABAAIAEBAAAAAAAAALAgAAJgAAACAgAAAAAAAAQQQAADECAACJUE5HDQoaCgAAAA1JSERSAAAAEAAAABAIBgAAAB/z/2EAAAHSSURBVDjLfVO/axR..." for engine "Twitter".
NOTIFY: Engine: "Twitter"; Verb: "engine-changed"
SearchService.init
_loadFromChromeURLs: loading engine from chrome url: resource://search-plugins/wikipedia.xml
_initFromURI: Loading engine from: "resource://search-plugins/wikipedia.xml".
getVisibleEngines: getting all visible engines
_init: Initing search plugin from [app]/chrome/browser/searchplugins/wikipedia.xml
_parseImage: Image textContent: "resource://search-plugins/images/wikipedia.ico"
_setIcon: Setting icon url "resource://search-plugins/images/wikipedia.ico" for engine "Wikipedia (en)".
NOTIFY: Engine: "Wikipedia (en)"; Verb: "engine-changed"
SearchService.init
_addEngineToStore: Adding engine: "Google"
_addEngineToStore: Duplicate engine found, aborting!
_addEngineToStore: Adding engine: "Amazon.com"
_addEngineToStore: Duplicate engine found, aborting!
_addEngineToStore: Adding engine: "Bing"
_addEngineToStore: Duplicate engine found, aborting!
_addEngineToStore: Adding engine: "DuckDuckGo"
_addEngineToStore: Duplicate engine found, aborting!
_addEngineToStore: Adding engine: "eBay"
_addEngineToStore: Duplicate engine found, aborting!
_addEngineToStore: Adding engine: "Twitter"
_addEngineToStore: Duplicate engine found, aborting!
_addEngineToStore: Adding engine: "Wikipedia (en)"
_addEngineToStore: Duplicate engine found, aborting!
_loadEngines: loading user-installed engines from the obsolete cache
_loadEnginesFromCache: Loading 7 engines from cache
_loadEnginesFromCache: skipped 7 read-only engines.
_loadEnginesMetadataFromCache, transfering metadata for Google
_loadEnginesMetadataFromCache, transfering metadata for Amazon.com
_loadEnginesMetadataFromCache, transfering metadata for Bing
_loadEnginesMetadataFromCache, transfering metadata for DuckDuckGo
_loadEnginesMetadataFromCache, transfering metadata for eBay
_loadEnginesMetadataFromCache, transfering metadata for Twitter
_loadEnginesMetadataFromCache, transfering metadata for Wikipedia (en)
_loadEngines: done using rebuilt cache
getVisibleEngines: getting all visible engines
_buildCache: Writing to cache file.
SearchService.init
getVisibleEngines: getting all visible engines
_buildCache: cache file written to disk.
SearchService.init
getVisibleEngines: getting all visible engines
SearchService.init
getVisibleEngines: getting all visible engines
SearchService.init
getVisibleEngines: getting all visible engines
SearchService.init
getVisibleEngines: getting all visible engines
SearchService.init
getVisibleEngines: getting all visible engines
SearchService.init

getEngines: getting all engines

Dale Harvey (:daleharvey)

Assignee

Comment 3

•

5 years ago

Will try to help debug this, but in https://bugzilla.mozilla.org/show_bug.cgi?id=1496075 I had an issue with a new test not seeing the reloaded engines on a 'restart', I solved it in the test only by adding a flag to force the |await fetchRegion|, but without that flag init would run, fetchRegion would run in parallel, the fetchRegion would complete before init was finished and so |maybeReloadEngines| would early return due to https://searchfox.org/mozilla-central/source/toolkit/components/search/SearchService.jsm#2856 and we wouldnt see the update engine list until the cache got invalidated

Mike de Boer [:mikedeboer]

Comment 4

•

5 years ago

Hi Dale, I'd like to fix this early during the Firefox 67 Beta cycle. Can you take a look, please?

Assignee: nobody → dharvey

Flags: needinfo?(dharvey)

Dale Harvey (:daleharvey)

Assignee

Comment 6

•

5 years ago

ok will look into this today

Flags: needinfo?(dharvey)

Dale Harvey (:daleharvey)

Assignee

Comment 7

•

5 years ago

So opposite of what I thought previously maybeReloadEngines is getting hit however it doesnt do anything as far as I can see to clear the currently installed engines, just attempts to install over them and they fail with

_addEngineToStore: Adding engine: "Google"
_addEngineToStore: Duplicate engine found, aborting!
_addEngineToStore: Adding engine: "Amazon.com"
_addEngineToStore: Duplicate engine found, aborting!
_addEngineToStore: Adding engine: "Bing"
_addEngineToStore: Duplicate engine found, aborting!
_addEngineToStore: Adding engine: "DuckDuckGo"
_addEngineToStore: Duplicate engine found, aborting!
_addEngineToStore: Adding engine: "eBay"
_addEngineToStore: Duplicate engine found, aborting!
_addEngineToStore: Adding engine: "Twitter"
_addEngineToStore: Duplicate engine found, aborting!
_addEngineToStore: Adding engine: "Wikipedia (en)"
_addEngineToStore: Duplicate engine found, aborting!

This issue will be fixed by https://bugzilla.mozilla.org/show_bug.cgi?id=1496075 in 68, but not sure what to do for 67, maybe tweak the duplicate engine detection, have maybeReloadEngines set an update flag so addEngineToStore updates duplicates instead of rejecting them?

Mike de Boer [:mikedeboer]

Comment 8

•

5 years ago

(In reply to Dale Harvey (:daleharvey) from comment #7)

This issue will be fixed by https://bugzilla.mozilla.org/show_bug.cgi?id=1496075 in 68, but not sure what to do for 67, maybe tweak the duplicate engine detection, have maybeReloadEngines set an update flag so addEngineToStore updates duplicates instead of rejecting them?

I think a flag would be prudent here. I'm happy to hear that your patch(es) in bug 1496075 fixes this problem as well!

Mike de Boer [:mikedeboer]

Comment 9

•

5 years ago

However, I think it'd be worth checking out how the _engineToUpdate flag is used throughout SearchService.jsm.
Perhaps even go as far as using engineUpdateService.update(engine), instead of addEngineToStore. But maybe not, because that updater is part of our OpenSearch legacy.

Dale Harvey (:daleharvey)

Assignee

Comment 10

•

5 years ago

Yeh thats what I took a look at

https://hg.mozilla.org/try/rev/a6fdd670fed2dc521eebd7cd721b91d7666b52a2

Fixed this issue for me, it seems unintrusive and like it shouldnt break anything but of course turns the tree orange

https://treeherder.mozilla.org/#/jobs?repo=try&revision=66ca497d30c2f46b21bc96f32d3765f023af8725&selectedJob=234874435

Mike de Boer [:mikedeboer]

Comment 11

•

5 years ago

Well, you should make sure that _engineToUpdate is unset as well

Dale Harvey (:daleharvey)

Assignee

Comment 12

•

5 years ago

Hrm not sure what you mean, it should be unset when updating by https://searchfox.org/mozilla-central/source/toolkit/components/search/SearchService.jsm#3028 right? Looking at the failures now

Dale Harvey (:daleharvey)

Assignee

Comment 13

•

5 years ago

Attached file Bug 1532730 - Update engines on reload. r?mikedeboer — Details

Pulsebot

Comment 14

•

5 years ago

Pushed by dharvey@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ff41bdbd94e5
Update engines on reload. r=mikedeboer

Natalia Csoregi [:nataliaCs]

Comment 15

•

5 years ago

bugherder

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

Status: NEW → RESOLVED

Closed: 5 years ago

status-firefox68: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → Firefox 68

Mike de Boer [:mikedeboer]

Comment 16

•

5 years ago

[Tracking Requested - why for this release]: This was reported during the 67 Nightly cycle, thus is affected as well. We'll need this patch uplifted to ensure regions are properly handled.

status-firefox67: --- → affected

tracking-firefox67: --- → ?

Pascal Chevrel:pascalc

Comment 17

•

5 years ago

Tracking for 67. Please prepare an uplift request.

tracking-firefox67: ? → +

Pascal Chevrel:pascalc

Updated

•

5 years ago

Flags: needinfo?(dharvey)

Timea Cernea [:tbabos][inactive]

Comment 18

•

5 years ago

•

Edited

Hey Mike,

During our Nightly validation process, we crossed upon this bug too. Even if is supposed to be fixed we are not quite sure if it is.

We use nordVPN to change our location (Romania) to USA.
Unfortunately, we will not get the "client=firefox-b-1-d" by duing a search with the awesome bar.
The only workaround we had was to change the browser.search.region manually in about:config, otherwise it will not work.

Worth mentioning that the above pref setting is available in about:config only when we start Nightly normally, without VPN. After enabling the VPN and setting it to USA for example, that pref is no longer displayed in about:config page.

We are unsure if the bug is not fixed or our nordVPN is faulty here. Please let us know if we can help further with debuging this.

edited after further testing

Timea Cernea [:tbabos][inactive]

Updated

•

5 years ago

Flags: needinfo?(mozilla)

Dale Harvey (:daleharvey)

Assignee

Comment 19

•

5 years ago

I 'think' what you are seeing is expected, if your geolocation says you are in the US, but your timezone doesnt match then we dont use that location

https://searchfox.org/mozilla-central/source/toolkit/components/search/SearchService.jsm#419

Flags: needinfo?(dharvey)

Dale Harvey (:daleharvey)

Assignee

Comment 20

•

5 years ago

Comment on attachment 9052267 [details]
Bug 1532730 - Update engines on reload. r?mikedeboer

Beta/Release Uplift Approval Request

Feature/Bug causing the regression: Bug 1492475
User impact if declined: Fresh users will not get the relevant search codes (little impact for user, lots for mozilla)
Is this code covered by automated tests?: Yes
Has the fix been verified in Nightly?: No
Needs manual test from QE?: Yes
If yes, steps to reproduce: Start firefox while in the USA with a fresh profile, restart the browser, do a search on the home page and you should see "?client=google-b-1-d" in the url
List of other uplifts needed: None
Risk to taking this patch: Medium
Why is the change risky/not risky? (and alternatives if risky): Small patch wuth automated testing, but not a totally obvious bug that existed for a while
String changes made/needed:

Attachment #9052267 - Flags: approval-mozilla-beta?

Dale Harvey (:daleharvey)

Assignee

Updated

•

5 years ago

Flags: qe-verify?

Timea Cernea [:tbabos][inactive]

Comment 21

•

5 years ago

Thanks Dale for the answer.

Verified Fixed on latest Nightly 68.0a1 (2019-04-04) (64-bit) on Windows 7/10, Ubuntu 16.04 and MacOS 10.13
We used nordVPN to change our location to USA and also changed the timezone. "client=google-b-1-d" was displayed in the URL.

status-firefox68: fixed → verified

Flags: qe-verify?

Pascal Chevrel:pascalc

Comment 22

•

5 years ago

Comment on attachment 9052267 [details]
Bug 1532730 - Update engines on reload. r?mikedeboer

Tracked P1 67 regression, fix with tests, verified by QA on Nightly, uplift approved for 67 beta 9 thanks.

Attachment #9052267 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Pascal Chevrel:pascalc

Comment 23

•

5 years ago

I would like this bug to be verified by QA again once it is in Beta, thanks.

Flags: qe-verify+

Daniel Varga [:dvarga]

Comment 24

•

5 years ago

bugherder uplift

https://hg.mozilla.org/releases/mozilla-beta/rev/3dc7b1a57231

status-firefox67: affected → fixed

Mike Kaply [:mkaply]

Reporter

Comment 25

•

5 years ago

Glad this worked (and that the timezone check worked)

Flags: needinfo?(mozilla)

Timea Cernea [:tbabos][inactive]

Comment 26

•

5 years ago

Verified - Fixed on latest Beta9 on Windows 10, Mac OS 10.13 and Ubuntu 18.04.

Status: RESOLVED → VERIFIED

status-firefox67: fixed → verified

Flags: qe-verify+