Closed Bug 1727183 Opened 2 months ago Closed 25 days ago

[Fission] Audit callers of Document::EnumerateSubDocuments

Categories

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

defect

Tracking

()

RESOLVED FIXED
Fission Milestone MVP
Tracking Status
firefox-esr78 --- disabled
firefox-esr91 --- disabled
firefox91 --- disabled
firefox92 --- wontfix
firefox93 --- affected

People

(Reporter: smaug, Assigned: edgar)

References

Details

(Whiteboard: fission-soft-blocker)

Some callers expect to be able to go through all the descendants using recursive calls.

Fission Milestone: --- → ?

I will take a look at this.

Assignee: nobody → echen

(In reply to Edgar Chen [:edgar] from comment #1)

I will take a look at this.

Thanks!

Tracking this bug for Fission Milestone MVP for now.

Fission Milestone: ? → MVP
Whiteboard: fission-soft-blocker

The usage in Document::NotifyLayerManagerRecreated looks fine, the remote iframe would be also handled from BrowserChild::ReinitRendering.

The usage for fullscreen Looks fine.

And there are some tests added for remote iframe, see bug 1665941.

The usage in nsHTMLDocument::WillIgnoreCharsetOverride looks fine, as it only cares about the subdocument with same principal.

The usage in PresShell.cpp

  • FlushThrottledStyles looks fine as this used while handling the event which only cares about the in-process subframes.
  • PresShell::SetIsActive looks fine per code comment.
  • PresShell::EndPaint looks fine, the remote iframe case would be handled when they getting painted.
  • PresShell::Freeze, the one called from nsDocumentViewer::Destroy probably need to handle the nested in-process case. Then we would need to do the same thing for PresShell::Thaw.

Edit: PresShell::Freeze/Thaw should be good per comment 8.

I should have put bug 1592895 which is the layout part of auditing.

See Also: → 1592895

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

I should have put bug 1592895 which is the layout part of auditing.

Thanks for this information, the layout part should be good. :)

See Also: → 1656105

I think the usages were already audited in bug 1592895 and bug 1656105.
One last thing we need to check is https://searchfox.org/mozilla-central/rev/a166f59fba89fc70ebfab287f4edb8e05ed4f6da/dom/base/Document.cpp#12419-12448 to make sure that the nested in-process case works, e.g. a.com -iframe> b.com -iframe> a.com.

Depends on: 1730117

(In reply to Edgar Chen [:edgar] from comment #10)

I think the usages were already audited in bug 1592895 and bug 1656105.
One last thing we need to check is https://searchfox.org/mozilla-central/rev/a166f59fba89fc70ebfab287f4edb8e05ed4f6da/dom/base/Document.cpp#12419-12448 to make sure that the nested in-process case works, e.g. a.com -iframe> b.com -iframe> a.com.

bug 1730117 is landed and stuck, I think we could close this now.

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