1599379 - Level 2 (Strict) List cookies are not being blocked after restart and load tracker immediately

Status: VERIFIED FIXED

(Core :: Privacy: Anti-Tracking, defect)

Description

[Simona Badau, Desktop QA]

Attachment: ETP-restart.mp4

Mozilla/5.0 (X11; Linux x86_64; rv:72.0) Gecko/20100101 Firefox/72.0
Build ID: 20191125214644

Affected versions:

• Nightly 72.0a1

Affected platforms:

• Ubuntu 18.04 x64
• Windows 10
• Mac OS X 10.14

Steps to reproduce:

1. Open Firefox
2. Navigate to: https://senglehardt.com/test/trackingprotection/test_pages/tracking_protection.html
3. Look at the "Level 2 (Strict) List" section at the bottom of the page.
4. Click on the Shield icon and turn ETP off
5. Click again on the Shield icon turn ETP back on
6. Restart Firefox (used keyboard combination: Ctrl+Shift+J and after Ctrl+Alt+R)
7. Look at the "Level 2 (Strict) List" section at the bottom of the page.

Actual results:
After the restart, the message "Cookies not blocked" is displayed.

Expected results:
After the restart, the displayed message should be "Cookies BLOCKED".

Comment 1

[(no longer active)]

This looks very similar to bug 1599137, in one case you restart the browser before an update, in the other case you just reload the page after changing settings...

I wonder if they somehow have the same underlying cause? Dimi, do you mind having a look at this one too? Setting a dependency for now...

Comment 2

[Dimi Lee [:dimi]]

This looks like a timing issue when you launch Firefox with page with trackers.
I can reproduce this without turning ETP on/off.

Comment 3

[Dimi Lee [:dimi]]

Attachment: Bug 1599379 - Create a DBService instance in GetWorker API when it doesn't exist. r?baku

There are two places using DBService during a page load at startup:

1. nsChannelClassifier::Start, used by Phishing Protection
2. AsyncChannelClassifier::CheckChannel, used by Tracking Protection

Tracking protection checks happen before establishing a network connection, so it happens
prior to phishing protection checkes. When we load a page at
startup, ::CheckChannel API is called, but DBService is not yet created.

This patch fixes this issue by creating a DBService instance when
::GetWorker API is called without a DBService instance.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ecc4010a247c
Create a DBService instance in GetWorker API when it doesn't exist. r=baku

Comment 5

[Andrei Ciure[:aciure]]

Backed out changeset ecc4010a247c (bug 1599379) for causing bustages

push that caused the backout: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=279017478&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel%2Crunnable&revision=ecc4010a247c0f439bf01b33644d822956f16028

backout: https://hg.mozilla.org/integration/autoland/rev/3a7b90a6ba2cc27dd87d4d809d5e400edc58c6ac

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/42913f0f886d
Create a DBService instance in GetWorker API when it doesn't exist. r=baku

Comment 7

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/42913f0f886d

Comment 8

[Julien Cristau [:jcristau]]

Do we need this in 72? Please request uplift if so.

Comment 9

[Simona Badau, Desktop QA]

Mozilla/5.0 (X11; Linux x86_64; rv:73.0) Gecko/20100101 Firefox/73.0

Verified as fixed using the latest Nightly 73.0a1 on Windows 10 x64, Mac OS X 10.14 and Ubuntu 18.06 x64.

Comment 10

[Dimi Lee [:dimi]]

Comment on attachment 9112487 [details]
Bug 1599379 - Create a DBService instance in GetWorker API when it doesn't exist. r?baku

Beta/Release Uplift Approval Request

• User impact if declined: Tracking protection related features don't work for the first few loads (number of loads affected is related to the timing).
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See bug description to reproduce this bug
• 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 advances the timing we create URL Classifier worker thread, it doesn't change the logic that how we classify trackers.
• String changes made/needed: None

Comment 11

[Julien Cristau [:jcristau]]

Comment on attachment 9112487 [details]
Bug 1599379 - Create a DBService instance in GetWorker API when it doesn't exist. r?baku

tracking protection fix, verified in nightly, approved for 72.0b5

Comment 12

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/releases/mozilla-beta/rev/ad508808fa91

Comment 13

[Simona Badau, Desktop QA]

Verified as fixed on Firefox 72 beta 5 - tested on Mac OS X 10.14, Ubuntu 18.04 x64, Windows 10 x64 and Windows 7.