Closed Bug 1581160 Opened 3 months ago Closed 3 months ago

Prepackage url-classifier-skip-urls into the binaries

Categories

(Firefox :: Remote Settings Client, task)

task
Not set

Tracking

()

VERIFIED FIXED
Firefox 71
Tracking Status
firefox69 + verified
firefox70 + verified
firefox71 + verified

People

(Reporter: ehsan, Assigned: ehsan)

References

Details

Attachments

(2 files)

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

[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.

Assignee: nobody → ehsan

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
Attachment #9092706 - Flags: approval-mozilla-release?
Attachment #9092706 - Flags: approval-mozilla-beta?
Flags: qe-verify+
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/97189ab33371
Prepackage url-classifier-skip-urls into the binaries; r=leplatrem,glasserc

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.

Attachment #9092706 - Flags: approval-mozilla-release?
Attachment #9092706 - Flags: approval-mozilla-release+
Attachment #9092706 - Flags: approval-mozilla-beta?
Attachment #9092706 - Flags: approval-mozilla-beta+
Status: NEW → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 71

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

Status: RESOLVED → REOPENED
Resolution: FIXED → ---

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
Attachment #9093036 - Flags: approval-mozilla-release?
Attachment #9093036 - Flags: approval-mozilla-beta?
Attachment #9092706 - Flags: approval-mozilla-release?
Attachment #9092706 - Flags: approval-mozilla-release+
Attachment #9092706 - Flags: approval-mozilla-beta?
Attachment #9092706 - Flags: approval-mozilla-beta+
Pushed by eakhgari@mozilla.com:
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

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

Flags: needinfo?(ehsan)
Pushed by eakhgari@mozilla.com:
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
QA Whiteboard: [qa-triaged]
Status: REOPENED → RESOLVED
Closed: 3 months ago3 months ago
Resolution: --- → FIXED

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.

Attachment #9092706 - Flags: approval-mozilla-release?
Attachment #9092706 - Flags: approval-mozilla-release+
Attachment #9092706 - Flags: approval-mozilla-beta?
Attachment #9092706 - Flags: approval-mozilla-beta+
Attachment #9093036 - Flags: approval-mozilla-release?
Attachment #9093036 - Flags: approval-mozilla-release+
Attachment #9093036 - Flags: approval-mozilla-beta?
Attachment #9093036 - Flags: approval-mozilla-beta+
Regressions: 1582053

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).

Status: RESOLVED → VERIFIED
See Also: → 1582123

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

Flags: needinfo?(ehsan)

(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.

Flags: needinfo?(ehsan)

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

Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.