Closed Bug 1562505 Opened 5 years ago Closed 1 year ago

migrate IsRootContentDocument to IsRootContentDocumentCrossProcess or IsRootContentDocumentInProcess

Categories

(Core :: Layout, task, P3)

task

Tracking

()

RESOLVED FIXED
Fission Milestone Future

People

(Reporter: hiro, Unassigned)

References

(Blocks 1 open bug)

Details

No description provided.
Depends on: 1588675
Depends on: 1596494

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? :)

Flags: needinfo?(hikezoe.birchill)

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.

Flags: needinfo?(hikezoe.birchill)

(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).

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
Flags: needinfo?(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.

Flags: needinfo?(dholbert) → needinfo?(hikezoe.birchill)

(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.

Type: defect → task
Flags: needinfo?(hikezoe.birchill)
Fission Milestone: --- → Future
Depends on: 1728660
Summary: Use IsRootContentDocumentCrossProcess for updating RCDRSF stuff or relevant → migrate IsRootContentDocument to IsRootContentDocumentCrossProcess or IsRootContentDocumentInProcess
Depends on: 1728665
Depends on: 1728670
Depends on: 1728682
Depends on: 1728690
Depends on: 1728691
Depends on: 1728693
Depends on: 1728697
Depends on: 1728698
Depends on: 1728699
Depends on: 1728700
Depends on: 1728702
Depends on: 1728706
Depends on: 1728714
Depends on: 1728716
Depends on: 1728718

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).

Severity: normal → S3

This is done now. Bug 1801517 removed IsRootContentDocument.

Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.