Add information about the old state flags and a detailed log of each tab's content blocking history to the onSecurityChange progress listener callbacks

RESOLVED FIXED in Firefox 64

Status

()

enhancement
RESOLVED FIXED
8 months ago
6 months ago

People

(Reporter: Ehsan, Assigned: Ehsan)

Tracking

(Blocks 1 bug)

unspecified
mozilla64
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox64 fixed)

Details

Attachments

(12 attachments)

46 bytes, text/x-phabricator-request
baku
: review+
Details | Review
46 bytes, text/x-phabricator-request
baku
: review+
Details | Review
46 bytes, text/x-phabricator-request
baku
: review+
Details | Review
46 bytes, text/x-phabricator-request
baku
: review+
Details | Review
46 bytes, text/x-phabricator-request
baku
: review+
Details | Review
46 bytes, text/x-phabricator-request
baku
: review+
Details | Review
46 bytes, text/x-phabricator-request
baku
: review+
Details | Review
46 bytes, text/x-phabricator-request
baku
: review+
Details | Review
46 bytes, text/x-phabricator-request
baku
: review+
Details | Review
46 bytes, text/x-phabricator-request
baku
: review+
Details | Review
46 bytes, text/x-phabricator-request
baku
: review+
Details | Review
46 bytes, text/x-phabricator-request
baku
: review+
Details | Review
Assignee

Description

8 months ago
This information will allow us the flexibility needed to build the upcoming UX designs, and also to fix issues such as bug 1488978 and also help in representing the status of what has happened on the page more accurately in the control centre.

I have a patch series that adds the necessary progress listener infrastructure.  The main part is the content blocking log, that exposes a JSON object with a detailed history of the content blocking decisions made on each tab.
Assignee

Comment 6

8 months ago
This check was originally added for tracking protection, and we need to keep
the state of the document updated for the rest of our blocking states even
for third-party channels.

Depends on D6595
Assignee

Comment 10

8 months ago
It is arguably more accurate to implement these boolean
getters in terms of whether we remember blocking anything
in the category being asked about.  This will allow us to
correctly account for hiding the sheild icon when all
currently blocked trackers become unblocked, for example.

Depends on D6599
Comment on attachment 9011340 [details]
Bug 1493563 - Part 1: Make ReportBlockingToConsole() accept an nsIURI* argument instead of an nsIHttpChannel*

Andrea Marchesini [:baku] has approved the revision.
Attachment #9011340 - Flags: review+
Comment on attachment 9011341 [details]
Bug 1493563 - Part 2: Record a log of content blocking actions on each top-level document

Andrea Marchesini [:baku] has approved the revision.
Attachment #9011341 - Flags: review+
Comment on attachment 9011342 [details]
Bug 1493563 - Part 3: Mark nsISecurityEventSink as non-scriptable since there are no scriptable consumers

Andrea Marchesini [:baku] has approved the revision.
Attachment #9011342 - Flags: review+
Comment on attachment 9011343 [details]
Bug 1493563 - Part 4: Present the old state and the content blocking log to the security event sink

Andrea Marchesini [:baku] has approved the revision.
Attachment #9011343 - Flags: review+
Comment on attachment 9011345 [details]
Bug 1493563 - Part 6: Only restrict notifying same loading URI channels to tracking protection notifications

Andrea Marchesini [:baku] has approved the revision.
Attachment #9011345 - Flags: review+
Comment on attachment 9011346 [details]
Bug 1493563 - Part 7: Store the log more compactly, and cap the size of the origin log at a maximum limit adjustable by a pref

Andrea Marchesini [:baku] has approved the revision.
Attachment #9011346 - Flags: review+
Comment on attachment 9011347 [details]
Bug 1493563 - Part 8: Report the memorty usage of the Content Blocking log

Andrea Marchesini [:baku] has approved the revision.
Attachment #9011347 - Flags: review+
Comment on attachment 9011349 [details]
Bug 1493563 - Part 10: Implement the per-document blocked states in terms of the content blocking log

Andrea Marchesini [:baku] has approved the revision.
Attachment #9011349 - Flags: review+
Comment on attachment 9011350 [details]
Bug 1493563 - Part 11: Add tests for the new onSecurityChange progresslistener arguments

Andrea Marchesini [:baku] has approved the revision.
Attachment #9011350 - Flags: review+
Comment on attachment 9011348 [details]
Bug 1493563 - Part 9: Notify about trackers being unblocked when being granted a first-party exception

Andrea Marchesini [:baku] has approved the revision.
Attachment #9011348 - Flags: review+
Comment on attachment 9011344 [details]
Bug 1493563 - Part 5: Present the old state and the content blocking log to the web progress listeners

Andrea Marchesini [:baku] has approved the revision.
Attachment #9011344 - Flags: review+

Comment 23

8 months ago
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f9bac5e73bd4
Part 1: Make ReportBlockingToConsole() accept an nsIURI* argument instead of an nsIHttpChannel*; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/de06604fefcb
Part 2: Record a log of content blocking actions on each top-level document; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/9085269938cd
Part 3: Mark nsISecurityEventSink as non-scriptable since there are no scriptable consumers; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/c64a2ea15b7b
Part 4: Present the old state and the content blocking log to the security event sink; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/287666ecdd17
Part 5: Present the old state and the content blocking log to the web progress listeners; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/84dc54c1a9c1
Part 6: Only restrict notifying same loading URI channels to tracking protection notifications; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/5f922cd6c21e
Part 7: Store the log more compactly, and cap the size of the origin log at a maximum limit adjustable by a pref; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/205d69890f50
Part 8: Report the memorty usage of the Content Blocking log; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/68de8f0866d0
Part 9: Notify about trackers being unblocked when being granted a first-party exception; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/651f87dca51f
Part 10: Implement the per-document blocked states in terms of the content blocking log; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/d755b96837ac
Part 11: Add tests for the new onSecurityChange progresslistener arguments; r=baku

Comment 25

8 months ago
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/66787e603401
Part 1: Make ReportBlockingToConsole() accept an nsIURI* argument instead of an nsIHttpChannel*; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/56bab7fae64d
Part 2: Record a log of content blocking actions on each top-level document; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/f4ca3d8fa90e
Part 3: Mark nsISecurityEventSink as non-scriptable since there are no scriptable consumers; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/a6141dbd0328
Part 4: Present the old state and the content blocking log to the security event sink; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/dbba8451aab1
Part 5: Present the old state and the content blocking log to the web progress listeners; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/7a8a0d1cfd69
Part 6: Only restrict notifying same loading URI channels to tracking protection notifications; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/eae105f2bf6d
Part 7: Store the log more compactly, and cap the size of the origin log at a maximum limit adjustable by a pref; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/f58d0b1ca088
Part 8: Report the memorty usage of the Content Blocking log; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/4f608b8801d4
Part 9: Notify about trackers being unblocked when being granted a first-party exception; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/8e25c301675a
Part 10: Implement the per-document blocked states in terms of the content blocking log; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/0bb55b604bfd
Part 11: Add tests for the new onSecurityChange progresslistener arguments; r=baku
Assignee

Updated

8 months ago
Flags: needinfo?(ehsan)

Comment 28

8 months ago
Note that I just landed https://bugzilla.mozilla.org/show_bug.cgi?id=1493655 on autoland which removes the TabParent implementation of nsISecureBrowserUI, and otherwise may conflict with some of the patches here. I would have landed on inbound to make rebasing easier, except that my patch depended on the fix in https://bugzilla.mozilla.org/show_bug.cgi?id=1494009 which landed on autoland but hasn't merged around to central yet.
Assignee

Updated

8 months ago
Blocks: 1490813, 1488978
Assignee

Comment 29

8 months ago
(In reply to :Gijs (out Thu 27 - Sun 30 / 9;  he/him) from comment #28)
> Note that I just landed https://bugzilla.mozilla.org/show_bug.cgi?id=1493655
> on autoland which removes the TabParent implementation of
> nsISecureBrowserUI, and otherwise may conflict with some of the patches
> here. I would have landed on inbound to make rebasing easier, except that my
> patch depended on the fix in
> https://bugzilla.mozilla.org/show_bug.cgi?id=1494009 which landed on
> autoland but hasn't merged around to central yet.

Thanks for the update!
Flags: needinfo?(ehsan)
Assignee

Comment 30

8 months ago
So here is some more information on the intermittent failure in comment 26.

Through a set of try pushes I realized that the intermittent failure stems from part 6.  After pushing a debugging patch, it became clear that the code in part 6 is being tickled when network.cookie.cookieBehavior is set to 1 (BEHAVIOR_REJECT_FOREIGN).  The test immediately before this one (test_sharedWorker_thirdparty.html) sets this pref to 1 <https://searchfox.org/mozilla-central/rev/ce57be88b8aa2ad03ace1b9684cd6c361be5109f/dom/workers/test/test_sharedWorker_thirdparty.html#50> so this is the result of the actions of the previous test bleeding into this one.  I pushed a few different band aids to try to see which one sticks...
Assignee

Comment 31

8 months ago
(In reply to :Ehsan Akhgari from comment #30)
> So here is some more information on the intermittent failure in comment 26.
> 
> Through a set of try pushes I realized that the intermittent failure stems
> from part 6.  After pushing a debugging patch, it became clear that the code
> in part 6 is being tickled when network.cookie.cookieBehavior is set to 1
> (BEHAVIOR_REJECT_FOREIGN).  The test immediately before this one
> (test_sharedWorker_thirdparty.html) sets this pref to 1
> <https://searchfox.org/mozilla-central/rev/
> ce57be88b8aa2ad03ace1b9684cd6c361be5109f/dom/workers/test/
> test_sharedWorker_thirdparty.html#50> so this is the result of the actions
> of the previous test bleeding into this one.  I pushed a few different band
> aids to try to see which one sticks...

OK, this theory didn't pan out unfortunately.  Back to square one...
Assignee

Comment 33

8 months ago
I tried everything else that I could think of and ran out of all other options.  Even setting the network.cookie.cookieBehavior pref here to 0 doesn't change anything, I really have no idea what to do about this.  I think this has exceeded the cost/benefit threshold at this point and I can justify disabling the test on Linux64 asan opt.
Comment on attachment 9012752 [details]
Bug 1493563 - Part 12: Disable the failing test_sharedworker_event_listener_leaks.html test on Linux64 opt asan builds

Andrea Marchesini [:baku] has approved the revision.
Attachment #9012752 - Flags: review+

Comment 35

8 months ago
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/932b41e211e0
Part 1: Make ReportBlockingToConsole() accept an nsIURI* argument instead of an nsIHttpChannel*; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/a1ff0cd62563
Part 2: Record a log of content blocking actions on each top-level document; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/c8ab17addb7a
Part 3: Mark nsISecurityEventSink as non-scriptable since there are no scriptable consumers; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/1844af4cc25b
Part 4: Present the old state and the content blocking log to the security event sink; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/83d6c2bf8dc6
Part 5: Present the old state and the content blocking log to the web progress listeners; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/ce0211be57a1
Part 6: Only restrict notifying same loading URI channels to tracking protection notifications; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/bb2720a401fe
Part 7: Store the log more compactly, and cap the size of the origin log at a maximum limit adjustable by a pref; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/3ef707008502
Part 8: Report the memorty usage of the Content Blocking log; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/02b8b073f7d7
Part 9: Notify about trackers being unblocked when being granted a first-party exception; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/344298c73ee7
Part 10: Implement the per-document blocked states in terms of the content blocking log; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/1ce58f004593
Part 11: Add tests for the new onSecurityChange progresslistener arguments; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/d2e83655082f
Part 12: Disable the failing test_sharedworker_event_listener_leaks.html test on Linux64 opt asan builds; r=baku
Assignee

Comment 37

8 months ago
You backed out the wrong patches.
Flags: needinfo?(ehsan)

Comment 38

8 months ago
Pushed by eakhgari@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/059af370e80e
Part 1: Make ReportBlockingToConsole() accept an nsIURI* argument instead of an nsIHttpChannel*; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/27165f07ad1a
Part 2: Record a log of content blocking actions on each top-level document; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/3243e8819a28
Part 3: Mark nsISecurityEventSink as non-scriptable since there are no scriptable consumers; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/d0997972e4d4
Part 4: Present the old state and the content blocking log to the security event sink; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/f8849239da42
Part 5: Present the old state and the content blocking log to the web progress listeners; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/acbf4450b55e
Part 6: Only restrict notifying same loading URI channels to tracking protection notifications; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/c25ea7b5e8d8
Part 7: Store the log more compactly, and cap the size of the origin log at a maximum limit adjustable by a pref; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/e255803a5af2
Part 8: Report the memorty usage of the Content Blocking log; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/c5858df458e7
Part 9: Notify about trackers being unblocked when being granted a first-party exception; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/dc92e014bec4
Part 10: Implement the per-document blocked states in terms of the content blocking log; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/cdb8932d1d8d
Part 11: Add tests for the new onSecurityChange progresslistener arguments; r=baku
https://hg.mozilla.org/integration/mozilla-inbound/rev/8db78a0b0d4f
Part 12: Disable the failing test_sharedworker_event_listener_leaks.html test on Linux64 opt asan builds; r=baku
Depends on: 1495207
Depends on: 1497555
No longer depends on: 1497555
Depends on: 1497555
Assignee

Updated

7 months ago
No longer depends on: 1497555
Assignee

Updated

7 months ago
No longer depends on: 1495207
No longer depends on: 1497014
Assignee

Updated

7 months ago
Depends on: 1497014
Assignee

Updated

6 months ago
Depends on: 1510911
You need to log in before you can comment on or make changes to this bug.