Closed Bug 1346518 Opened 4 years ago Closed 3 years ago

Crash in mozilla::a11y::Accessible::RemoveChild

Categories

(Core :: Disability Access APIs, defect)

52 Branch
x86
Windows 10
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox-esr52 --- fixed
firefox53 --- wontfix
firefox54 --- fixed
firefox55 --- fixed

People

(Reporter: calixte, Assigned: surkov)

References

(Blocks 2 open bugs)

Details

(Keywords: crash, regression, Whiteboard: [clouseau][aes+])

Crash Data

Attachments

(2 files)

This bug was filed from the Socorro interface and is 
report bp-23d9a615-5bf1-4994-b600-66af02161130.
=============================================================

There 4 crashes on nightly 55 with buildid 20170310110248. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1321384.

[1] https://hg.mozilla.org/mozilla-central/rev?node=dea9f3038c4d1cad37ac5d9908dbaef42fabecf2
Flags: needinfo?(surkov.alexander)
How is it different from bug 1321384?
Flags: needinfo?(surkov.alexander)
Did you mean Accessible::RemoveChild signature and crashes like this:
https://crash-stats.mozilla.com/report/index/eaab4989-c625-4174-904f-431cf2170310
Oh my bad... I made a mistake, I picked up the wrong crash report. Yes it's exactly this signature.
Crash Signature: [@ mozilla::a11y::Accessible::SetARIAHidden] → [@ mozilla::a11y::Accessible::RemoveChild ]
Summary: Crash in mozilla::a11y::Accessible::SetARIAHidden → Crash in mozilla::a11y::Accessible::RemoveChild
We have a bug 1342433 that should resolve the issue leading to a stack from comment #2.

Also I don't see how the tree may get corrupted by a tree recreation under a wrong container, the latter code should fix all issue introduced by that. So this stack may be unrelated with aria-hidden issue.
there's one more bug 1347667 for this signature, let's if these two bugs (comment #4) fix this bug.
(In reply to alexander :surkov from comment #5)
> there's one more bug 1347667 for this signature, let's if these two bugs
> (comment #4) fix this bug.

there's a stack not covered by those two https://crash-stats.mozilla.com/report/index/cd485e66-7442-40a5-9ea2-9e3712170320
Attached patch assertionsSplinter Review
1) add empty parent assertion, in other words split ASSERT(aChild->mParent == this) into two
2) get unwanted wording change back from https://hg.mozilla.org/mozilla-central/diff/dea9f3038c4d/accessible/generic/Accessible.cpp
Attachment #8849227 - Flags: review?(yzenevich)
Keywords: leave-open
Attachment #8849227 - Flags: review?(yzenevich) → review+
Attached patch assert2Splinter Review
add extra assertion, possibly catching cases like https://crash-stats.mozilla.com/report/index/cd485e66-7442-40a5-9ea2-9e3712170320
Assignee: nobody → surkov.alexander
Attachment #8849673 - Flags: review?(yzenevich)
Duplicate of this bug: 1349160
Comment on attachment 8849673 [details] [diff] [review]
assert2

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

looks good.
Attachment #8849673 - Flags: review?(yzenevich) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/b0bb65bdf413cda117ec915f8aa97d262b94f642
Bug 1346518 - extend Accessible::RemoveChild debugging assertions, part2, r=yzen
What's the status of this bug?  it's leave-open, so the landings (and other bugs mentioned here as possible fixes) haven't moved this to RESOLVED.
Flags: needinfo?(surkov.alexander)
(In reply to Randell Jesup [:jesup] from comment #15)
> What's the status of this bug?  it's leave-open, so the landings (and other
> bugs mentioned here as possible fixes) haven't moved this to RESOLVED.

this crash signature may be caused by number of reasons, few of them them were addressed, at least one is not yet fixed, for example, this one https://crash-stats.mozilla.com/report/index/cd485e66-7442-40a5-9ea2-9e3712170320
Flags: needinfo?(surkov.alexander)
I filed a bug 1351414 that could help with 'No parent' assertions
Flags: needinfo?(surkov.alexander)
Depends on: 1342433, 1351414
Depends on: 1349160
The issues on att.com that cause this crash should be repaired by the patches in bug 1353094.  We think there may be other causes to this crash, tho.
Blocks: 1353039
Depends on: 1353094
Whiteboard: [clouseau] → [clouseau][aes+]
This signature is the #5 Window top crash in Nightly 20170421030241.
(In reply to Nicholas Nethercote [:njn] from comment #22)
> This signature is the #5 Window top crash in Nightly 20170421030241.

I expect bug 1353094 helping it.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1353094
Technically bug 1353094 is a blocking one, not a dupe. Also do we have data that no other cases of this crash signature has left?
There are still some crashes on 53, 54, and 55. If this isn't a duplicate I guess we can re-open the bug. However with so few crashes, I don't think release management needs to track this issue.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #26)
> There are still some crashes on 53, 54, and 55. If this isn't a duplicate I
> guess we can re-open the bug. However with so few crashes, I don't think
> release management needs to track this issue.

I believe that 55 crashes should be fixed by recent bug 1363027, 53 and 54 crashes like [1] can be fixed by backporting bug 1347667, should I request the backporting or we don't care enough about this low volume crash to take a risk of backporting?

https://crash-stats.mozilla.com/report/index/409c6855-6a0d-4305-b1dd-80e850170529
Status: REOPENED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
Relevant patches from comment 27 have been backported to ESR52. Calling this fixed there as well.
You need to log in before you can comment on or make changes to this bug.