1646491 - Figure out if the usage of nsContentUtils::GetRootDocument in FullscreenRoots is OK

Status: RESOLVED FIXED

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

Description

[Kris Maglione [:kmag]]

It's possible we have some other code that makes things work when OOP frames are involved, but I'm not sure.

Comment 1

[Kris Maglione [:kmag]]

Note this also includes use of functions like GetRootDocument, which does the same thing indirectly.

Comment 2

[Neha Kochar [:neha]]

Henri, could you look into figuring this out for Fission-compatibility?

Comment 3

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

(hsivonen is on pto for couple weeks still, so I can look at this)

FullscreenChange doesn't have any GetInProcessParent usage. It is unclear to me what this bug is about.

Comment 4

[Kris Maglione [:kmag]]

(In reply to Olli Pettay [:smaug] from comment #3)

(hsivonen is on pto for couple weeks still, so I can look at this)

FullscreenChange doesn't have any GetInProcessParent usage. It is unclear to me what this bug is about.

I think the main issue is the use of nsContentUtils::GetRootDocument in the fullscreen code in Document.cpp, which uses GetInProcessParent indirectly, but there are also some direct usages: https://searchfox.org/mozilla-central/rev/82c04b9cad5b98bdf682bd477f2b1e3071b004ad/dom/base/Document.cpp#12995,13031

Comment 5

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Alphan has been looking into Fullscreen Fission issues. I believe we can start with Alphan. Thank you. :)

Comment 6

[Alphan Chen [:alchen]]

There are three parts for fullscreen code in Document.cpp.

I think the main issue is the use of nsContentUtils::GetRootDocument in the fullscreen code in Document.cpp, which uses GetInProcessParent indirectly,

1. The usage of nsContentUtils::GetRootDocument in FullscreenRoots.
   https://searchfox.org/mozilla-central/rev/03794edd6edcc3fc1e222de966cb27256ce08998/dom/base/nsContentUtils.cpp#6596
   https://searchfox.org/mozilla-central/rev/03794edd6edcc3fc1e222de966cb27256ce08998/dom/base/Document.cpp#12934,12960,13200,13826,13924

but there are also some direct usages:

2. The usage of GetInProcessRootTreeItem() and GetInProcessParent() in PendingFullscreenChangeList.
   https://searchfox.org/mozilla-central/rev/82c04b9cad5b98bdf682bd477f2b1e3071b004ad/dom/base/Document.cpp#12995,13031

3. Here is the another usage of GetInProcessRootTreeItem.
   https://searchfox.org/mozilla-central/rev/03794edd6edcc3fc1e222de966cb27256ce08998/dom/base/Document.cpp#13765,13854

I will handle 2 and 3 in Bug 1585074.

Comment 7

[Alphan Chen [:alchen]]

There are three parts for fullscreen code in Document.cpp.

I think the main issue is the use of nsContentUtils::GetRootDocument in the fullscreen code in Document.cpp, which uses GetInProcessParent indirectly,

1. The usage of nsContentUtils::GetRootDocument in FullscreenRoots.
   https://searchfox.org/mozilla-central/rev/03794edd6edcc3fc1e222de966cb27256ce08998/dom/base/nsContentUtils.cpp#6596
   https://searchfox.org/mozilla-central/rev/03794edd6edcc3fc1e222de966cb27256ce08998/dom/base/Document.cpp#12934,12960,13200,13826,13924

but there are also some direct usages:

2. The usage of GetInProcessRootTreeItem() and GetInProcessParent() in PendingFullscreenChangeList.
   https://searchfox.org/mozilla-central/rev/82c04b9cad5b98bdf682bd477f2b1e3071b004ad/dom/base/Document.cpp#12995,13031

3. Here is the another usage of GetInProcessRootTreeItem.
   https://searchfox.org/mozilla-central/rev/03794edd6edcc3fc1e222de966cb27256ce08998/dom/base/Document.cpp#13765,13854

I will handle 2 and 3 in Bug 1585074.

Comment 8

[Steven MacLeod [:smacleod]]

This is okay and working in fission due to Alphan's other changes. New tests have been added[1] to verify this and ensure things keep working.

[1] See Bug 1665941 and specifically D90940