1562505 - migrate IsRootContentDocument to IsRootContentDocumentCrossProcess or IsRootContentDocumentInProcess

Status: NEW

(Core :: Layout, task, P3)

Comment 1

[Neha Kochar [:neha]]

Hiro, I came across this when looking at all sub-bugs for rendering-fission meta bug. Is there any work left here or can we call this done? :)

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Not yet. I just skimmed nsPresContext::IsRootContentDocument call sites (related to APZC or some), at least this call site in ScrollToShowRect should use IsRootContentDocumentCrossProcess, also this call site in ScrollFrameHelper::IsAlwaysActive looks confilict with the work making all ancestor scroll frames active that Timothy did recently. Though I am not 100% sure about the latter call site.

There's presumably some more.

Comment 3

[Hiroyuki Ikezoe (:hiro)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #2)

Not yet. I just skimmed nsPresContext::IsRootContentDocument call sites (related to APZC or some), at least this call site in ScrollToShowRect should use IsRootContentDocumentCrossProcess,

In terms of the functionality of the ScrollToShowRect function, this issue is harmless since the function is only used in PresShell::ScrollFrameRectIntoView(), I made it work for fission (bug 1553012).

Comment 4

[Frank Griffith Jr]

Questions

• With the fixes in place, is Bug 1495940 still dependent on the remaining work?
• Is this a blocker for M7? If so, can we update to the appropriate milestone so we have visibility on this as a sub-bug risk? ty

Comment 5

[Daniel Holbert [:dholbert]]

hiro's probably in a better position to answer comment 4 than I am; I'm just now looking at this ticket for the first time. --> Redirecting ni.

Comment 6

[Hiroyuki Ikezoe (:hiro)]

(In reply to Frank Griffith from comment #4)

Questions

• With the fixes in place, is Bug 1495940 still dependent on the remaining work?

Yes, at least we should use IsRootContentDocumentInProcess in call sites where IsRootContentDocument is OK for fission and use IsRootContentDocumentCrossProcess for places where we can replace as I commented in comment 2.

• Is this a blocker for M7? If so, can we update to the appropriate milestone so we have visibility on this as a sub-bug risk? ty

I don't think this is a blocker for M7 (I just asked Timothy about the second case in comment 2). I don't know what the rest of milestones we have. (the only one I can recall is fission-future). Frank, can you please set the milestone?

I am changing this issue as a task.

Comment 7

[Timothy Nikkel (:tnikkel)]

There is only one caller left ScrollFrameHelper::IsAlwaysActive. This one isn't very important. It's part of the IsScrollingActive code, which doesn't have a lot of users, and I'm hoping to remove most users and rename the remaining ones as this notion of "active scrolling" differs from the more important notion of active scrolling (ASRs, displayports, scrollids).