1705542 - Crash in [@ InvalidArrayIndex_CRASH | mozilla::a11y::RemoteAccessible::ChildAtPoint]

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Andrew McCreight [:mccr8]]

Maybe Fission related. (DOMFissionEnabled=1)

Crash report: https://crash-stats.mozilla.org/report/index/eeef70fe-a0a4-4722-aaf1-ffe5f0210409

MOZ_CRASH Reason: ElementAt(aIndex = 0, aLength = 0)

Top 10 frames of crashing thread:

0 XUL InvalidArrayIndex_CRASH xpcom/ds/nsTArray.cpp:28
1 XUL mozilla::a11y::RemoteAccessible::ChildAtPoint accessible/ipc/other/RemoteAccessible.cpp:817
2 XUL mozilla::a11y::OuterDocAccessible::ChildAtPoint accessible/generic/OuterDocAccessible.cpp:272
3 libobjc.A.dylib cache_getImp 
4 XUL mozilla::a11y::AccessibleOrProxy::ChildAtPoint accessible/base/AccessibleOrProxy.cpp:59
5 XUL -[mozAccessible moxHitTest:] accessible/mac/mozAccessible.mm:287
6 XUL -[MOXAccessibleBase accessibilityHitTest:] accessible/mac/MOXAccessibleBase.mm:377
7 AppKit -[NSWindow accessibilityHitTest:] 
8 AppKit -[NSApplication accessibilityHitTest:] 
9 AppKit _NSAccessibilityGetScreenHeightForElement 

Only a couple of these crashes, but they all have index and length of 0, so I guess some code is trying to access an empty array.

Comment 1

[James Teh [:Jamie]]

mOuterDoc is true even though it has no child document. I guess this could happen if the embedded doc is changed and a client call comes in after the old doc is removed but before the new one is added.

The simplest fix would be to set mOuterDoc = false in ClearChildDoc. This feels kinda wrong, though; it's still an OuterDoc even if it doesn't currently have a document attached. Also, we probably eventually want to get rid of mOuterDoc and just use Accessible::IsOuterDoc (which uses mType).

The other fix is to change anything that checks mOuterDoc so that it doesn't assume there will be a child. Note that ChildAtPoint isn't the only method making this assumption.

Comment 2

[Chris Peterson [:cpeterson]]

5 out of 6 of these crash reports have Fission enabled. So this is not a Fission-specific crash, but maybe Fission is making this crash more likely?

Tracking for Fission M8 to see if the crash volume increases as we increase our Beta experiment.

Comment 3

[James Teh [:Jamie]]

Attachment: Bug 1705542: Don't assume an OuterDoc RemoteAccessible has a child document.

An OuterDoc RemoteAccessible can have no child document for a short time if the embedded doc is changed.
As part of this, get rid of the mOuterDoc variable, since it is now redundant and somewhat misleading.
Instead, use IsOuterDoc(), since RemoteAccessible now has acc types.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f99981eb9e49
Don't assume an OuterDoc RemoteAccessible has a child document. r=eeejay

Comment 5

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/f99981eb9e49

Comment 6

[Pascal Chevrel:pascalc (PTO until April 26)]

No crash in 89