1727183 - [Fission] Audit callers of Document::EnumerateSubDocuments

Status: RESOLVED FIXED

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

Description

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

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

Comment 1

[Edgar Chen [:edgar]]

I will take a look at this.

Comment 2

[Chris Peterson [:cpeterson]]

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

Comment 3

[Edgar Chen [:edgar]]

The usage in AutoPrintEventDispatcher::CollectInProcessSubdocuments looks fine, the remote iframe would be handled in BrowserChild::RecvCloneDocumentTreeIntoSelf.

Comment 4

[Edgar Chen [:edgar]]

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

Comment 5

[Edgar Chen [:edgar]]

The usage for fullscreen Looks fine.

• CountFullscreenSubDocuments
• GetFullscreenLeaf
• ResetFullscreen

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

Comment 6

[Edgar Chen [:edgar]]

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

Comment 7

[Edgar Chen [:edgar]]

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.

Comment 8

[Hiroyuki Ikezoe (:hiro)]

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

Comment 9

[Edgar Chen [:edgar]]

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

Comment 10

[Edgar Chen [:edgar]]

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.

Comment 11

[Edgar Chen [:edgar]]

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