1312046 - Crash in GetProxiedAccessibleInSubtree

Status: VERIFIED FIXED

(Core :: Disability Access APIs, defect)

Description

[(No longer employed by Mozilla) Aaron Klotz]

This bug was filed from the Socorro interface and is 
report bp-4885c328-2dc2-4ca1-ab44-e2cc22161021.
=============================================================

Comment 1

[Alice0775 White]

I can reproduce the crash on Windows10 Home 64bit, Nightly52.0a1 32bit.

Reproducible: 100%

Steps To Reproduce:
1. Open attachment 8806341 [details] of Bug 1312697
2. Scroll by clicking of "M"
3. Reload (F5) several times

Actual Results:
Browser Crashes

Comment 2

[Marco Zehe (:MarcoZ)]

Note that I was unable to reproduce with these steps using the 64 bit version of current 52.0a1 Nightly with NVDA running. I reloaded about ten times or so, but waited for each load to complete, including the screen reader digesting this huge document. No cash here.

Comment 3

[Alice0775 White]

(In reply to Alice0775 White from comment #1)
> I can reproduce the crash on Windows10 Home 64bit, Nightly52.0a1 32bit.
> 
> Reproducible: 100%
> 
> Steps To Reproduce:
> 1. Open attachment 8806341 [details] of Bug 1312697
> 2. Scroll by clicking of "M"
> 3. Reload (F5) several times
> 
> Actual Results:
> Browser Crashes

I filed a new Bug 1314310

Comment 4

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Add null pointer check

The circumstances around this are unclear; my best guess is that something is enumerating child accessibles between proxy construction and setting of the com proxy.

The Real Fix is probably adding the setting of the com proxy to the PDocAccessibleConstructor, but I am still thinking about how I want to do that. This will get rid of the crashes at least.

Comment 5

[Trevor Saunders (:tbsaunde)]

Comment on attachment 8806521 [details] [diff] [review]
Add null pointer check

>   wrapper->GetNativeInterface(getter_AddRefs(comProxy));
>+  if (!comProxy) {
>+    return nullptr;

if this crash is bad enough we need to do this can we at least add an assert in the hope it'll help us find the cause if its not what you suspect or if it is that, maybe let us remove this later.

Comment 6

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Add null pointer check (r2)

Revised with assertion

Comment 7

[(No longer employed by Mozilla) Aaron Klotz]

https://hg.mozilla.org/integration/mozilla-inbound/rev/feebdb353fde62eb98e85f59d8a6e73816363b86
Bug 1312046: Add check for null COM proxy to GetProxiedAccessibleInSubtree; r=tbsaunde

Comment 8

[Phil Ringnalda (:philor)]

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

Comment 10

[[:philipp]]

hi, the signature is present in 51.0b too - would it make sense to uplift the patch?
http://bit.ly/2je22KS

Comment 11

[(No longer employed by Mozilla) Aaron Klotz]

Any presence of that signature in 51 is due to people who have force-enabled e10s. I am not sure that it is worth the risk to do a late-beta uplift for an unsupported configuration.

Comment 12

[[:philipp]]

thanks for taking a look!

Comment 13

[u554753]

Verified with the following user agent:
 Mozilla/5.0 (Windows NT 10.0; WOW64; rv:55.0) Gecko/20100101 Firefox/55.0