1656105 - [meta] Audit uses of Document::EnumerateSubDocuments

Status: RESOLVED FIXED

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

Description

[Hiroyuki Ikezoe (:hiro)]

Document::EnumerateSubdocuments enumerates all subdocument in the same process, it won't work across processes at all.

If we need to something in cross process in the call site, we probably do the stuff via PBrowser IPC call(s).

Comment 1

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

Olli, can you please audit this? Thanks!

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Part of the call sites in layout stuff has been already done in bug 1592895.

Comment 3

[Nika Layzell [:nika] (ni? for response)]

I'm not sure how much benefit this brings us anymore, as the remaining ones in layout should've been audited in bug 1592895, and there are few remaining uses in non-layout code, mostly around fullscreen (which should've been handled by bug 1597439) and suppressing document event loops (which is somewhat covered by bug 1640766). We may be able to close this bug out now?

Comment 4

[Steven MacLeod [:smacleod]]

There was just a single case left that I was unsure about: https://searchfox.org/mozilla-central/rev/644e42ded761d4f3ce108fa776197730a9a2c535/dom/base/Document.cpp#12792. This is used by nsIDOMWindowUtils for plugins: https://searchfox.org/mozilla-central/rev/644e42ded761d4f3ce108fa776197730a9a2c535/dom/interfaces/base/nsIDOMWindowUtils.idl#1736

Bug 1505913 would indicate that this has been fixed and is okay as-is. I was unsure though when seeing uses and comments like https://searchfox.org/mozilla-central/rev/644e42ded761d4f3ce108fa776197730a9a2c535/browser/actors/PluginChild.jsm#832-833 which appears to try and collect the plugins for the whole page. Maybe this is handled at a higher level by calling this in multiple processes, but I figured I'd just ask before digging any deeper.

:Gijs, do you believe the work is complete for making uses of nsIDOMWindowUtils.plugins supported in Fission?

Comment 5

[:Gijs (he/him)]

I think all of the related code just wants ripping out because NPAPI plugins are no longer supported - bug 1677878. This code (ie the windowUtils.plugins API) is not needed for the EME/CDM/video-DRM provider type plugins. :handyman can confirm if this is ready to just be removed now (I think so?).

Comment 6

[David Parks [:handyman]]

Indeed, I have removed plugins from nsIDOMWindowUtils.idl (and the GetPlugins method from Document). See bug 1682030.

Note that the Plugin JSActors get greatly reduced in my patches. The only thing they still do is GMP crash reporting.

Comment 7

[Steven MacLeod [:smacleod]]

Thanks for weighing in. Calling this complete.