Open Bug 1699846 Opened 2 months ago Updated 2 days ago

Audit callers of nsPresContext::GetParentPresContext()

Categories

(Core :: Layout, task)

task

Tracking

()

Fission Milestone M7a

People

(Reporter: dholbert, Unassigned, NeedInfo)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Similar to bug 1599913)

nsPresContext::GetParentPresContext() works fine in-process, but doesn't traverse beyond oop-iframe process boundaries. We need to be sure its callers are OK with that.

See Also: → 1599913

We should complete this audit in M7a. Daniel, please assign someone to this task for M7a (Fx90).

Fission Milestone: --- → M7a
Flags: needinfo?(dholbert)
Depends on: 1709460

There are 15 callsites ("Uses" at https://searchfox.org/mozilla-central/search?q=GetParentPresContext&path= ).

Starting to go through them... The first two look un-concerning.

https://searchfox.org/mozilla-central/rev/2f109387cc6886859d3f985ed9aca352fff653b8/gfx/layers/apz/util/APZCCallbackHelper.cpp#428

  • This one is basically-innocuous; it's an assertion that's becoming less strict. I filed bug 1709460 on that.

https://searchfox.org/mozilla-central/rev/2f109387cc6886859d3f985ed9aca352fff653b8/layout/base/GeometryUtils.cpp#236

  • This one is innocuous / correct. It's checking whether we can compare frames from two different documents, and it's correct & intentional that it fails to work for cross-origin content.

[I'll try to get through the rest before too long; hopping into a meeting now.]

I'll jump ahead to nsPresContext.cpp:

  • Its first call is in nsPresContext::GetInProcessRootContentDocumentPresContext(). That one's definitely fine as-is since it's in an explicitly "in-process" function.
  • Its second call is in nsPresContext::GetRootPresContext(), which seems to be a rabbit-hole in and of itself, with lots of callers that need to be triaged
  • Its third call is in nsPresContext::NotifyInvalidation, which seems to be about handling paints and invalidation. This one looks fine as-is; I don't imagine paint-invalidations would need or want to cross process boundaries.

Besides those and comment 2, the other GetParentPresContext calls are in these files:
PresShell.cpp
ViewportUtils.cpp
nsLayoutUtils.cpp
nsGfxScrollFrame.cpp

Most of these calls look to be APZ-related, and I'm not familiar enough with our APZ code/invariants to know what to make of those. (And I'm not sure which [if any] of those might be GeckoView-specific, which could put them out of scope for the first major fission release.)

Flags: needinfo?(dholbert)

(In reply to Daniel Holbert [:dholbert] from comment #3)

Besides those and comment 2, the other GetParentPresContext calls are in these files:
PresShell.cpp
ViewportUtils.cpp
nsLayoutUtils.cpp
nsGfxScrollFrame.cpp

Most of these calls look to be APZ-related, and I'm not familiar enough with our APZ code/invariants to know what to make of those.

botond, perhaps you could triage/audit the GetParentPresContext calls in these four files? (from the "Uses (nsPresContext::GetParentPresContext)" section of https://searchfox.org/mozilla-central/search?q=GetParentPresContext&path= )

Flags: needinfo?(botond)
You need to log in before you can comment on or make changes to this bug.