1852721 - Lots of gtk warnings with a11y enabled

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Emilio Cobos Álvarez [:emilio]]

(firefox-nightly:76895): GLib-GObject-CRITICAL **: 10:20:06.179: invalid (NULL) pointer instance
[Parent 76895, Main Thread] WARNING: g_signal_emit_by_name: assertion 'G_TYPE_CHECK_INSTANCE (instance)' failed: 'glib warning', file /builds/worker/checkouts/gecko/toolkit/xre/nsSigHandlers.cpp:167

(firefox-nightly:76895): GLib-GObject-CRITICAL **: 10:20:06.179: g_signal_emit_by_name: assertion 'G_TYPE_CHECK_INSTANCE (instance)' failed
[Parent 76895, Main Thread] WARNING: invalid (NULL) pointer instance: 'glib warning', file /builds/worker/checkouts/gecko/toolkit/xre/nsSigHandlers.cpp:167

(firefox-nightly:76895): GLib-GObject-CRITICAL **: 10:20:06.179: invalid (NULL) pointer instance
[Parent 76895, Main Thread] WARNING: g_signal_emit_by_name: assertion 'G_TYPE_CHECK_INSTANCE (instance)' failed: 'glib warning', file /builds/worker/checkouts/gecko/toolkit/xre/nsSigHandlers.cpp:167

(firefox-nightly:76895): GLib-GObject-CRITICAL **: 10:20:06.179: g_signal_emit_by_name: assertion 'G_TYPE_CHECK_INSTANCE (instance)' failed

Comment 1

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1852721 - Null-check parent before emitting signal. r=jamie,eeejay

This avoids warning spam on GTK with a11y enabled.

Comment 2

[Emilio Cobos Álvarez [:emilio]]

Attachment: Backtrace

Comment 3

[Emilio Cobos Álvarez [:emilio]]

event in #31 is (mozilla::a11y::AccHideEvent *) 0x7f6fc34fe510. The accessible is for the back button it seems:

(rr) p accessible->mContent->Dump()
XUL* toolbarbutton@0x7f6fd406d3a0 id="back-button" class="toolbarbutton-1 chromeclass-toolbar-additional" data-l10n-id="toolbar-button-back-2" removable="false" overflows="false" keepbroadcastattributeswhencustomizing="true" command="Browser:BackOrBackDuplicate" onclick="checkForMiddleClick(this, event);" tooltip="back-button-tooltip" context="backForwardMenu" disabled="true" oncommand="BrowserBack(event);" label="Back" type="menu" state=[100820000] flags=[00082002] selectorflags=[00000000] primaryframe=0x7f6fcb819470 refcount=17<
  XUL* menupopup@0x7f6fd4074520 onpopupshowing="return FillHistoryMenu(event.target);" oncommand="gotoHistoryIndex(event); event.stopPropagation();" context="" state=[820000] flags=[00080002] selectorflags=[00000000] primaryframe=0x7f6fcba05270 refcount=11<>
  XUL* image@0x7f6fd4089af0 class="toolbarbutton-icon" label="Back" type="menu" state=[800000] flags=[00000000] selectorflags=[00000000] primaryframe=0x7f6fcbafceb8 refcount=5<>
  XUL* label@0x7f6fd40891f0 class="toolbarbutton-text" crop="end" flex="1" value="Back" state=[820000] flags=[00000002] selectorflags=[00000000] primaryframe=(nil) refcount=3<>

So we hit this with target being that, but target->LocalParent() being null, and aParent being null ends up in here which just emits a signal to a null AtkObject.

target->mParent is nulled out here:

0  mozilla::a11y::LocalAccessible::UnbindFromParent() (this=this@entry=0x7f6fcbcd6f60) at /home/emilio/src/moz/gecko-5/accessible/generic/LocalAccessible.cpp:2610
#1  0x00007f6fe62fda40 in mozilla::a11y::LocalAccessible::RemoveChild(mozilla::a11y::LocalAccessible*) (this=<optimized out>, aChild=0x7f6fcbcd6f60) at /home/emilio/src/moz/gecko-5/accessible/generic/LocalAccessible.cpp:2709
#2  0x00007f6fe62f25d0 in mozilla::a11y::DocAccessible::ContentRemoved(mozilla::a11y::LocalAccessible*) (this=this@entry=0x7f6fcb813c00, aChild=0x800) at /home/emilio/src/moz/gecko-5/accessible/generic/DocAccessible.cpp:2158
#3  0x00007f6fe62eee13 in mozilla::a11y::DocAccessible::ContentRemoved(nsIContent*) (this=this@entry=0x7f6fcb813c00, aContentNode=aContentNode@entry=0x7f6fd406d3a0)
    at /home/emilio/src/moz/gecko-5/accessible/generic/DocAccessible.cpp:2167
#4  0x00007f6fe62f27f3 in mozilla::a11y::DocAccessible::RecreateAccessible(nsIContent*) (this=0x7f6fcb813c00, aContent=0x7f6fd406d3a0) at /home/emilio/src/moz/gecko-5/accessible/generic/DocAccessible.cpp:1420
#5  0x00007f6fe62eeccc in mozilla::a11y::DocAccessible::UpdateAccessibleOnAttrChange(mozilla::dom::Element*, nsAtom*)
    (this=this@entry=0x7f6fcb813c00, aElement=aElement@entry=0x7f6fd406d3a0, aAttribute=aAttribute@entry=0x7f6fdee1ea40 <mozilla::detail::gGkAtoms+75936>) at /home/emilio/src/moz/gecko-5/accessible/generic/DocAccessible.cpp:1798
#6  0x00007f6fe62ee81f in mozilla::a11y::DocAccessible::AttributeChanged(mozilla::dom::Element*, int, nsAtom*, int, nsAttrValue const*)
    (this=0x7f6fcb813c00, aElement=0x7f6fd406d3a0, aNameSpaceID=0, aAttribute=0x7f6fdee1ea40 <mozilla::detail::gGkAtoms+75936>, aModType=2, aOldValue=0x7ffcfe6180b0)
    at /home/emilio/src/moz/gecko-5/accessible/generic/DocAccessible.cpp:768
#7  0x00007f6fe291f4ca in mozilla::dom::MutationObservers::NotifyAttributeChanged(mozilla::dom::Element*, int, nsAtom*, int, nsAttrValue const*)::$_0::operator()(nsIMutationObserver*) const (aObserver=0x0, this=<optimized out>)
    at /home/emilio/src/moz/gecko-5/dom/base/MutationObservers.cpp:149
#8  ForEachAncestorObserver<mozilla::dom::MutationObservers::NotifyAttributeChanged(mozilla::dom::Element*, int, nsAtom*, int, nsAttrValue const*)::$_0>(nsINode*, mozilla::dom::MutationObservers::NotifyAttributeChanged(mozilla::dom::Element*, int, nsAtom*, int, nsAttrValue const*)::$_0&, unsigned int) (aNode=0x7f6fd406d3a0, aCallback=8, aFunc=<optimized out>) at /home/emilio/src/moz/gecko-5/dom/base/MutationObservers.cpp:60
#9  Notify<(NotifyPresShell)2, mozilla::dom::MutationObservers::NotifyAttributeChanged(mozilla::dom::Element*, int, nsAtom*, int, nsAttrValue const*)::$_0>(nsINode*, mozilla::dom::MutationObservers::NotifyAttributeChanged(mozilla::dom::Element*, int, nsAtom*, int, nsAttrValue const*)::$_0&&, unsigned int) (aNode=0x7f6fd406d3a0, aCallback=8, aNotify=<optimized out>) at /home/emilio/src/moz/gecko-5/dom/base/MutationObservers.cpp:94
#10 mozilla::dom::MutationObservers::NotifyAttributeChanged(mozilla::dom::Element*, int, nsAtom*, int, nsAttrValue const*)
    (aElement=0x7f6fd406d3a0, aNameSpaceID=0, aAttribute=0x7f6fdee1ea40 <mozilla::detail::gGkAtoms+75936>, aModType=2, aOldValue=0x7ffcfe6180b0) at /home/emilio/src/moz/gecko-5/dom/base/MutationObservers.cpp:148
#11 0x00007f6fe28c4432 in mozilla::dom::Element::SetAttrAndNotify(int, nsAtom*, nsAtom*, nsAttrValue const*, nsAttrValue&, nsIPrincipal*, unsigned char, bool, bool, bool, mozilla::dom::Document*, mozAutoDocUpdate const&)
    (this=this@entry=0x7f6fd406d3a0, aNamespaceID=aNamespaceID@entry=0, aName=0x7f6fdee1ea40 <mozilla::detail::gGkAtoms+75936>, aPrefix=aPrefix@entry=0x0, aOldValue=<optimized out>, aParsedValue=..., aSubjectPrincipal=0x0, aModType=2 '\002', aFireMutation=false, aNotify=<optimized out>, aCallAfterSetAttr=<optimized out>, aComposedDocument=0x7f6fdb473300, aGuard=<optimized out>) at /home/emilio/src/moz/gecko-5/dom/base/Element.cpp:2657
#12 0x00007f6fe28c1006 in mozilla::dom::Element::SetAttr(int, nsAtom*, nsAtom*, nsTSubstring<char16_t> const&, nsIPrincipal*, bool)
    (this=this@entry=0x7f6fd406d3a0, aNamespaceID=aNamespaceID@entry=0, aName=aName@entry=0x7f6fdee1ea40 <mozilla::detail::gGkAtoms+75936>, aPrefix=aPrefix@entry=0x0, aValue=u"menu", aSubjectPrincipal=aSubjectPrincipal@entry=0x0, aNotify=<optimized out>) at /home/emilio/src/moz/gecko-5/dom/base/Element.cpp:2500
#13 0x00007f6fe28c0d07 in mozilla::dom::Element::SetAttr(int, nsAtom*, nsTSubstring<char16_t> const&, nsIPrincipal*, bool)
    (this=0x7f6fd406d3a0, aNameSpaceID=0, aName=0x7f6fdee1ea40 <mozilla::detail::gGkAtoms+75936>, aValue=u"menu", aTriggeringPrincipal=0x0, aNotify=true) at /home/emilio/src/moz/gecko-5/obj-debug/dist/include/mozilla/dom/Element.h:953
#14 mozilla::dom::Element::SetAttribute(nsTSubstring<char16_t> const&, nsTSubstring<char16_t> const&, nsIPrincipal*, mozilla::ErrorResult&)
    (this=0x7f6fd406d3a0, aName=<optimized out>, aValue=u"menu", aTriggeringPrincipal=0x0, aError=...) at /home/emilio/src/moz/gecko-5/dom/base/Element.cpp:1497
#15 0x00007f6fe3627742 in mozilla::dom::Element_Binding::setAttribute(JSContext*, JS::Handle<JSObject*>, void*, JSJitMethodCallArgs const&) (cx=<optimized out>, obj=Traceback (most recent call last):
  File "/home/emilio/src/moz/gecko-5/js/src/gdb/mozilla/Root.py", line 55, in to_strin

On this JS stack:

(rr) call DumpJSStack()
0 SetClickAndHoldHandlers() ["chrome://browser/content/browser.js":861:13]
    this = [object Window]
1 _delayedStartup("[object NotifyPaintEvent]") ["chrome://browser/content/browser.js":1883:6]
    this = [object Object]

So that all looks rather reasonable. Do you know what's supposed to happen?

Comment 4

[James Teh [:Jamie]]

Oh! Of course, it's a hide event, not a show event. That makes more sense.

This is problematic because ATK needs a parent for hide events, but we've already unbound from the parent by the time we fire coalesced events. I'm... not really sure what we can do about that. Keeping the parent around seems like a big change and might not even be feasible.

I do notice that ATK seems to have an accessible_parent member which we use as a cache. Can we perhaps rely on that somehow? That's not guaranteed to be set, but maybe we can make sure it is in this case somehow?

Comment 7

[Emilio Cobos Álvarez [:emilio]]

Can we take the patch for now even if it's a wallpaper? I don't have a lot of time to investigate comment 4, but people keep getting confused about these warnings because they look crashy (even though they are not).

Comment 8

[dbiro97]

Emilio, they are indeed warnings. However, it is heavily dumped. I had to direct the logs to /dev/null, because firefox ate my notebook otherwise.
But it's true, firefox works functionally, it doesn't crash and the fix was easy by force turning off the accessibility feature.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/173608723df3
Null-check parent before emitting signal. r=Jamie

Comment 12

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/173608723df3