1646516 - [meta] Audit uses of nsContentUtils::GetRootDocument

Status: RESOLVED FIXED

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

Description

[Kris Maglione [:kmag]]

+++ This bug was initially created as a clone of Bug #1646510 +++

Similar to uses of nsIDocShellTreeItem, many/most of the callers of these methods will probably do the wrong thing under Fission with remote frames.

Comment 1

[Neha Kochar [:neha]]

Don't know how this missed our triage till now :(

Comment 2

[Neha Kochar [:neha]]

Steven, can you please add a comment to GetRootDocument() definition to mention this shouldn't be used in new code, and also audit the remaining callers of this and add comments to this bug about which ones need to be fixed and which ones are okay and not a problem with Fission?

Comment 3

[Chris Peterson [:cpeterson]]

Most of the callers are related to fullscreen, which Alphan already audited in bug 1646491. For the fullscreen code calling GetRootDocument, you may be able to just copy/paste his bug comments.

Comment 4

[Neha Kochar [:neha]]

Edgar, can you audit and confirm, please?

Comment 5

[Edgar Chen [:edgar]]

I will take a look.

Comment 6

[Edgar Chen [:edgar]]

Most of the callers are related to fullscreen, which has been checked in bug 1646491.
Others are,

1. https://searchfox.org/mozilla-central/rev/54097530955a98c768f2aaf56925578ec886ec77/dom/base/DOMIntersectionObserver.cpp#452,460
2. https://searchfox.org/mozilla-central/rev/54097530955a98c768f2aaf56925578ec886ec77/layout/base/GeometryUtils.cpp#330
3. https://searchfox.org/mozilla-central/rev/54097530955a98c768f2aaf56925578ec886ec77/layout/base/PresShell.cpp#8415

1. and 2) looks fine since they do expect nsContentUtils::GetRootDocument doesn't cross the process boundary.

I am checking 3) and will file a separated bug if needed.

Comment 7

[Edgar Chen [:edgar]]

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

3. https://searchfox.org/mozilla-central/rev/54097530955a98c768f2aaf56925578ec886ec77/layout/base/PresShell.cpp#8415

3. is for ESC key handling for fullscreen that works fine with Fission.

Comment 8

[Edgar Chen [:edgar]]

I have audited the all of the usage, and renamed GetRootDocument to GetInProcessSubtreeRootDocument in bug 1709200 to reduce confusion. So close as FIXED.