Example failure: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8a2410d7603104bf634f29939d63348f600479b2&selectedJob=202959354. From the stack, I think what's happening is: - a call to CustomElementRegistry::CallGetCustomInterface during mozilla::PresShell::UpdateCanvasBackground causes the CE backup queue to run - This causes an unrelated connectedCallback to run that appends a child (<dropmarker> in this case) - Doing some logging, the interface number causing this is (c67ed254-fd3b-4b10-96a2-c58b7b6497d1) which is NS_ELEMENT_IID. I've confirmed in a try push that by explicitly skipping that IID the problem goes away: https://hg.mozilla.org/try/rev/780065fadc9b573b36d12173a2bb57f244a4e883. Since calling CustomElementRegistry::CallGetCustomInterface seems to have side effects like CustomElementReactionsStack::InvokeBackupQueue, I guess we should be limiting when we make that call. I'm thinking either: 1) Skip any interface that can't be implemented in JS 2) Make a whitelist of interfaces that we expect to implement with CE and only make the call for them (see the list at https://bgrins.github.io/xbl-analysis/tree/#implements) 3) Store the list of IIDs that a Custom Element implements directly in the registry. Right now getCustomInterfaceCallback checks the ID but always returns the same proxy regardless of ID: https://searchfox.org/mozilla-central/rev/a11c128b90ea85d7bb68f00c5de52c0794e0b215/toolkit/content/customElements.js#132. We know the list of IDs before doing customElements.define(), so it seems like we could look up if an element implements it from C++ and only call getCustomInterfaceCallback when it matches.

Any opinion on our options here? And, is there an easy way to check for (1) given an interface ID?

FWIW, that QI is completely useless: https://searchfox.org/mozilla-central/rev/a11c128b90ea85d7bb68f00c5de52c0794e0b215/layout/base/PresShell.cpp#5330

(But this is still bad)

Here are the interfaces that we currently implement from XBL: 'nsIObserver', 'nsIWeakReference', 'nsIDOMXULSelectControlItemElement', 'nsIAutoCompleteInput', 'nsIDOMXULMenuListElement', 'nsIAutoCompletePopup', 'nsIBrowser', 'nsIDOMXULButtonElement', 'nsIDateTimeInputArea', 'nsIDOMXULControlElement', 'nsIDOMXULContainerItemElement', 'nsIDOMXULContainerElement', 'nsIDOMXULMultiSelectControlElement', 'nsITimerCallback', 'nsIDOMXULSelectControlElement', 'nsIDOMXULRelatedElement', We should be able to safely skip anything else (we aren't planning to add new ones).

Yeah, but doing a microtask checkpoint from QI looks still bad, I think we QI to some of those from layout and such, and we don't want to run random microtask at that point.

(In reply to Emilio Cobos Álvarez (:emilio) from comment #5) > Yeah, but doing a microtask checkpoint from QI looks still bad, I think we > QI to some of those from layout and such, and we don't want to run random > microtask at that point. IIUC then I don't think even option (3) would solve this issue since that would still end up calling into JS when an element did in fact match the list of known IDs. What are our options in that case? I guess if we could somehow store the interface proxy [0] on the element itself instead of as a function on the CE registry then combined with option (3) we could avoid calling into JS when doing Qi. [0]: https://searchfox.org/mozilla-central/rev/a11c128b90ea85d7bb68f00c5de52c0794e0b215/toolkit/content/customElements.js#147

Is the microtask checkpoint just a consequence of calling into JS or is there something in particular that CustomElementRegistry::CallGetCustomInterface (https://searchfox.org/mozilla-central/source/dom/base/CustomElementRegistry.cpp#1275) does that could be changed to not cause the checkpoint?

> And, is there an easy way to check for (1) given an interface ID? Should be able to by asking for interface info from xpconnect. > And, is there an easy way to check for (1) given an interface ID? That's been the case all along for things with a scripted QI, right? Or does xpconnect not do microtask checkpoints? Anyway, the long-term solution here should be bug 1492326: we should stop hacking QI. But if people actually want these interfaces, we would still be invoking the relevant getters... > Is the microtask checkpoint just a consequence of calling into JS Pretty much, yes. Web IDL callbacks do a microtask checkpoint when returning, because that's what the spec says to do...

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #8) > Anyway, the long-term solution here should be bug 1492326: we should stop > hacking QI. But if people actually want these interfaces, we would still > be invoking the relevant getters... Right, if we actually want to use the interface we'll eventually need to call into JS. With XBL, does it wait to call any JS until you reference a property on the interface, or does it do it immediately as a result of a matching Qi? Because I think my option (3) in Comment 0 and would be approximately the latter. It's also not clear to me which of these states Bug 1492326 would leave us in. Neil, what are you thinking here? Should we go straight to Bug 1492326, do one of the options outlined above seem like a safe place to start, or do you have other ideas? I'd like to get to a consensus on what to do since this is blocking landing the de-XBLed <browser> implementation and also seems like this could bite us in unexpected ways for other Custom Element migrations that use `implementCustomInterface`.

I think Boris answered :)

> does it wait to call any JS until you reference a property on the interface It's complicated (surprise!). With XBL, the set of interfaces implemented is declarative. nsXBLPrototypeBinding maps those strings to IIDs and stores the list of IIDs. Then nsBindingManager::GetBindingImplementation queries that list (all in C++) and if the given IID is in the list then it depends on whether there's an existing nsXPCWrappedJS for the object. If not, we just create one; no calls to JS. If there is one already, we call QI on it. This can in general call into JS, _if_ the bound element has a "QueryInterface" property whose value is callable. If not, it definitely won't call into JS. This is the common case, I would think (possibly the only case) since XBL implements="" typically is not aiming to use a separate object as the implementation. If it does call into JS, it looks at first glance like it would _not_ perform a microtask checkpoint as part of that call. Calls through XPConnect into JS generally don't, apparently, because they are trying to hide whether the interface is implemented via JS or C++. I guess one thing we could try to do is add a way to call a WebIDL callback without triggering microtask stuff.... > It's also not clear to me which of these states Bug 1492326 would leave us in. Bug 1492326 would more or less leave us in your state 3 (or _maybe_ 2). QI would not be affected, but someone who actually wanted that JS-implemented thing would incur a call into JS to get it.

I've already started to continue the work I had started on bug 1492326 as perhaps the simplest approach in the long run. Otherwise, I had also suggested just implementing implementCustomInterface in C++.

I think this was fixed by Bug 1492326