1581160 - Prepackage url-classifier-skip-urls into the binaries

Status: VERIFIED FIXED

(Firefox :: Remote Settings Client, task)

Description

[(no longer active)]

We should probably prepackage this collection into our binaries by adding it here: https://searchfox.org/mozilla-central/source/services/settings/dumps/main/moz.build#5

Comment 1

[(no longer active)]

Attachment: Bug 1581160 - Prepackage url-classifier-skip-urls into the binaries;

Comment 2

[(no longer active)]

[Tracking Requested - why for this release]: We should try to uplift this patch in order to make sure Firefox binaries have the work-around for bug 1580416 as soon as they start to run.

Comment 3

[(no longer active)]

Comment on attachment 9092706 [details]
Bug 1581160 - Prepackage url-classifier-skip-urls into the binaries;

Beta/Release Uplift Approval Request

• User impact if declined: Facebook games like the one mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=1580416#c0 may be broken for up to 24 hours after downloading a new Firefox instance.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Load https://apps.facebook.com/ppearls/ with a Facebook login in a newly installed Firefox instance and make sure the game starts to run.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This patch only ensures that the remote settings collection that works around bug 1580416 is available immediately inside the binary right after it is installed.
• String changes made/needed: None

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/97189ab33371
Prepackage url-classifier-skip-urls into the binaries; r=leplatrem,glasserc

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9092706 [details]
Bug 1581160 - Prepackage url-classifier-skip-urls into the binaries;

Prepackages content going out to users via remote-settings after the latency of first-update. Approved for 70.0b7 and 69.0.1.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

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

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/03b97487f359

Comment 8

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/97189ab33371

Comment 9

[(no longer active)]

This isn't fully fixed yet, because we pass syncIfEmpty: false to RemoteSettings.get() but that parameter prevents the local database from being checked.

Comment 10

[(no longer active)]

Attachment: Bug 1581160 - Part 2: Do not prevent the local database from being checked in UrlClassifierSkipListService.jsm;

Comment 11

[(no longer active)]

Comment on attachment 9093036 [details]
Bug 1581160 - Part 2: Do not prevent the local database from being checked in UrlClassifierSkipListService.jsm;

Beta/Release Uplift Approval Request

• User impact if declined: See comment 3.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See comment 3.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Pretty small fix, this was found as a follow-up to the QA tests on the previous fix. The JSM service itself has automated tests.
• String changes made/needed: None

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/92ed48a0afb3
Part 2: Do not prevent the local database from being checked in UrlClassifierSkipListService.jsm; r=johannh,leplatrem

Comment 13

[Arthur Iakab [arthur_iakab]]

Backed out changeset 92ed48a0afb3 (bug 1581160) for causing xpcshell failures on test_http2-proxy.js CLOSED TREE

Backout revision https://hg.mozilla.org/integration/autoland/rev/6ac23178d059c4f68f1133cb934a259bcb19de25

Failed push https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=266877315&revision=92ed48a0afb31560c9d51d29e8d9760bfc10cb52&searchStr=os%2Cx%2C10.14%2Cdebug%2Cxpcshell%2Ctests%2Ctest-macosx1014-64%2Fdebug-xpcshell-e10s-2%2Cx%28x2%29

Failure logs https://treeherder.mozilla.org/logviewer.html#?job_id=266877315&repo=autoland
https://treeherder.mozilla.org/logviewer.html#?job_id=266877328&repo=autoland

ehsan akhgari can you please take a look?

Comment 14

[Cristian Brindusan [:cbrindusan]]

There are also some bc permafailures on browser_siteSpecificWorkArounds.js
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=266887543&repo=autoland&lineNumber=13894

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2751ade4931f
Part 2: Do not prevent the local database from being checked in UrlClassifierSkipListService.jsm; r=johannh,leplatrem

Comment 16

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/2751ade4931f

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9092706 [details]
Bug 1581160 - Prepackage url-classifier-skip-urls into the binaries;

Follow-up fix from the last patch so the local DB will be checked.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d101d46a6dc577e6db5d21e9ceca64d2249530d1
https://hg.mozilla.org/releases/mozilla-release/rev/066c53c8aa9b37529925885a33c94054bc6cfcf5

Comment 19

[Cristian Fogel [:cfogel]]

Issue with the game loading hang still manifests with the current RC build - 69.0.1(20190917135527), 70.0b7 task_Cluster(20190917125555) until a page refresh.
Marking as verified for 69_taskCluster(20190917125740) and 71.0a1 (2019-09-17).

Comment 20

[Cristian Fogel [:cfogel]]

Updating status, added bug 1582123 for the above mentioned issue.

Comment 21

[(no longer active)]

(In reply to Cristian Fogel, QA [:cfogel] from comment #19)

Issue with the game loading hang still manifests with the current RC build - 69.0.1(20190917135527), 70.0b7 task_Cluster(20190917125555) until a page refresh.

This is because the latest beta (beta 7) is built from 2771f6fe489942ec0091773e98ff00f0409f876e which is before the revision in which the second part of the fix for this bug landed. This wouldn't be a problem in the next beta build.

Comment 22

[Cristian Fogel [:cfogel]]

Thank you for the confirmation!
Removing QE flag and will keep track of in the other bug.