nsContentUtils::WidgetForDocument from Background Script returns non-null widget
Categories
(Core :: Layout, defect)
Tracking
()
People
(Reporter: jgilbert, Unassigned)
References
Details
It seems like this is causing bug 1679671, where WebGL expects to get LayersBackend::NONE for windowless (therefore widget-less?) documents like for web-ext Background Scripts:
https://searchfox.org/mozilla-central/rev/1843375acbbca68127713e402be222350ac99301/dom/html/HTMLCanvasElement.cpp#1273
WidgetForDocument calls nsContentUtils::FindPresShellForDocument, which early on sees that doc->GetDisplayDocument() returns null, but it doesn't return null for the PresShell, instead keeping going and calling doc->GetPresShell(), which returns non-null, and is then returned by FindPresShellForDocument.
There are a couple callers for both these functions, so it feels risky to make them suddenly return null.
NI smaug and masayuki, since y'all touched it last. :)
NB: I'm going to try changing HTMLCanvasElement::GetCompositorBackendType() to check for GetDisplayDocument being null, which I think is the case I want. Let me know if that's totally wrong!
Reporter | ||
Updated•4 years ago
|
Reporter | ||
Comment 1•4 years ago
|
||
Actually, reading some documentation comments it looks like "Display Document" is some other thing.
What should I use for "Is this document attached to a window?"
Comment 2•4 years ago
|
||
I'm not familiar with it. I just factored there to getting rid of nsIPresShell
. And yeah, as you say, it's risky to change it. Perhaps, PresShell
should have a method to check whether it's windowless document or not, and WidgetForDocument()
should take additional argument whether to check it or not.
Comment 3•4 years ago
|
||
If you need to know if document is currently attached to a window, check whether you can get inner window from it? Or check if you can get docshell from it?
But what are you actually trying to do.
https://searchfox.org/mozilla-central/rev/1843375acbbca68127713e402be222350ac99301/dom/html/HTMLCanvasElement.cpp#1273 seems to
be checking if owner document has a relevant widget. It doesn't check if canvas element itself is in document.
Should that use GetComposedDoc(), not OwnerDoc()? (note, GetComposedDoc() may return null, so some null checks might be needed.)
Reporter | ||
Comment 4•4 years ago
|
||
What I'm wanting is a way to know that the (background script) document doesn't have an actual present-to-screen object, but I'm not sure what the local noun for that is. (Is this a Window? It sounds like it's not a Widget?)
The core issue here is we want to make different choices for webgl context creation depending on whether the element will ever be composited.
Reporter | ||
Comment 5•4 years ago
|
||
(We still need to assume that even if a Canvas isn't in a Document, but might later be added to one, that we treat it like it may eventually be composited)
Comment 6•4 years ago
|
||
What is background script?
But if a document doesn't have access to docshell, it isn't in any visible tab.
And an element which isn't in a composed Doc isn't visible ever, even if the document itself is visible.
Or if you care about display:none case, if the element doesn't have a primary frame.
Reporter | ||
Comment 7•4 years ago
|
||
Background Scripts are a WebExtensions thing: https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/background
It has a document, but (it sounds like) it shouldn't have a docshell, but (iirc from stepping through it) WidgetForDocument seems to ignore the case where there's a null docshell. It definitely has a Document, and definitely is never presented to any screen or window. There may or may not be a window
global, I'm not sure, but it's never shown in any OS window, so it probably shouldn't have a Widget. (but it gets one via WidgetForDocument)
It sounds like the issue here is that WidgetForDocument is ignoring the case where there's a null docshell, and that'd be where our bug here is. (again, if I'm remember this right from my step-through)
Description
•