Closed Bug 1147646 Opened 7 years ago Closed 7 years ago

Crash in mozilla::a11y::DocAccessible::ProcessContentInserted while stability testing

Categories

(Core :: Disability Access APIs, defect)

ARM
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla39
blocking-b2g 2.2+
Tracking Status
firefox37 --- wontfix
firefox38 --- wontfix
firefox39 --- fixed
b2g-v2.2 --- fixed
b2g-master --- fixed

People

(Reporter: ggrisco, Assigned: surkov)

References

Details

(Keywords: crash, Whiteboard: [b2g-crash][caf-crash 589][caf priority: p1][CR 812999])

Crash Data

Attachments

(7 files)

The following crash has been seen while running stability tests.

[@ mozilla::a11y::DocAccessible::ProcessContentInserted | 
mozilla::a11y::NotificationController::ContentInsertion::Process | mozilla::a11y::NotificationController::WillRefresh | nsRefreshDriver::Tick ]

Maybe this is related to bug 1145724.

Cafbot will upload minidump and extra file (with logs).
Hi Mike,
Do you know who can take a look at this?
Flags: needinfo?(mlee)
Whiteboard: [CR 812999] → [caf priority: p1][CR 812999]
Whiteboard: [caf priority: p1][CR 812999] → [b2g-crash][caf-crash 589][caf priority: p1][CR 812999]
Keywords: crash
is there a link at mxr to see where it crashes (the dump is not applied to trunk)
Hi David,

Can you assign someone on your team to look into this issue?

Thanks,
Mike
Flags: needinfo?(mlee) → needinfo?(dbolter)
Alex it looks like either container or aContainer must be null? (in DocAccessible::ProcessContentInserted)
Flags: needinfo?(dbolter) → needinfo?(surkov.alexander)
(In reply to David Bolter [:davidb] from comment #9)
> Alex it looks like either container or aContainer must be null? (in
> DocAccessible::ProcessContentInserted)

what is changeset the dump is for? it's quite different from nightly sources.
Flags: needinfo?(surkov.alexander)
(In reply to alexander :surkov from comment #10)
> (In reply to David Bolter [:davidb] from comment #9)
> > Alex it looks like either container or aContainer must be null? (in
> > DocAccessible::ProcessContentInserted)
> what is changeset the dump is for? it's quite different from nightly sources.

It's Gecko 37, which is currently in the mozilla-beta repository.
With null pointer check, bug 1145724 has not crashed in latest test runs (80+ hours).  But this crash is now seen 8 time so far.  This bug is now the biggest obstacle to meeting stability goals.  Can we perform similar null check to prevent the crash?
Flags: needinfo?(mlee)
Attached patch patchSplinter Review
Attachment #8584545 - Flags: review?(dbolter)
Assignee: nobody → surkov.alexander
Comment on attachment 8584545 [details] [diff] [review]
patch

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

r=me, looks like the right place for the check. Thanks!
Attachment #8584545 - Flags: review?(dbolter) → review+
btw, if you can reproduce it then could you enable "tree" a11y logging (http://asurkov.blogspot.ca/2012/10/debugging-firefox-accessibility.html) and file output here. It'd be interesting to know exactly why the container is not in DOM tree longer.
Greg re: your comment 15:

> With null pointer check, bug 1145724 has not crashed in latest test runs
> (80+ hours).

That's great news.

> ...But this crash is now seen 8 time so far.  This bug is now the
> biggest obstacle to meeting stability goals.  Can we perform similar null
> check to prevent the crash?

Does the proposed check in attachment 8584545 [details] [diff] [review] suffice?

Mike
Flags: needinfo?(mlee) → needinfo?(ggrisco)
(In reply to Mike Lee [:mlee] from comment #19)
> Does the proposed check in attachment 8584545 [details] [diff] [review]
> suffice?
it should be
I'm going to apply the patch today, hopefully we can see the benefit from it by Monday.  Thanks for the quick turnaround.

To enable logging, I tried |export A11YLOG=focus,tree,DOMEvents;| but it didn't seem to have any effect for me.  But I also don't see Screen Reader option in setting menu.
Flags: needinfo?(ggrisco)
https://hg.mozilla.org/mozilla-central/rev/fef7f455e20c
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
blocking-b2g: 2.2? → ---
Attachment #8584545 - Flags: approval-mozilla-b2g37?
We need this fixed for v2.2
blocking-b2g: --- → 2.2+
surkov/:dbolter, can you please fill the details here to consider uplift for b2g37 branch ? Thanks! 
NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: 
Testing completed: 
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch:
Flags: needinfo?(surkov.alexander)
Flags: needinfo?(dbolter)
(In reply to Greg Grisco from comment #21)
> I'm going to apply the patch today, hopefully we can see the benefit from it
> by Monday.  Thanks for the quick turnaround.
> 
> To enable logging, I tried |export A11YLOG=focus,tree,DOMEvents;| but it
> didn't seem to have any effect for me.  But I also don't see Screen Reader
> option in setting menu.

Greg, was anyone able to verify the fix?
Flags: needinfo?(ggrisco)
(In reply to David Bolter [:davidb] from comment #25)
> Greg, was anyone able to verify the fix?

Hi David, we didn't see the crash in first 60 hours of test, so it looks good so far.  Probably best if we wait one more day before claiming victory though.  I'll keep my ni? and report back tomorrow.
(In reply to bhavana bajaj [:bajaj] from comment #24)
> surkov/:dbolter, can you please fill the details here to consider uplift for

[Approval Request Comment]
Bug caused by (feature/regressing bug #): unknown
User impact if declined: crashes
Testing completed: yes
Risk to taking this patch (and alternatives if risky): low, might affect on screen readers
String or UUID changes made by this patch: no
Flags: needinfo?(surkov.alexander)
Flags: needinfo?(dbolter)
We're still not seeing this reproduce, thanks!
Flags: needinfo?(ggrisco)
Attachment #8584545 - Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
You need to log in before you can comment on or make changes to this bug.