1599043 - Changing the propagation of the OnContentBlockingEvent

Status: RESOLVED FIXED

(Core :: Privacy: Anti-Tracking, task, P1)

Description

[Dimi Lee [:dimi]]

To support fission, we plan to move the OnContentBlockingEvent entirely to the parent.

Content blocking events are now propagated in the following call sites:

1. UrlClassifierCommon::SetBlockedContent
2. UrlClassifierCommon::AnnotateChannel
3. AntiTrackingCommon::NotifyBlockingDecision
4. AntiTrackingCommon::AddFirstPartyStorageAccessGrantedFor

Comment 1

[Dimi Lee [:dimi]]

The plan is to move the OnContentBlockingEvent into the parent process while maintaining two copies of the ContentBlockingLog in both the parent and content process and gradually move the use cases of OnContentBlockingEvent in the child process to the parent process(Bug 1599046).

This allows us to land patch piece by piece instead of landing a giant patch to complete all the tasks.

Comment 2

[Tim Huang[:timhuang]]

Attachment: Bug 1599043 - Part 1: Add a ContentBlockingLog into the WindowGlobalParent r?dimi

This adds a ContentBlockingLog into the WindowGlobalParent. This
ContentBlockingLog is bascially a copy of the log in the content
process. This log in parent is needed for the OnContentBlockingEvent in
parent process since we need an overview of the content blocking events
for OnContentBlockingEvent. And the ContentBlockingLog exits in the
content process in this stage, so we need to maintain a copy of it to
get the overview of the blocking events in parent.

Comment 3

[Tim Huang[:timhuang]]

Attachment: Bug 1599043 - Part 2: Allow the WindowGlobalParent to send OnContentBlockingEvent in parent. r?dimi

This patch makes the WindowGlobalParent to be able to send the
OnContentBlockingEvent in the parent process.

Depends on D54929

Comment 4

[Tim Huang[:timhuang]]

Attachment: Bug 1599043 - Part 3: Stop propagation of the OnContentBlockingEvent in content processes. r?dimi

This patch moves the propatation of the onContentBlockingEvent in the
content processes. After this, the
nsGlobalWindowOuter::NotifyContentBlockingEvent will only update the
ContentBlockingLog.

Depends on D55645

Comment 5

[Tim Huang[:timhuang]]

Attachment: Bug 1599043 - Part 4: Notify the OnContentBlockingEvent in the parent process in UrlClassifierCommon. r=dimi,Ehsan

We make the OnContentBlockingEvent to be notified in the parent procees
for UrlClassifierCommon. There are two place would trigger this,
UrlClassifierCommon::SetBlockedContent and
UrlClassifierCommon::AnnotateChannel. But we still send to the child
process to notify the content blocking since we need to update the log
in the content process in this stage.

Depends on D55646

Comment 6

[Tim Huang[:timhuang]]

Attachment: Bug 1599043 - Part 5: Add an IPC message to allow content process to notify the OnContentBlockingEvent in the parent. r?dimi

This patch adds an IPC message which allows content process to notify
the OnContentBlockingEvent in the parent process. This is needed because
there are some situations that the content blocking happens in content
processes, so we need to tell the parent process to notify it. Such as,
AntiTrackingCommon::AntiTrackingCommon::AddFirstPartyStorageAccessGrantedFor().

Depends on D55647

Comment 7

[Tim Huang[:timhuang]]

Attachment: Bug 1599043 - Part 6: Make the AntiTrackingCommon to send IPC to parent to notify the OnContentBlockingEvent. r=dimi,Ehsan

There are two places in the AntiTrackingCommon that it would notify the
content blocking event, NotifyBlockingDecision() and
AddFirstPartyStorageAccessGrantedFor(). We make these two places to
send a IPC to parent to issue parent to notify the event.

Depends on D55648

Comment 8

[Tim Huang[:timhuang]]

Attachment: Bug 1599043 - Part 7: Update tests to adapt the change of the OnContentBlockingEvent. r=dimi,johannh

There are three major issues that we need to fix for tests if we move the
OnContentBlockingEvent to the parent process. First, we notify the event
in the idle queue in the parent process. For some reasons, the test
script could outrun the callbacks of the OnContentBlockingEvent. So, it
could happen that the UI hasn't got updated while we check its state. To
fix this, we make the test script to wait the UI to be updated
explicitly.

Second, the tracking UI tests would access the ContentblockingLog in the
content in order to get the hosts of different blocking categories. And
the log in the content may not be updated when we open the subview. And
there is no explicit way to know whether the log is updated or not. So,
we use SetTimeout to temporary fix this issue. Once we finish the work
that moving the ContentBlockingLog into the parent (Bug1599046), we can
remove this work-around.

Third, there is one test that it waits OnContentBlockingEvent in the
content. We make it to listen the event in the parent to fix it.

Depends on D55649

Comment 9

[Tim Huang[:timhuang]]

Attachment: Bug 1599043 - Part 8: Remove the unnecessary OnContentBlockingEvent event in PBrowser. r=dimi,Ehsan

The OnContentBlockingEvent is no longer needed once we make the
OnContentBlockingEvent parent only.

Comment 10

[Tim Huang[:timhuang]]

Attachment: Bug 1599043 - Part 9: Set channel matched info in channels in parent process. r=dimi

This patch makes the matched info also set into the channels in the
parent process. This info is neeced for the ContentBlocking telemetry
and GeckoView also relies on it.

Depends on D55788

Comment 11

[Tim Huang[:timhuang]]

Attachment: Bug 1599043 - Part 10: Make GeckoView to listen OnContentBlockingEvent on the parent process.

The GeckoView is listening OnContentBlockingEvent in the content process.
As we move the event into the parent process, we have to change it to
listen the event in the parent process.

This patch also adds a workaround in the test
ContentBlockingControllerTest#getLog(). This workaround adds a 500ms
delays before we check the ContentBlockingLog. This is needed because there
is a delay between the notification of OnContentBlockingEven in the parent
process and the actual recording of the log in the content process. This
workaround will be no longer needed once we move the log entirely to the
parent process (Bug 1599046).

Depends on D56748

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/68219c132efc
Part 1: Add a ContentBlockingLog into the WindowGlobalParent r=dimi,Ehsan
https://hg.mozilla.org/integration/autoland/rev/0ad30634e416
Part 2: Allow the WindowGlobalParent to send OnContentBlockingEvent in parent. r=dimi,Ehsan
https://hg.mozilla.org/integration/autoland/rev/aaf83f7806b4
Part 3: Stop propagation of the OnContentBlockingEvent in content processes. r=dimi,Ehsan
https://hg.mozilla.org/integration/autoland/rev/c8ed9c52508c
Part 4: Notify the OnContentBlockingEvent in the parent process in UrlClassifierCommon. r=dimi,Ehsan
https://hg.mozilla.org/integration/autoland/rev/a68bd2acf5d6
Part 5: Add an IPC message to allow content process to notify the OnContentBlockingEvent in the parent. r=dimi,Ehsan
https://hg.mozilla.org/integration/autoland/rev/6b97581c33b6
Part 6: Make the AntiTrackingCommon to send IPC to parent to notify the OnContentBlockingEvent. r=dimi,Ehsan
https://hg.mozilla.org/integration/autoland/rev/e4ea0d47136b
Part 7: Update tests to adapt the change of the OnContentBlockingEvent. r=dimi,johannh
https://hg.mozilla.org/integration/autoland/rev/1eadd9f5b50e
Part 8: Remove the unnecessary OnContentBlockingEvent event in PBrowser. r=dimi,Ehsan
https://hg.mozilla.org/integration/autoland/rev/7c3f0a58cc6c
Part 9: Set channel matched info in channels in parent process. r=dimi
https://hg.mozilla.org/integration/autoland/rev/463b815557e4
Part 10: Make GeckoView to listen OnContentBlockingEvent on the parent process. r=geckoview-reviewers,agi

Comment 13

[Oana Pop-Rus]

Backed out 10 changesets (bug 1599043) for build bustages in nsGlobalWindowOuter.cpp

Push that started the failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=superseded%2Ctestfailed%2Cbusted%2Cexception%2Crunnable&revision=463b815557e472c6d158cc0c2cc591cb140a4eb8&selectedJob=280883693

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=280883693&repo=autoland&lineNumber=32878

Backout: https://hg.mozilla.org/integration/autoland/rev/d8ea4edd854df037cba6750f5dc1390ff3b8afc2

Comment 14

[BugBot [:suhaib / :marco/ :calixte]]

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:timhuang, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 15

[Tim Huang[:timhuang]]

We planed to land this patch in Nightly 74. I will land this when the soft code freeze ends.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2c4f84b0e661
Part 1: Add a ContentBlockingLog into the WindowGlobalParent r=dimi,Ehsan
https://hg.mozilla.org/integration/autoland/rev/7316a18109f9
Part 2: Allow the WindowGlobalParent to send OnContentBlockingEvent in parent. r=dimi,Ehsan
https://hg.mozilla.org/integration/autoland/rev/c4c58db85479
Part 3: Stop propagation of the OnContentBlockingEvent in content processes. r=dimi,Ehsan
https://hg.mozilla.org/integration/autoland/rev/3de15e4fce42
Part 4: Notify the OnContentBlockingEvent in the parent process in UrlClassifierCommon. r=dimi,Ehsan
https://hg.mozilla.org/integration/autoland/rev/88500327ad55
Part 5: Add an IPC message to allow content process to notify the OnContentBlockingEvent in the parent. r=dimi,Ehsan
https://hg.mozilla.org/integration/autoland/rev/ec18a6d2b59b
Part 6: Make the AntiTrackingCommon to send IPC to parent to notify the OnContentBlockingEvent. r=dimi,Ehsan
https://hg.mozilla.org/integration/autoland/rev/26f6cebb90ac
Part 7: Update tests to adapt the change of the OnContentBlockingEvent. r=dimi,johannh
https://hg.mozilla.org/integration/autoland/rev/462cef293a45
Part 8: Remove the unnecessary OnContentBlockingEvent event in PBrowser. r=dimi,Ehsan
https://hg.mozilla.org/integration/autoland/rev/578070e7b439
Part 9: Set channel matched info in channels in parent process. r=dimi
https://hg.mozilla.org/integration/autoland/rev/3a266fb44ba4
Part 10: Make GeckoView to listen OnContentBlockingEvent on the parent process. r=geckoview-reviewers,agi

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b2de33dc3f66
Part 11: Set browser_protections_UI.js as expected to pass to fix unexpected pass. CLOSED TREE

Comment 18

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/2c4f84b0e661
https://hg.mozilla.org/mozilla-central/rev/7316a18109f9
https://hg.mozilla.org/mozilla-central/rev/c4c58db85479
https://hg.mozilla.org/mozilla-central/rev/3de15e4fce42
https://hg.mozilla.org/mozilla-central/rev/88500327ad55
https://hg.mozilla.org/mozilla-central/rev/ec18a6d2b59b
https://hg.mozilla.org/mozilla-central/rev/26f6cebb90ac
https://hg.mozilla.org/mozilla-central/rev/462cef293a45
https://hg.mozilla.org/mozilla-central/rev/578070e7b439
https://hg.mozilla.org/mozilla-central/rev/3a266fb44ba4
https://hg.mozilla.org/mozilla-central/rev/b2de33dc3f66