Closed Bug 1366264 Opened 3 years ago Closed 2 years ago

Crash in IPCError-browser | PDocAccessibleParent::RecvShowEvent failed to add children

Categories

(Core :: Disability Access APIs, defect, P1, critical)

Unspecified
Windows 7
defect

Tracking

()

RESOLVED DUPLICATE of bug 1376754

People

(Reporter: jseward, Assigned: handyman)

References

Details

(Keywords: crash, Whiteboard: aes+)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is 
report bp-3235f663-c95f-4953-9fbf-6d60c0170519.
=============================================================

This is #13 topcrash in the Windows nightlies of 20170518030213.
Flags: needinfo?(surkov.alexander)
cc'ing more people, who's more aware of e10s a11y architecture than me, for ideas
Flags: needinfo?(surkov.alexander)
Whiteboard: aes+
Depends on: 1390143
Turns out this was due to even coalescing but isn't related to the SendPDocAccessibleConstructor message.

The patch pretty much speaks for itself.  Its pretty involved how this code leads to the error we are seeing but here's the broad strokes:

1. [child] We send a ShowEvent to create the DocAccessible tree in the parent, using IDs that are just the 64-bit pointers to DocAccessibles in the child.
2. [child] A pagehide queues hide events in the DocAccessible's NotificationController.  Specifically, accessibles that are hidden have HideEventTarget set to true.
3. [child] The nsIContent is removed.  This causes the NotificationController to coalesce events.
4. [child] An accessible ACC has its hide event coalesced.  HideEventTarget is not cleared.  This is the bug.
5. [child] A child of ACC incorrectly has its hide event coalesced, based on the belief that its parent ACC still has a hide event.
6. [child] The accessible tree gets freed.
7. [child] Another accessible tree is allocated and ShowEvent-ed.  Allocators being what they are, there is a fair chance that an accessible is allocated in the same spot as a previously deleted one since they will have the same size.  Remember, the 'this' pointer is used as the ID in the parent.
8. [parent] The ShowEvent is received and has an ID for the old accessible tree, including the one whose hide was incorrectly coalesced, which led to it not being freed in the parent.  Boom.  Note that the early error would have been this [1] but because its just an NS_ERROR, we don't get the failure until the IPC_FAIL.
8b. [child] Actually, when encountered in a debug build, the error you see is from DocAccessible::ContentRemoved [2].  It happens some time after step 5.

-----

These are pretty lousy STR.  This works for me to reproduce this issue nearly every time (with the caveat about the earlier assert in step 8b).  

1. Run a debug version of nightly with a11y on (accessibility.force_disabled = -1).  Release doesn't seem to repro this.
2. Open two tabs.  Open one to Pickle Rick!:
https://sketchfab.com/models/d779bda72fe441c297836a197640a709
I don't think it matters what page the other is on.
3. Make sure you are on the Non-Pickle Rick (NPR) page and close the browser.
4. Restart the browser and restore to the NPR.  Switch tabs to Pickle Rick.  Wait for the page to load the model.

Expected Results:
Pickle Rick.

Actual Results:
CRASH!  (before Pickle Rick is displayed)

-----

[1] http://searchfox.org/mozilla-central/source/accessible/ipc/DocAccessibleParent.cpp#115
[2] http://searchfox.org/mozilla-central/source/accessible/generic/DocAccessible.cpp#2008
Assignee: nobody → davidp99
Attachment #8897680 - Flags: review?(aklotz)
I just landed the exact same patch for bug 1376754, doh!
Comment on attachment 8897680 [details] [diff] [review]
Unmark coalesced hidden accessibles

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

yep, just landed, see comment #4, and thanks David for nailing it down, now I'm more confident in taken approach :)
Attachment #8897680 - Flags: review?(aklotz)
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1376754
(In reply to alexander :surkov from comment #5)
> yep, just landed, see comment #4, and thanks David for nailing it down, now
> I'm more confident in taken approach :)

Yes, great analysis! Thank you so much!
Depends on: 1394924
You need to log in before you can comment on or make changes to this bug.