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•5 years ago
|
|
|
Reporter | |
Comment 1•5 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•5 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•5 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•5 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•5 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•5 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•5 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
•