David Bolter [:davidb] (NeedInfo me for attention) Reporter
	Description • 9 years ago

This bug was filed from the Socorro interface and is 
report bp-107671ed-2b58-4875-92ba-12a2a2151008.
=============================================================

Some STR from crash report comments:
1) Go to messenger.com
2) Type in the name of a contact
3) Click that contact
4) Wait
5) Crash

and,

messenger.com contact search (left sidebar), typing a search then clicking a contact from the results (or simply hitting TAB) crashes the browser 

More reports here: http://is.gd/sAf15p

	Marco Zehe (:MarcoZ)
	Comment 1 • 9 years ago

Note that you need a Facebook account to be able to follow these steps.

	Brian Birtles (:birtles, Feb 26~ parental leave)
	Comment 2 • 9 years ago

I'm seeing this repeatedly on Aurora. Using the less restrictive signature it seems there have been 113 crashes in the last week at the time of writing:
https://crash-stats.mozilla.com/report/list?product=Firefox&signature=mozilla%3A%3Aa11y%3A%3ADocAccessible%3A%3AUpdateTreeOnInsertion

	alexander :surkov (:asurkov) Assignee
	Comment 4 • 9 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=40f59b488271

	alexander :surkov (:asurkov) Assignee
	Comment 5 • 9 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ae6eb66055bb

	David Bolter [:davidb] (NeedInfo me for attention) Reporter
	Comment 6 • 9 years ago

Comment on attachment 8691545 [details] [diff] [review]
patch

Review of attachment 8691545 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/generic/DocAccessible.cpp
@@ +2160,5 @@
>  void
>  DocAccessible::PutChildrenBack(nsTArray<RefPtr<Accessible> >* aChildren,
>                                 uint32_t aStartIdx)
>  {
> +  nsTArray<nsRefPtr<Accessible> > containers;

Better as "nsTArray<RefPtr<Accessible>" ;)

@@ +2197,5 @@
>  
>    // And put it back where it belongs to.
>    aChildren->RemoveElementsAt(aStartIdx, aChildren->Length() - aStartIdx);
>    for (uint32_t idx = 0; idx < containers.Length(); idx++) {
> +    if (containers[idx]->IsInDocument()) {

OK I guess? When does this fail?

	alexander :surkov (:asurkov) Assignee
	Comment 7 • 9 years ago

(In reply to David Bolter [:davidb] from comment #6)

> > +  nsTArray<nsRefPtr<Accessible> > containers;
> 
> Better as "nsTArray<RefPtr<Accessible>" ;)

yeah, try build says same

> @@ +2197,5 @@
> >  
> >    // And put it back where it belongs to.
> >    aChildren->RemoveElementsAt(aStartIdx, aChildren->Length() - aStartIdx);
> >    for (uint32_t idx = 0; idx < containers.Length(); idx++) {
> > +    if (containers[idx]->IsInDocument()) {
> 
> OK I guess? When does this fail?

I don't see a code path but it looks like that something destroys container accessibles before we process them. I'll add an assertion for this case.

	alexander :surkov (:asurkov) Assignee
	Comment 8 • 9 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=17d257c4d305

	alexander :surkov (:asurkov) Assignee
	Comment 9 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/cf2dbc87221d6005db2074a0e4782b8d4198e0ae
Bug 1213281 - crash in mozilla::a11y::DocAccessible::UpdateTreeOnInsertion, r=davidb

	Carsten Book [:Tomcat]
	Comment 10 • 9 years ago
bugherder

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

	Brian Birtles (:birtles, Feb 26~ parental leave)
	Comment 11 • 9 years ago

Alexander, do you mind requesting approval to land on Aurora?

Without this affected users are unable to use facebook messenger without crashing so I hope we don't ship to release.

	Marco Zehe (:MarcoZ)
	Comment 12 • 9 years ago

Comment on attachment 8691545 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: bug 1133213 or follow-ups (aria-owns work)
[User impact if declined]: Crash on messenger.com and other pages with anything that might use accessibility APIs. Also see comments above.
[Describe test coverage new/current, TreeHerder]: On m-c for days.
[Risks and why]: None known.
[String/UUID change made/needed]: None.

	Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)
	Comment 13 • 9 years ago

David, could you please verify this crash has gone away on the latest Nightly? Thanks!

	Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)
	Comment 14 • 9 years ago

Comment on attachment 8691545 [details] [diff] [review]
patch

Crash fixes are good. Let's uplift to Aurora44.

	David Bolter [:davidb] (NeedInfo me for attention) Reporter
	Comment 15 • 9 years ago

(In reply to Ritu Kothari (:ritu) from comment #13)
> David, could you please verify this crash has gone away on the latest
> Nightly? Thanks!

Yes, I agree with Ritu's comment 14. (No crashes for 45)

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 16 • 9 years ago

This doesn't appear to apply cleanly to aurora, can we get a rebased patch for it?

	alexander :surkov (:asurkov) Assignee
	Comment 17 • 9 years ago

(In reply to Wes Kocher (:KWierso) from comment #16)
> This doesn't appear to apply cleanly to aurora, can we get a rebased patch
> for it?

it may have a different implementation, so it might require a new patch. The search doesn't give me any crash reports, not sure why https://crash-stats.mozilla.com/report/list?product=Firefox&range_unit=days&range_value=28&signature=mozilla%3A%3Aa11y%3A%3ADocAccessible%3A%3AUpdateTreeOnInsertion%28mozilla%3A%3Aa11y%3A%3AAccessible*%29#tab-reports. Do we have a stack for aurora?

	David Bolter [:davidb] (NeedInfo me for attention) Reporter
	Comment 18 • 9 years ago

(In reply to alexander :surkov from comment #17)
> Do we have a stack for aurora?

Yes, try http://is.gd/KWN3kC

Example: https://crash-stats.mozilla.com/report/index/2944e72d-531d-4535-b553-ae2852151203

	David Bolter [:davidb] (NeedInfo me for attention) Reporter
	Comment 20 • 9 years ago

Comment on attachment 8695422 [details] [diff] [review]
crash

Review of attachment 8695422 [details] [diff] [review]:
-----------------------------------------------------------------

r=me, looks familiar ;)

	Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)
	Comment 21 • 9 years ago

This is verified based on comment 15.

	Carsten Book [:Tomcat]
	Comment 22 • 9 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/d61e62d4fc42

	Carsten Book [:Tomcat]
	Comment 23 • 9 years ago
bugherder uplift

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/d61e62d4fc42

	BugBot [:suhaib / :marco/ :calixte]
	Comment 24 • 8 years ago

Crash volume for signature 'mozilla::a11y::DocAccessible::UpdateTreeOnInsertion':
 - nightly(version 50):0 crashes from 2016-06-06.
 - aurora (version 49):0 crashes from 2016-06-07.
 - beta   (version 48):0 crashes from 2016-06-06.
 - release(version 47):54 crashes from 2016-05-31.
 - esr    (version 45):9 crashes from 2016-04-07.

Crash volume on the last weeks:
            W. N-1  W. N-2  W. N-3  W. N-4  W. N-5  W. N-6  W. N-7
 - nightly       0       0       0       0       0       0       0
 - aurora        0       0       0       0       0       0       0
 - beta          0       0       0       0       0       0       0
 - release       2       9       8       5       9       9      11
 - esr           1       1       1       0       2       0       3

Affected platform: Windows