1908393 - BounceTrackingState never gets destroyed even when the tab closes

Status: RESOLVED FIXED

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

Description

[Emma Zühlcke [:emz]]

Both BrowsingContextWebProgress and the static map sBounceTrackingStates have strong reference to BounceTrackingState. That means even when a tab gets destroyed and with it BrowsingContextWebProgress we keep BounceTrackingState alive. Since we create one BounceTrackingState per tab but don't destroy it that means we effectively leak them. For longer sessions with a lot of tabs this could have a negative impact on memory usage.

Comment 1

[Emma Zühlcke [:emz]]

The map was introduced in Bug 1839916, but the feature was only recently enabled in release in Bug 1899130. 128+ is affected. At this point I consider it low severity since we don't leak that much memory. If the issue becomes more severe we can disable the feature in release via Nimbus.

Comment 2

[Emma Zühlcke [:emz]]

Might be nice to uplift a fix to ESR when we have it.

Comment 3

[Emma Zühlcke [:emz]]

Attachment: Bug 1908393 - Only hold strong references to BounceTrackingState in BrowsingContextWebProgress. r=#anti-tracking!,nika!

This ensures that the lifetime of a BounceTrackingState is tied more closely to the lifetime of a tab.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0f6febee3f34
Only hold strong references to BounceTrackingState in BrowsingContextWebProgress. r=nika,anti-tracking-reviewers,timhuang

Comment 5

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/0f6febee3f34

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for ESR128 approval. It grafts cleanly.

Comment 7

[Emma Zühlcke [:emz]]

Comment on attachment 9413576 [details]
Bug 1908393 - Only hold strong references to BounceTrackingState in BrowsingContextWebProgress. r=#anti-tracking!,nika!

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Requested by release manager.
• User impact if declined: Potentially leaking memory / unnecessary objects the more tabs get opened. This would impact longer sessions more.
• Fix Landed on Version: 130
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): Not a completely trivial patch therefore medium risk. The patch changes how BounceTrackingState (per tab) is managed and there is a small risk we're regressing further.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9413576 [details]
Bug 1908393 - Only hold strong references to BounceTrackingState in BrowsingContextWebProgress. r=#anti-tracking!,nika!

Approved for 128.2esr.

Comment 9

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/2595e9392227