Fix crash when system retains an accessibility object.

Fix crash when system retains an accessibility object.

This is a follow-up to r207709.

BUG=242944
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=233506

[falken, 2013-11-05 05:32:27]

Dominic, could you please review? Looks like it should just have been part of
https://codereview.chromium.org/17261008

[dmazzoni, 2013-11-05 16:44:32]

Did you hit this crash, do you have a stack trace or crash id?

https://codereview.chromium.org/58993002/diff/1/content/browser/accessibility...
File content/browser/accessibility/browser_accessibility_cocoa.mm (right):

https://codereview.chromium.org/58993002/diff/1/content/browser/accessibility...
content/browser/accessibility/browser_accessibility_cocoa.mm:1424: if (!child)
This doesn't look right, I don't see how |child| should ever be null.

The issue fixed by the previous change was that BrowserAccessibilityCocoa's
browserAccessibility_ pointer becomes NULL - I don't see how we should ever have
a pointer to a NULL child...

https://codereview.chromium.org/58993002/diff/1/content/browser/accessibility...
content/browser/accessibility/browser_accessibility_cocoa.mm:1434: if
(![childResult accessibilityIsIgnored]) {
I think childResult could be nil here. Is that the actual bug?

[falken, 2013-11-05 16:53:01]

On 2013/11/05 16:44:32, Dominic Mazzoni wrote:
> Did you hit this crash, do you have a stack trace or crash id?

Yes I commented on the bug. Looks like a child is null. I can post more crash
ids when back at my workstation.

> 
>
https://codereview.chromium.org/58993002/diff/1/content/browser/accessibility...
> File content/browser/accessibility/browser_accessibility_cocoa.mm (right):
> 
>
https://codereview.chromium.org/58993002/diff/1/content/browser/accessibility...
> content/browser/accessibility/browser_accessibility_cocoa.mm:1424: if (!child)
> This doesn't look right, I don't see how |child| should ever be null.
> 
> The issue fixed by the previous change was that BrowserAccessibilityCocoa's
> browserAccessibility_ pointer becomes NULL - I don't see how we should ever
have
> a pointer to a NULL child...
> 
>
https://codereview.chromium.org/58993002/diff/1/content/browser/accessibility...
> content/browser/accessibility/browser_accessibility_cocoa.mm:1434: if
> (![childResult accessibilityIsIgnored]) {
> I think childResult could be nil here. Is that the actual bug?

[dmazzoni, 2013-11-05 16:58:40]

On 2013/11/05 16:53:01, falken wrote:
> On 2013/11/05 16:44:32, Dominic Mazzoni wrote:
> > Did you hit this crash, do you have a stack trace or crash id?
> 
> Yes I commented on the bug. Looks like a child is null. I can post more crash
> ids when back at my workstation.

Ah, I see the stack trace now.

The bug isn't that |child| is null, it's that the child's browserAccessibility_
is null, so that's why origin() crashes.

You could fix it in origin or in hitTest.

[falken, 2013-11-06 03:14:06]

On 2013/11/05 16:58:40, Dominic Mazzoni wrote:
> On 2013/11/05 16:53:01, falken wrote:
> > On 2013/11/05 16:44:32, Dominic Mazzoni wrote:
> > > Did you hit this crash, do you have a stack trace or crash id?
> > 
> > Yes I commented on the bug. Looks like a child is null. I can post more
crash
> > ids when back at my workstation.
> 
> Ah, I see the stack trace now.
> 
> The bug isn't that |child| is null, it's that the child's
browserAccessibility_
> is null, so that's why origin() crashes.
> 
> You could fix it in origin or in hitTest.

Ah I see. How's this patch?

BTW, here are some more crash ids:
(canary)
43f9c71d2e637dff
b15905ec635c674a
(stable)
628e47cbd31db2c5
e652e34e66322404
d7cf09c04b27c41f

[dmazzoni, 2013-11-06 06:41:45]

lgtm

[commit-bot: I haz the power, 2013-11-06 06:46:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/falken@chromium.org/58993002/110001

[commit-bot: I haz the power, 2013-11-06 10:28:31]

Retried try job too often on linux_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[commit-bot: I haz the power, 2013-11-06 12:27:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/falken@chromium.org/58993002/110001

[commit-bot: I haz the power, 2013-11-06 15:48:13]

Retried try job too often on linux_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[commit-bot: I haz the power, 2013-11-07 02:25:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/falken@chromium.org/58993002/110001

[commit-bot: I haz the power, 2013-11-07 06:37:50]

Change committed as 233506