1972353 - UpdateTopInnerSizeForRFP() doesn't work properly in testing due to system principal check in Document::RecomputeResistFingerprinting

Status: ASSIGNED

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

Description

[Pier Angelo Vendrame]

My patch for Bug 1591337 has been backed because of broken tests on Android.
After investigating a little bit, I found that nsScreen::GetTopWindowInnerRectForRFP() always returns 0x0 windows (and this makes dom/base/test/test_bug418986-1.html pass on Android, currently, with all sizes set to 0, which is kinda strange).
I think that eventually the problem is this early return in nsPresContext::UpdateTopInnerSizeForRFP().
I wonder if this is related to mochitests running in iframes or something similar.
After removing this check, the tests for my patch still fail: both width and height are off by 4px, but I haven't managed to investigate about why this happens yet.

Comment 1

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

Set release status flags based on info from the regressing bug 1885101

:fkilic, since you are the author of the regressor, bug 1885101, could you take a look?

For more information, please visit BugBot documentation.

Comment 2

[Fatih Kilic [:fkilic]]

Just having ScreenAvailRect does cause (0px,0px). We should have ScreenAvailRect,ScreenRect,ScreenOrientation,WindowOuterSize in the ShouldRFP call.

I'm not sure why the 0x0 would happen on that test though. During the test, we use resistFingerprinting instead of individual targets, so ShouldRFP shouldn't matter. I wonder if it is chrome specific.

Comment 3

[Fatih Kilic [:fkilic]]

Attachment: Bug 1972353 - Add IsRFPEnabledFor(..., RFPTargetSet, ...). r?tjr

This patch allows us to check more than one target with a single call. Optional patch.

Comment 4

[Fatih Kilic [:fkilic]]

Attachment: Bug 1972353 - Check for all RFP targets that use the TopInnerSizeForRFP value. r?tjr

Comment 5

[Fatih Kilic [:fkilic]]

After removing this check, the tests for my patch still fail: both width and height are off by 4px, but I haven't managed to investigate about why this happens yet

I think we place 2px border on each side of an iframe. so that equates to 4px, but I'm not sure why that would affect your test.

(putting leave open because I don't think these patches will fix the off-by-4px issue.)

Comment 6

[Fatih Kilic [:fkilic]]

I think we place 2px border on each side of an iframe. so that equates to 4px, but I'm not sure why that would affect your test.

I mean after all, the point of TopInnerSizeForRFP is to return the inner size of the top document, not the top iframe.

Are you having this issue only on Android?

Comment 7

[Pier Angelo Vendrame]

(In reply to Fatih Kilic [:fkilic] from comment #6)

I think we place 2px border on each side of an iframe. so that equates to 4px, but I'm not sure why that would affect your test.

I mean after all, the point of TopInnerSizeForRFP is to return the inner size of the top document, not the top iframe.

Are you having this issue only on Android?

Yep!
But on Android we still return the inner window as it is, on other platforms we spoof values.
So, maybe I'm not seeing the problem because the iframe size is close to the top size and the spoofing removes any differences.

Comment 8

[Fatih Kilic [:fkilic]]

But on Android we still return the inner window as it is, on other platforms we spoof values.

You mean we don't apply any offset for Android right, because I think we return TopInnerSizeForRFP on Android too.

So, maybe I'm not seeing the problem because the iframe size is close to the top size and the spoofing removes any differences.

I think the 4px difference would be still there honestly.

I rechecked the UpdateTopInnerSizeForRFP function, and I and I don't know why we would have this difference.

Comment 9

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

Set release status flags based on info from the regressing bug 1885101

Comment 10

[Fatih Kilic [:fkilic]]

AFAICT, the code that runs in bug418986-1.js file is executed with a system privileged doc. I can't tell why that's the case though. Like, for dom/base/test/chrome/test_bug418986-1.xhtml I understand it, but for dom/base/test/test_bug418986-1.html I don't understand why. Anyway, the UpdateTopInnerSizeForRFP function returns early because of the RFP check and since we only record the top doc's inner size and because it is considered a chrome doc, we never store the inner size. so we always return 0 during testing. I couldn't diagnose why this is the case. This happens on both desktop and Android, but doesn't happen with live docs. So, we are not causing any issues to real websites.