[e10s] crash in mozilla::a11y::ProxyAccessible::Shutdown()

VERIFIED FIXED in Firefox 42

Status

()

defect
--
critical
VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: dmajor, Assigned: lsocks)

Tracking

({crash, topcrash-win})

36 Branch
mozilla42
x86
All
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(e10sm8+, firefox35 unaffected, firefox36- affected, firefox40- affected, firefox41- affected, firefox42+ verified)

Details

(crash signature)

Attachments

(3 attachments, 3 obsolete attachments)

This bug was filed from the Socorro interface and is 
report bp-27e05da5-7a49-459c-9d73-4717d2141110.
=============================================================

mDoc is null at ProxyAccessible::Shutdown. This definitely started on nightly  	20141105030203, but I don't see anything suspicious in the regression range: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5dde8ea48fef&tochange=62990ec7ad78

Trevor any ideas?
Flags: needinfo?(tbsaunde+mozbugs)
[Tracking Requested - why for this release]: 9% of nightly crashes
So, I think what's happening is that we call ProxyAccessible::Shutdown() on a DocAccessibleParent where mDoc is null.  I don't think that can happen if the outer document is also being shutdown because of the order in DocAccessible::Shutdown where the child docs are shut down first, but it might be possible if content is being removed from the outer document, and that includes a sub document.
Flags: needinfo?(tbsaunde+mozbugs)
Important crash, tracking.
This crash has been an on off thing for me on some builds I can reliably trigger the crash with snagit.exe on some it seems to work... but I see the accessibility prompt flicker when snagit freezes the screen for its capture.

Just tried on Build ID Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:36.0) Gecko/20100101 Firefox/36.0 ID:20141118144012 CSet: 084441e904d1

Story:
open firefox
take a screen shot with snagit
accessibility prompt drops down from urlbar
snagit freezes screen for capture
the prompt goes away when the screen is frozen 
after taking the screen shot, the prompt is gone, 
but the pref browser.tabs.remote.autostart.disabled-because-using-a11y is set to true 
no crash on this build.
This crash has been an on off thing for me on some builds I can reliably trigger the crash with snagit.exe on some it seems to work... but I see the accessibility prompt flicker when snagit freezes the screen for its capture.

Just tried on Build ID Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:36.0) Gecko/20100101 Firefox/36.0 ID:20141118144012 CSet: 084441e904d1

Story is slightly different
opened firefox
take a screen shot with snagit
accessibility prompt drops down from urlbar
snagit freezes screen for capture
the prompt goes away when the screen is frozen 
after taking the screen shot, the prompt is gone, 
but the pref browser.tabs.remote.autostart.disabled-because-using-a11y is set to true 
went into about config and disabled that pref again
continued writing into this bug
crash upon pressing "save changes" of this bug
This crash is here: bp bp-393e1ae4-9602-4d16-9641-171272141119	19.11.2014	19:15
I see that too, Linux & latest nightly, every time when e10s is enabled.
OS: Windows NT → All
Summary: crash in mozilla::a11y::ProxyAccessible::Shutdown() → [e10s] crash in mozilla::a11y::ProxyAccessible::Shutdown()
The e10s may be a different crash in ProxyAccessible::Shutdown(), there's the bt:

#4  0x00007f9dad29216d in AsmJSFaultHandler(int, siginfo_t*, void*) (signum=11, info=0x7fff1d2444f0, context=0x7fff1d2443c0)
    at /home/komat/tmp676-trunk-gtk3/src-gtk2/js/src/asmjs/AsmJSSignalHandlers.cpp:924
#5  0x00007f9db32620d0 in <signal handler called> () at /lib64/libpthread.so.0
#6  0x00007f9dac51dd4a in mozilla::a11y::ProxyAccessible::Shutdown() (this=0x7f9d7a730d90)
    at /home/komat/tmp676-trunk-gtk3/src-gtk2/accessible/ipc/ProxyAccessible.cpp:18
#7  0x00007f9dac51dda4 in mozilla::a11y::ProxyAccessible::Shutdown() (this=0x7f9d7a730940)
    at /home/komat/tmp676-trunk-gtk3/src-gtk2/accessible/ipc/ProxyAccessible.cpp:25
#8  0x00007f9dac51dda4 in mozilla::a11y::ProxyAccessible::Shutdown() (this=0x7f9d75e2b0d0)
    at /home/komat/tmp676-trunk-gtk3/src-gtk2/accessible/ipc/ProxyAccessible.cpp:25
#9  0x00007f9dac51d3a7 in mozilla::a11y::DocAccessibleParent::RecvHideEvent(unsigned long const&) (this=0x7f9d7f7d0a20, aRootID=@0x7fff1d244a30: 139860106820848) at /home/komat/tmp676-trunk-gtk3/src-gtk2/accessible/ipc/DocAccessibleParent.cpp:112
#10 0x00007f9da9c3fd24 in mozilla::a11y::PDocAccessibleParent::OnMessageReceived(IPC::Message const&) (this=0x7f9d7f7d0a20, 
    __msg=...) at /home/komat/tmp676-trunk-gtk3/src-gtk2/objdir/ipc/ipdl/PDocAccessibleParent.cpp:445
#11 0x00007f9da9bd1a09 in mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&) (this=0x7f9d7acc2000, __msg=...) at /home/komat/tmp676-trunk-gtk3/src-gtk2/objdir/ipc/ipdl/PContentParent.cpp:2765
#12 0x00007f9da9a6abcb in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) (this=0x7f9d7acc2060, aMsg=...) at /home/komat/tmp676-trunk-gtk3/src-gtk2/ipc/glue/MessageChannel.cpp:1231
#13 0x00007f9da9a6a694 in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message const&) (this=0x7f9d7acc2060, aMsg=...)
    at /home/komat/tmp676-trunk-gtk3/src-gtk2/ipc/glue/MessageChannel.cpp:1158
#14 0x00007f9da9a6a59a in mozilla::ipc::MessageChannel::OnMaybeDequeueOne() (this=0x7f9d7acc2060)
    at /home/komat/tmp676-trunk-gtk3/src-gtk2/ipc/glue/MessageChannel.cpp:1142
#15 0x00007f9da9a7c354 in DispatchToMethod<mozilla::ipc::MessageChannel, bool (mozilla::ipc::MessageChannel::*)()>(mozilla::ipc::MessageChannel*, bool (mozilla::ipc::MessageChannel::*)(), Tuple0 const&) (obj=0x7f9d7acc2060, method=(bool (mozilla::ipc::MessageChannel::*)(mozilla::ipc::MessageChannel * const)) 0x7f9da9a6a3bc <mozilla::ipc::MessageChannel::OnMaybeDequeueOne()>, arg=...) at /home/komat/tmp676-trunk-gtk3/src-gtk2/ipc/chromium/src/base/tuple.h:383
#16 0x00007f9da9a7bcf4 in RunnableMethod<mozilla::ipc::MessageChannel, bool (mozilla::ipc::MessageChannel::*)(), Tuple0>::Run() (this=0x7f9d84a1cb80) at /home/komat/tmp676-trunk-gtk3/src-gtk2/ipc/chromium/src/base/task.h:307
#17 0x00007f9da9a5c281 in mozilla::ipc::MessageChannel::RefCountedTask::Run() (this=0x7f9d848fd4e0)
    at ../../dist/include/mozilla/ipc/MessageChannel.h:437
#18 0x00007f9da9a5c456 in mozilla::ipc::MessageChannel::DequeueTask::Run() (this=0x7f9d771dfe80)
    at ../../dist/include/mozilla/ipc/MessageChannel.h:454
#19 0x00007f9da9a2f905 in MessageLoop::RunTask(Task*) (this=0x7f9d94c2c360, task=0x7f9d771dfe80)
    at /home/komat/tmp676-trunk-gtk3/src-gtk2/ipc/chromium/src/base/message_loop.cc:361
#20 0x00007f9da9a2f97d in MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const&) (this=0x7f9d94c2c360, pending_task=...) at /home/komat/tmp676-trunk-gtk3/src-gtk2/ipc/chromium/src/base/message_loop.cc:369
#21 0x00007f9da9a2fd48 in MessageLoop::DoWork() (this=0x7f9d94c2c360)
    at /home/komat/tmp676-trunk-gtk3/src-gtk2/ipc/chromium/src/base/message_loop.cc:447
#22 0x00007f9da9a6e36c in mozilla::ipc::DoWorkRunnable::Run() (this=0x7f9d94c3ca00)
    at /home/komat/tmp676-trunk-gtk3/src-gtk2/ipc/glue/MessagePump.cpp:233
#23 0x00007f9da95ec007 in nsThread::ProcessNextEvent(bool, bool*) (this=0x7f9db2262f40, aMayWait=false, aResult=0x7fff1d2461ff) at /home/komat/tmp676-trunk-gtk3/src-gtk2/xpcom/threads/nsThread.cpp:855

Asserts here: 
MOZ_ASSERT(!mOuterDoc);
(In reply to Martin Stránský from comment #7)
> The e10s may be a different crash in ProxyAccessible::Shutdown(), there's
> the bt:

this code should only run with e10s.

> 
> #4  0x00007f9dad29216d in AsmJSFaultHandler(int, siginfo_t*, void*)
> (signum=11, info=0x7fff1d2444f0, context=0x7fff1d2443c0)
>     at
> /home/komat/tmp676-trunk-gtk3/src-gtk2/js/src/asmjs/AsmJSSignalHandlers.cpp:
> 924
> #5  0x00007f9db32620d0 in <signal handler called> () at
> /lib64/libpthread.so.0
> #6  0x00007f9dac51dd4a in mozilla::a11y::ProxyAccessible::Shutdown()
> (this=0x7f9d7a730d90)
>     at
> /home/komat/tmp676-trunk-gtk3/src-gtk2/accessible/ipc/ProxyAccessible.cpp:18

Is this a debug build? in my tree that line is an assert.  I'd really like a test case since I've never seen it fire, but I guess we could downgrade that assert to NS_ASSERTION.
(In reply to Trevor Saunders (:tbsaunde) from comment #8)
> > #4  0x00007f9dad29216d in AsmJSFaultHandler(int, siginfo_t*, void*)
> > (signum=11, info=0x7fff1d2444f0, context=0x7fff1d2443c0)
> >     at
> > /home/komat/tmp676-trunk-gtk3/src-gtk2/js/src/asmjs/AsmJSSignalHandlers.cpp:
> > 924
> > #5  0x00007f9db32620d0 in <signal handler called> () at
> > /lib64/libpthread.so.0
> > #6  0x00007f9dac51dd4a in mozilla::a11y::ProxyAccessible::Shutdown()
> > (this=0x7f9d7a730d90)
> >     at
> > /home/komat/tmp676-trunk-gtk3/src-gtk2/accessible/ipc/ProxyAccessible.cpp:18
> 
> Is this a debug build? in my tree that line is an assert.  I'd really like a
> test case since I've never seen it fire, but I guess we could downgrade that
> assert to NS_ASSERTION.

Yes, that's a debug build and triggered assertion. It's latest trunk, Fedora 21, x86_64, e10s enabled. It happens almost immediately after start with 3-4 tabs opened.
> but I guess we could downgrade that assert to NS_ASSERTION.

With the relaxed assert it crashes here, because mChildDocs is not empty:

157	void
158	DocAccessibleParent::Destroy()
159	{
160	  MOZ_ASSERT(mChildDocs.IsEmpty(), <<--- crash
161	      "why wheren't the child docs destroyed already?");
162	  MOZ_ASSERT(!mShutdown);
163	  mShutdown = true;
> Yes, that's a debug build and triggered assertion. It's latest trunk, Fedora
> 21, x86_64, e10s enabled. It happens almost immediately after start with 3-4
> tabs opened.

funny, I'm also testing on x86_64 linux, I wonder why I haven't seen it.




(In reply to Martin Stránský from comment #10)
> > but I guess we could downgrade that assert to NS_ASSERTION.
> 
> With the relaxed assert it crashes here, because mChildDocs is not empty:
> 
> 157	void
> 158	DocAccessibleParent::Destroy()
> 159	{
> 160	  MOZ_ASSERT(mChildDocs.IsEmpty(), <<--- crash

yeah, we'd need to downgrade that one too.
Seems like an e10s-only bug, not tracking anymore
That mChildren pointer is the poison value, so that ProxyAccessible is dead. (On a side note, is it normal to have such a deep stack of Shutdowns?)
(In reply to David Bolter [:davidb] from comment #13)
> This is still around. Is mchildren garbage?
> https://crash-stats.mozilla.com/report/index/223a7259-4338-4069-b111-
> b0ac02150416

presumably.  I thought ofr a while I could explain that, but that theory seems to be wrong.  How did you find this crash?  Please tell me you have some idea how to reproduce.


(In reply to David Major [:dmajor] from comment #14)
> That mChildren pointer is the poison value, so that ProxyAccessible is dead.
> (On a side note, is it normal to have such a deep stack of Shutdowns?)

I doubt that's the average case, but it doesn't seem unreasonable either, it could be from destorying a whole document or something.
(In reply to Trevor Saunders (:tbsaunde) from comment #15)
> (In reply to David Bolter [:davidb] from comment #13)
> > This is still around. Is mchildren garbage?
> > https://crash-stats.mozilla.com/report/index/223a7259-4338-4069-b111-
> > b0ac02150416
> 
> presumably.  I thought ofr a while I could explain that, but that theory
> seems to be wrong.  How did you find this crash?  Please tell me you have
> some idea how to reproduce.

Sorry I don't. I found this crash when I was looking around in crash-stats.
I have a screenshot tool installed ( EasyScreenshot 1.2.5.1 ).
Apparently someone else above can get it with SnagIt .

Is it a coincidence?
No reliable reproduction.

First time clean profile. XFCE desktop gives me a11y on (e10s on by default.) hola.org I think happen around time I pressed accept to add-on popup. (wasn't attentive as not expecting crash.)
bp-85ce56bb-1c8a-426b-b2be-c8fab2150507

Tried to reproduce with low luck.
Twice in many attempts managed to crash both with just started clean profiles.
Visit hola.org. Crash as soon as I clicked link "Get Hola, it's free!"
bp-473a5924-5fa0-455a-9d18-fc1ab2150507
bp-8e79c2a0-5e0e-4058-95a8-7d8282150507


(My bet is this crash will be noticeable when uplifted to aurora.
Nightly users are blocked browser.tabs.remote.autostart.disabled-because-using-a11y unless they reset.)
p.s. hola not e10s compatible.
This spiked in 20150524030234 and is now the #2 crash on nightly. Did something land recently that might have provoked the issue? Any better luck reproducing this time?
Flags: needinfo?(tbsaunde+mozbugs)
Bug 1164640 about 15% of nightly users were blocked from e10s.

Found I can get crashes refreshing some wikia pages. Not every refresh but crashes often enough to test to know it reproduces (at least one cause of) the bug. e.g.
http://unepic.wikia.com/wiki/Unepic_Wiki
(In reply to Trevor Saunders (:tbsaunde) from comment #15)
> (In reply to David Bolter [:davidb] from comment #13)

> (In reply to David Major [:dmajor] from comment #14)
> > That mChildren pointer is the poison value, so that ProxyAccessible is dead.
> > (On a side note, is it normal to have such a deep stack of Shutdowns?)
> 
> I doubt that's the average case, but it doesn't seem unreasonable either, it
> could be from destorying a whole document or something.

I think it is pretty unusual to be that deep no?
Added a tracking flag for FF40 as KaiRo mentioned this as a top issue in Dev edition in today's channel meeting.
This topcrash bug has been sitting idle for a long time. davidb could you help find an owner to actively investigate?
Flags: needinfo?(dbolter)
In case it helps, in crash reports this is the first frequent interesting (i.e. non-Facebook) URL that shows up:
http://tangerine-tycoon.wikia.com/wiki/Tangerine_Tycoon_Wiki

Trevor can you summarize our current plan for this? That is, no obvious way forward except to catch this in rr... and how that will be actively pursued very soon etc.
Flags: needinfo?(dbolter)
(In reply to David Bolter [:davidb] from comment #24)
> In case it helps, in crash reports this is the first frequent interesting
> (i.e. non-Facebook) URL that shows up:
> http://tangerine-tycoon.wikia.com/wiki/Tangerine_Tycoon_Wiki

Well facebok might be a fine reproduction case if someone can get it to crash for them.

> Trevor can you summarize our current plan for this? That is, no obvious way
> forward except to catch this in rr... and how that will be actively pursued
> very soon etc.

I not really sure what else there is to say.  I was hoping this was just the opt non debug version of 1170595, but aparently not.  Given that I'm not really sure there's anything to do accept try and catch it under rr.  Lorien's trying to do that now, but she seems to have found some other crash we'll need to deal with first.
Flags: needinfo?(tbsaunde+mozbugs)
I think we should turn e10s off for a11y in 40 at least. Jim what is the preferred way to do that?
Flags: needinfo?(jmathies)
(In reply to David Bolter [:davidb] from comment #26)
> I think we should turn e10s off for a11y in 40 at least. Jim what is the
> preferred way to do that?

That seems extreme, what's preventing us from finding a fix?
Flags: needinfo?(jmathies) → needinfo?(dbolter)
Does this happen in 41 at all? If this is 40 only, then I can see us disabling there although a fix would obviously be better.
Hmm, am I looking at the wrong signature here? ProxyAccessible vs. DocAccessible?
(In reply to Jim Mathies [:jimm] from comment #30)
> Hmm, am I looking at the wrong signature here? ProxyAccessible vs.
> DocAccessible?

I think so :)  We haven't been able to reproduce this one (ProxyAccessible).
Flags: needinfo?(dbolter)
(In reply to David Bolter [:davidb] from comment #31)
> (In reply to Jim Mathies [:jimm] from comment #30)
> > Hmm, am I looking at the wrong signature here? ProxyAccessible vs.
> > DocAccessible?
> 
> I think so :)  We haven't been able to reproduce this one (ProxyAccessible).

Ok but why would we disable to deal with it vs. trying to find str and fixing it?
> Ok but why would we disable to deal with it vs. trying to find str and fixing it?
FWIW I've been nagging people for a fix for months, and it hasn't amounted to anything.
(In reply to Jim Mathies [:jimm] from comment #32)
> (In reply to David Bolter [:davidb] from comment #31)
> > (In reply to Jim Mathies [:jimm] from comment #30)
> > > Hmm, am I looking at the wrong signature here? ProxyAccessible vs.
> > > DocAccessible?
> > 
> > I think so :)  We haven't been able to reproduce this one (ProxyAccessible).
> 
> Ok but why would we disable to deal with it vs. trying to find str and
> fixing it?

My thinking was that it was enough to leave it crashing on nightly and try to fix it there, and we could stop the bleeding on 40. (It is the top crasher) I suppose letting it ride might help us find STR?
Tracked down why I had accessibility on; dconf org.gnome.desktop.interface toolkit-accessibility was on.

accessibility.ipc_architecture.enabled false is one way to stop this crash. (Personally been stable with accessibility on with main default profile aurora and only had wikia being a trigger for crash.)

Using http://unepic.wikia.com/wiki/Unepic_Wiki
Found if I refresh quickly I occasionally get content crash
bp-a7d9e978-b3c8-445b-9ca2-48c122150616
which is top crash bug 1170153.

Then trying without such refresh frequency this browser crash
bp-a7d9e978-b3c8-445b-9ca2-48c122150616

I downloaded the debug build firefox-41.0a1.en-US.debug-linux-x86_64.tar.bz2
Content crash every time going to page. (Lacks symbols but maybe assertions or process of some help.)
Stop tracking for 40 as it will move to beta soon and e10s is disabled in beta.
This is essentially now the #1 crasher on Nightly (as sse2_composite_src_x888_8888 fades from stats)

It occurs mostly on Windows, plus a handful of reports on Linux
Keywords: topcrashtopcrash-win
top crasher on aurora
Assignee: nobody → jmathies
Hope you don't mind if I take this Jim. I was able to reproduce using the steps in bug 1181177 (using a longer sleep time, as the reporter suggested).
Assignee: jmathies → wmccloskey
(In reply to Bill McCloskey (:billm) from comment #39)
> Hope you don't mind if I take this Jim. I was able to reproduce using the
> steps in bug 1181177 (using a longer sleep time, as the reporter suggested).

huh! neither me or Lorien ever could I wonder what is different.  If you can give me access to an rr recording of the crash I can debug it if it turns out to be complicated.
So far I've figured out that there are two ProxyAccessibles with the same mID. That's the direct cause of the crash. Now I need to figure out why the child is doing that.
I stepped through the child and I think I found the problem. The child is creating an Accessible that I'll call A. The child sends A to the parent via SendShowEvent. Later on, the child deletes A with this stack:

#5  mozilla::a11y::Accessible::RemoveChild (this=<optimized out>, aChild=<optimized out>)
    at /home/billm/moz/gecko/accessible/generic/Accessible.cpp:2083
#6  0x00002aaaad49c22d in mozilla::a11y::HTMLLIAccessible::UpdateBullet (this=0x2aaacfdfa300, 
    aHasBullet=<optimized out>) at /home/billm/moz/gecko/accessible/html/HTMLListAccessible.cpp:111
#7  0x00002aaaad47c71c in nsAccessibilityService::UpdateListBullet (this=<optimized out>, 
    aPresShell=<optimized out>, aHTMLListItemContent=<optimized out>, aHasBullet=<optimized out>)
    at /home/billm/moz/gecko/accessible/base/nsAccessibilityService.cpp:609
#8  0x00002aaaad1e8e91 in nsBulletFrame::DidSetStyleContext (this=0x2aaad2c2f7e8, 
    aOldStyleContext=0x2aaad17debc8) at /home/billm/moz/gecko/layout/generic/nsBulletFrame.cpp:165
#9  0x00002aaaad15a0dd in nsIFrame::SetStyleContext (this=this@entry=0x2aaad2c2f7e8, 
    aContext=<optimized out>) at /home/billm/moz/gecko/layout/base/../generic/nsIFrame.h:543

The parent is never informed that the Accessible has been deleted since we never send a hide event. So the parent still has a ProxyAccessible around whose ID is A.

Later on, the child allocates a new Accessible that has the same address as A (let's call it A'). The parent gets a SendShowEvent for A' and now it has two ProxyAccessibles with the same ID.

Trevor, can you take over from here? If you need more details, let me know. It seems like we need a message to inform the parent that an Accessible has been deleted. The parent would delete the ProxyAccessible and remove it from its parent ProxyAccessible.
Flags: needinfo?(tbsaunde+mozbugs)
> Trevor, can you take over from here? If you need more details, let me know.

yeah.  This multiple proxies with the same id was an issue I was aware of and working on, but this is a different cause than the other one I know of.

> It seems like we need a message to inform the parent that an Accessible has
> been deleted. The parent would delete the ProxyAccessible and remove it from
> its parent ProxyAccessible.

Yeah, hide events cause that message to be sent so the issue is that bullet changes don't cause show / hide events.  That should be simple to fix, but I'm worried there are yet more cases of missing events :(
Flags: needinfo?(tbsaunde+mozbugs)
> Yeah, hide events cause that message to be sent so the issue is that bullet changes don't
> cause show / hide events.  That should be simple to fix, but I'm worried there are yet more
> cases of missing events :(

Yeah, that's why I was thinking it might be better to send a message on destruction. That way we're guaranteed never to get multiple Accessibles with the same ID.

Also, we need to protect against this problem in the parent. The child should never be able to cause the parent to crash. One way to do that would be to add a check here:
https://dxr.mozilla.org/mozilla-central/source/accessible/ipc/DocAccessibleParent.cpp#74
It would fail if there's already an Accessible with the given ID. In that case we should return false from RecvShowEvent, which would cause the child to crash.
(In reply to Bill McCloskey (:billm) from comment #44)
> > Yeah, hide events cause that message to be sent so the issue is that bullet changes don't
> > cause show / hide events.  That should be simple to fix, but I'm worried there are yet more
> > cases of missing events :(
> 
> Yeah, that's why I was thinking it might be better to send a message on
> destruction. That way we're guaranteed never to get multiple Accessibles
> with the same ID.

yeah, I'd been trying to avoid that for fear of sending tons of events, but maybe that's a mistake / premature optimization.

> Also, we need to protect against this problem in the parent. The child
> should never be able to cause the parent to crash. One way to do that would
> be to add a check here:
> https://dxr.mozilla.org/mozilla-central/source/accessible/ipc/
> DocAccessibleParent.cpp#74
> It would fail if there's already an Accessible with the given ID. In that
> case we should return false from RecvShowEvent, which would cause the child
> to crash.

yeah, I didn't say it been I've been working on this already because of the other issue we found with multiple ids.  That said While I don't like it much I'd been tempted to try and perform updates when we get the same id again.  That was because the other case involved   somewhat dupplicate events.  However this case makes me think its worth reconsidering if that's a good idea.  Either way we'll probably have some tricky bugs.
I'm unable to actually test this.

This is modelled this off of what we do in HTMLImageMapAccessible::UpdateChildAreas. I was thinking that it might be better to always call DocAccessible::ProcessContentRemoved/Inserted for any Accessible insertions/removals instead of creating events in different places as we do now, but the tree update code might be unnecessary for what we're doing here.
Attachment #8632256 - Flags: feedback?(tbsaunde+mozbugs)
> This is modelled this off of what we do in
> HTMLImageMapAccessible::UpdateChildAreas. I was thinking that it might be
> better to always call DocAccessible::ProcessContentRemoved/Inserted for any
> Accessible insertions/removals instead of creating events in different
> places as we do now, but the tree update code might be unnecessary for what
> we're doing here.


I'm not sure the generic code even works in this case, because bullets are special snow flakes.
Attachment #8632256 - Flags: feedback?(tbsaunde+mozbugs) → feedback+
Attachment #8632256 - Attachment is obsolete: true
Attachment #8633075 - Flags: review?(tbsaunde+mozbugs)
Attachment #8633075 - Attachment is obsolete: true
Attachment #8633075 - Flags: review?(tbsaunde+mozbugs)
Attachment #8633076 - Flags: review?(tbsaunde+mozbugs)
Attachment #8633076 - Attachment is obsolete: true
Attachment #8633076 - Flags: review?(tbsaunde+mozbugs)
Attachment #8633511 - Flags: review?(tbsaunde+mozbugs)
Attachment #8633511 - Flags: review?(tbsaunde+mozbugs) → review+
Posted patch ID checkSplinter Review
I think we should do this too. If it ever becomes legal to reuse IDs (and the crash that happens when you reuse them is fixed) it will be easy to remove this.
Attachment #8633526 - Flags: review?(tbsaunde+mozbugs)
> I think we should do this too. If it ever becomes legal to reuse IDs (and
> the crash that happens when you reuse them is fixed) it will be easy to
> remove this.

thanks for dealing with that.

As I said I was thinking about trying to handle this case without killing the child if it seemed like it might be sane to reuse the proxy, but on further examination of that patch this morning I came to the conclusion its a bad idea.  Its just way too complicated.
Attachment #8633526 - Flags: review?(tbsaunde+mozbugs) → review+
https://hg.mozilla.org/mozilla-central/rev/5f190b448f83
Status: NEW → RESOLVED
Closed: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Oops. Still have to land my patch.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
https://hg.mozilla.org/mozilla-central/rev/d72b754ac74f
Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
Bill, this is a top-crasher on Aurora. Do you think the patch is stable and should be uplifted to m-a?
Flags: needinfo?(wmccloskey)
That's up to Trevor, but I think it should be uplifted.
Assignee: wmccloskey → lorien
Flags: needinfo?(wmccloskey) → needinfo?(tbsaunde+mozbugs)
There haven't been any reports of this against Nightly(Fx42) after the fix was landed there on 20150716.
Status: RESOLVED → VERIFIED
(In reply to Ritu Kothari (:ritu) from comment #58)
> Bill, this is a top-crasher on Aurora. Do you think the patch is stable and
> should be uplifted to m-a?

we could, but I'd say its better to just take bug 1182097.
Flags: needinfo?(tbsaunde+mozbugs)
Untracking e10s issue for Aurora given that it is not enabled by default. Tracked for 42 instead.
You need to log in before you can comment on or make changes to this bug.