(Away) Reporter
	Description • 9 years ago

This bug was filed from the Socorro interface and is 
report bp-199b1006-4e4d-4751-bbfc-dcb2a2150813.
=============================================================

This signature spiked in 41b1 (because this code is new to 41).

Frame 	Module 	Signature 	Source
0 	xul.dll 	mozilla::a11y::ARIAGridCellAccessible::GroupPosition() 	accessible/generic/ARIAGridAccessible.cpp
1 	xul.dll 	mozilla::a11y::Accessible::NativeAttributes() 	accessible/generic/Accessible.cpp
2 	xul.dll 	mozilla::a11y::HyperTextAccessible::NativeAttributes() 	accessible/generic/HyperTextAccessible.cpp
3 	xul.dll 	mozilla::a11y::ARIAGridCellAccessible::NativeAttributes() 	accessible/generic/ARIAGridAccessible.cpp
4 	xul.dll 	mozilla::a11y::Accessible::Attributes() 	accessible/generic/Accessible.cpp
5 	xul.dll 	mozilla::a11y::ia2Accessible::get_attributes(wchar_t**) 	accessible/windows/ia2/ia2Accessible.cpp

	(Away) Reporter
	Comment 1 • 9 years ago

[Tracking Requested - why for this release]: topcrash on 41

	(Away) Reporter
	Comment 2 • 9 years ago

http://hg.mozilla.org/releases/mozilla-beta/annotate/be2423a7ec20/accessible/generic/ARIAGridAccessible.cpp#l693

Something in this chain is null: Table()->AsAccessible()->GetContent().
I think it is the |Table()|.

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

Tracked for 41 and 42 as crashes are bad.

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

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

r=me. Do we have other places in our table code where we have similar constructs that could bite us in the future?

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

(In reply to Marco Zehe (:MarcoZ) from comment #5)
> Comment on attachment 8649278 [details] [diff] [review]
> patch
> 
> r=me. Do we have other places in our table code where we have similar
> constructs that could bite us in the future?

it seems no other places.

	Pulsebot
	Comment 7 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/2e89ce960e5f

	Ryan VanderMeulen [:RyanVM]
	Comment 8 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/2e89ce960e5f

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

Alexander, given that this was a top crash for 41, do you want to request uplift to Beta on this patch? It looks safe to me but you are the best judge. Thanks!

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

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

Approval Request Comment
[Feature/regressing bug #]: bug 1177268
[User impact if declined]: assistive technologies users keep crashing
[Describe test coverage new/current, TreeHerder]: not covered
[Risks and why]: no risk, a null check
[String/UUID change made/needed]: no

	Sylvestre Ledru [:Sylvestre]
	Comment 11 • 9 years ago

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

Fix a top crash, taking it.

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

https://hg.mozilla.org/releases/mozilla-beta/rev/b28e5271cb63

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

https://hg.mozilla.org/releases/mozilla-aurora/rev/6dda342701ab

	(Away) Reporter
	Comment 14 • 9 years ago

This crash is still seen on all channels where the patch landed. Looking at the disassembly of beta4, I'm pretty certain the null pointer is |Table()|, not |table|.

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

Comment on attachment 8653627 [details] [diff] [review]
patch2

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

Yikes! *That* is where it failed! OK, let's hope that we've now kicked it into obedience! Thanks! r=me.

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

Comment on attachment 8653627 [details] [diff] [review]
patch2

Umm ... I said: r=me!

	(Away) Reporter
	Comment 18 • 9 years ago

Comment on attachment 8653627 [details] [diff] [review]
patch2

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

::: accessible/generic/ARIAGridAccessible.cpp
@@ +690,5 @@
>  ARIAGridCellAccessible::GroupPosition()
>  {
>    int32_t count = 0, index = 0;
> +  Accessible* table = Table();
> +  if (table && nsCoreUtils::GetUIntAttr(table->->AsAccessible()->GetContent(),

There's a double arrow here.

	Pulsebot
	Comment 19 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/eb84e5f9c664

	Ryan VanderMeulen [:RyanVM]
	Comment 20 • 9 years ago

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

	Ryan VanderMeulen [:RyanVM]
	Comment 21 • 9 years ago

Please request Aurora/Beta approval on this when you get a chance.

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

Comment on attachment 8653627 [details] [diff] [review]
patch2

Approval Request Comment
[Feature/regressing bug #]: bug 1177268
[User impact if declined]: Crashes.
[Describe test coverage new/current, TreeHerder]: On Central, follow-up/corrected patch to previous one from this bug.
[Risks and why]: Null check.
[String/UUID change made/needed]: None.

	Sylvestre Ledru [:Sylvestre]
	Comment 23 • 9 years ago

Comment on attachment 8653627 [details] [diff] [review]
patch2

> [Risks and why]: Null check.
I don't see a risk evaluation here?! Please be more explicit for future uplift requests.

	Ryan VanderMeulen [:RyanVM]
	Comment 24 • 9 years ago

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

	Ryan VanderMeulen [:RyanVM]
	Comment 25 • 9 years ago

https://hg.mozilla.org/releases/mozilla-beta/rev/ae10d6a0419a