1721840 - ScreenOrientation::ShouldResistFingerprinting is hangey?

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P3)

Description

[Tom Ritter [:tjr]]

According to hang-stats mozilla::dom::ScreenOrientation::ShouldResistFingerprinting() is consistently found in hangs every day.

The content of this function is

bool ScreenOrientation::ShouldResistFingerprinting() const {
  bool resist = false;
  if (nsCOMPtr<nsPIDOMWindowInner> owner = GetOwner()) {
    resist = nsContentUtils::ShouldResistFingerprinting(owner->GetDocShell());
  }
  return resist;
}

Which seems pretty tight. The call to ShouldResistFingerprinting() inside the function cascades down to several other ShouldResistFingerprinting() functions, some which are inlined I'm sure, but the more complicated one seems unlikely to be inlined...

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

This is a bit surprising. Shouldn't resist fingerprinting pref be usually off?

Comment 2

[Tom Ritter [:tjr]]

Doesn't matter if it's off or not; we still check if it is enabled on a ton of code paths.

Presently our plan has been to change all of the ShouldRFP checks into detailed checks that look at the current context (principal/addon/origin). This would let us control RFP on a very granular level, which is something people have sought for a long time. Maybe we need to wrap those detailed checks with a simple "Is RFP enabled at all" check which might avoid this hangey problem which is presumably coming from the GetOwner()/GetDocShell() calls. (Because we don't see other ShouldRFP checks showing up in hang stats, just this one.)

Comment 3

[Alisha Sharma]

Attachment: Bug 1721840: Wrap RFP Check with a more coarse one to avoid hangs r?tjr

Comment 4

[Alisha Sharma]

Attachment: Commit for Bug 1721840: Patch Test Edited r=tjr

Depends on D126165

Comment 5

[Ben]

Attachment: Bug 1721840 - Surrounded bool resist = false parameter with outlying if statement and return function. r?tjr

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f70260102c95
Wrap RFP Check with a more coarse one to avoid hangs r=tjr

Comment 7

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/f70260102c95

Comment 8

[Ben]

Attachment: Bug 1721840 - Wrapped bool ScreenOrientation function with if statement and return function. *fixed formatting* r?tjr

Depends on D128171

Comment 9

[Ben]

Attachment: Bug 1721840 - Surrounded book ShouldResistFingerprinting function with if statement and return function (formatting fixed). r?tjr

Depends on D128172

Comment 10

[ccraig8]

Attachment: WIP: Bug 1721840

Cody Craig
Surrounded lines 573-577 with if(nsContentUtils::ShouldResistFingerprinting()){/stuff already in lines goes here/}
return false;

Should offer a good testing tool for further work