1559663 - Crash in [@ nsFocusManager::ActivateRemoteFrameIfNeeded]

Status: RESOLVED WORKSFORME

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

Description

[Julien Cristau [:jcristau]]

This bug is for crash report bp-34c76059-c26b-42ca-bff5-8229e0190615.

Crashes starting in nightly buildid 20190613215335, with MOZ_RELEASE_ASSERT(mFocusedElement == &aElement), mostly on linux.

Top 10 frames of crashing thread:

0 libxul.so nsFocusManager::ActivateRemoteFrameIfNeeded dom/base/nsFocusManager.cpp:1736
1 libxul.so nsFocusManager::Focus dom/base/nsFocusManager.cpp:1891
2 libxul.so nsFocusManager::SetFocusInner dom/base/nsFocusManager.cpp:1316
3 libxul.so mozilla::dom::Element::Focus dom/base/Element.cpp:354
4 libxul.so mozilla::dom::XULElement_Binding::focus dom/bindings/XULElementBinding.cpp:9095
5 libxul.so bool mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions> dom/bindings/BindingUtils.cpp:3171
6 libxul.so js::InternalCallOrConstruct js/src/vm/Interpreter.cpp:540
7 libxul.so Interpret js/src/vm/Interpreter.cpp:595
8 libxul.so js::RunScript js/src/vm/Interpreter.cpp:425
9 libxul.so js::InternalCallOrConstruct js/src/vm/Interpreter.cpp:568

Comment 1

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

Though the crash volume is low at this moment, the assert was added in bug 1546019. Emilio, you might find it interesting to take a look. :)

Comment 2

[Emilio Cobos Álvarez (:emilio)]

So this is weird, since we set the focused element in here:

• https://searchfox.org/mozilla-central/rev/928742d3ea30e0eb4a8622d260041564d81a8468/dom/base/nsFocusManager.cpp#1866

And we then pass that to ActivateRemoteFrameIfNeeded. This means that something is stealing the focus, presumably something under ScrollIntoView, but if that happens probably that means we shouldn't be calling into ActivateRemoteFrameIfNeeded.

Jason, do you or anyone on your team have seen this assertion fire?

Overall I'm not really concerned about this since the behavior is the same as before my patch and the crash is a diagnostic assert which doesn't crash release, but...

Comment 3

[Jason Kratzer [:jkratzer]]

Emilio, I'm not seeing anything matching that in our logs but I'll add a watch for it in the event one turns up. Also, I'm on PTO this week but I'll look into adding something a bit more direct to try and reproduce this.

Comment 4

[Jason Kratzer [:jkratzer]]

Emilio - I finally had a chance to look into this and it seems that we've got full coverage of the code traversed in that stack. Our fuzzers still haven't found that crash but it doesn't appear that there's much more I can add that will cause it to trigger.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

Closing because no crashes reported for 12 weeks.