1873490 - Remove StorageDisabledByAntitracking Functions to Simplify StorageAccess helpers

Status: RESOLVED FIXED

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

Description

[Benjamin VanderSloot [:bvandersloot]]

Found this as a refactor that will make things a little clearer in the short term before we solve Bug 1765334 by adding a StorageKey.

Comment 1

[Benjamin VanderSloot [:bvandersloot]]

Attachment: Bug 1873490, part 1 - Refactor the only external call to StorageDisabledByAntiTracking - r=#anti-tracking!

I'm considering dropping this if statement entirely because this is only hit when we disable network state partitioning.
According to code coverage, we only see the ""_ns as the result of this ternary in our testing too.

But, if you disable network state partitioning and then block cookies from a given domain, then we would still want to partition network state I suppose, so I'll leave it like this..

Comment 2

[Benjamin VanderSloot [:bvandersloot]]

Attachment: Bug 1873490, part 2 - Refactor StorageDisabledByAntiTrackingInternal away by coalescing it into StorageDisabledByAntiTracking - r=#anti-tracking!

This makes the single remaining function larger, but makes things much easier to read to my eyes and reuses the branches that exist instead of mirroring them in both functions

Depends on D198034

Comment 3

[Benjamin VanderSloot [:bvandersloot]]

Attachment: Bug 1873490, part 3 - Refactor StorageDisabledByAntiTracking away by coalescing it into InternalStorageAllowedCheck - r=#anti-tracking!

Similarly to part 2, this makes one function bigger, but removes a funciton.

IMO the "and then do more stuff" helper call at the end of a function without a clear separation/purpose and no other callsites
mostly makes the code harder to reason about. Especially when that helper is exported in the header so you can't tell at a
glance that there aren't other callers.

Depends on D198035

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f720e27569ad
part 1 - Refactor the only external call to StorageDisabledByAntiTracking - r=anti-tracking-reviewers,timhuang
https://hg.mozilla.org/integration/autoland/rev/1dbd7bc7c9f2
part 2 - Refactor StorageDisabledByAntiTrackingInternal away by coalescing it into StorageDisabledByAntiTracking - r=anti-tracking-reviewers,timhuang
https://hg.mozilla.org/integration/autoland/rev/357d024bdffa
part 3 - Refactor StorageDisabledByAntiTracking away by coalescing it into InternalStorageAllowedCheck - r=anti-tracking-reviewers,timhuang

Comment 5

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/f720e27569ad
https://hg.mozilla.org/mozilla-central/rev/1dbd7bc7c9f2
https://hg.mozilla.org/mozilla-central/rev/357d024bdffa