Closed
Bug 1128751
Opened 10 years ago
Closed 10 years ago
[e10s] content process crash (abort?) when viewing article on forbes.com, with " ###!!! ASSERTION: no proxy for event!", "adding child to unknown accessible", and "IPDL protocol error: Handler for ShowEvent returned error code"
Categories
(Core :: Disability Access APIs, defect)
Tracking
()
RESOLVED
FIXED
mozilla39
People
(Reporter: dholbert, Assigned: tbsaunde)
References
Details
Attachments
(2 files)
22.93 KB,
text/plain
|
Details | |
6.99 KB,
patch
|
davidb
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Reporter | ||
Comment 1•10 years ago
|
||
Spinning this bug off for the content-process crash that remains after bug 1124821. STR: 1. Be logged into a graphical desktop like Ubuntu Unity or gnome-shell, and run: gsettings set org.gnome.desktop.interface toolkit-accessibility true to enable a11y and make Firefox detect that it should enable a11y. 2. In a profile with e10s enabled (e.g. a fresh profile that you're just using for the first time), visit http://www.forbes.com/sites/sarahjeong/2015/01/22/the-dread-pirates-diary/ 3. Click "Continue to site" link at upper-right, on splash page. 4. Wait ~30 seconds. ACTUAL RESULTS: The tab crashes (or maybe aborts?) and shows the tab crashed UI. I've got a "rr" recording of this that tbsaunde has kindly agreed to poke around in... stay tuned.
Summary: [e10s] content process crash (abort?) when viewing article on forbes.com, with " ###!!! ASSERTION: no proxy for event!" → [e10s] content process crash (abort?) when viewing article on forbes.com, with " ###!!! ASSERTION: no proxy for event!", "adding child to unknown accessible", and "IPDL protocol error: Handler for ShowEvent returned error code"
Reporter | ||
Comment 2•10 years ago
|
||
Here's a log of what the terminal output looks like (as replayed by "rr"), just before the child process dies. (I believe the nsStringStats output at the end is an indication that the child process has just died.)
Reporter | ||
Comment 3•10 years ago
|
||
Also, FWIW, we get a bunch of instances of this assertion throughout the recording: { ###!!! ASSERTION: does it make sense to Show()/Hide() a disabled widget?: 'mEnabled', file widget/PuppetWidget.cpp, line 173 } I think this is unrelated to the actual issue here, since it's so frequent & happens much much earlier than the child process dying. I'm only bringing it up to say that it's probably ignorable. (whereas the other assertions all seem a11y-related & relevant)
Updated•10 years ago
|
Blocks: e10sa11y2
tracking-e10s:
--- → +
Reporter | ||
Comment 5•10 years ago
|
||
Bug 1138700 seems very similar to this bug, & may be easier to trigger (since it doesn't require interaction). Just enter Responsive Design Mode (ctrl+shift+m) and visit wired.com, and wait for a second or two. And have your system configured so that a11y is enabled, per comment 0 here. tbsaunde / smaug, can you reproduce bug 1138700 locally? If so, maybe you no longer to rely on rr & remote debugging on my machine..
Reporter | ||
Comment 6•10 years ago
|
||
I can't reproduce this or Bug 1138700 in current nightly, actually! I'll see if I can figured out when it was fixed in a bit. (sanity-check: I do still have a11y enabled in "gsettings", and Firefox is still running in a11y-enabled mode, as evidenced by its force-disabling e10s after I quit & restart the browser.)
Reporter | ||
Comment 7•10 years ago
|
||
Fix range (using this bug's forbes.com STR) is: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c5b90c003be8&tochange=993eb76a8bd6 which includes: > db7e1eae0a00 Trevor Saunders β bug 1123511 - temporarily disable ipc accessibility for crashes I'm betting that's what's saving us here. So, this isn't really fixed; just hacked around.
Assignee | ||
Comment 8•10 years ago
|
||
The code for managing document lifetimes assumed documents could not be rebound to parents, however that is not the case.
Attachment #8582551 -
Flags: review?(dbolter)
Updated•10 years ago
|
Assignee: nobody → tbsaunde+mozbugs
Comment 9•10 years ago
|
||
Comment on attachment 8582551 [details] [diff] [review] handle unbinding and rebinding of documents in content processes Review of attachment 8582551 [details] [diff] [review]: ----------------------------------------------------------------- r=me, nice catch! Nits follow: ::: accessible/base/NotificationController.cpp @@ +281,5 @@ > for (size_t i = 0; i < newDocCount; i++) { > DocAccessible* childDoc = newChildDocs[i]; > + Accessible* parent = childDoc->Parent(); > + DocAccessibleChild* parentIPCDoc = mDocument->IPCDoc(); > + uint64_t id = reinterpret_cast<uintptr_t>(parent->UniqueID()); I'll trust you that there is always a parent in this context (and you didn't add any risk beyond what we had anyway). @@ +282,5 @@ > DocAccessible* childDoc = newChildDocs[i]; > + Accessible* parent = childDoc->Parent(); > + DocAccessibleChild* parentIPCDoc = mDocument->IPCDoc(); > + uint64_t id = reinterpret_cast<uintptr_t>(parent->UniqueID()); > + MOZ_ASSERT(id); nit: remove two spaces. ::: accessible/ipc/DocAccessibleParent.cpp @@ +138,5 @@ > return true; > } > + > +bool > +DocAccessibleParent::RecvBindChildDoc(PDocAccessibleParent* aChildDoc, const uint64_t& aID) Note to self: as per IRC, I was worried this might not get called before the next WillRefresh but long story short, if it doesn't then we should still be okay (given message order should be preserved, presumably at worst we'll do extra unbind/bind pairs - and hopefully that is unlikely). ::: accessible/ipc/DocAccessibleParent.h @@ +48,5 @@ > + virtual bool RecvBindChildDoc(PDocAccessibleParent* aChildDoc, const uint64_t& aID) override; > + void Unbind() > + { > + mParent = nullptr; > +mParentDoc->mChildDocs.RemoveElement(this); nit: indent 4 spaces.
Attachment #8582551 -
Flags: review?(dbolter) → review+
Assignee | ||
Comment 10•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/063b559e525d
Comment 11•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/063b559e525d
Status: NEW → RESOLVED
Closed: 10 years ago
status-firefox39:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
You need to log in
before you can comment on or make changes to this bug.
Description
•