(Away) Reporter
	Description • 10 years ago

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?

	(Away) Reporter
	Comment 1 • 10 years ago

[Tracking Requested - why for this release]: 9% of nightly crashes

	Trevor Saunders (:tbsaunde)
	Comment 2 • 10 years ago

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.

	Sylvestre Ledru [:Sylvestre]
	Comment 3 • 10 years ago

Important crash, tracking.

	martin
	Comment 4 • 10 years ago

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.

	martin
	Comment 5 • 10 years ago

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

	Martin Stránský [:stransky] (ni? me)
	Comment 6 • 9 years ago

I see that too, Linux & latest nightly, every time when e10s is enabled.

	Martin Stránský [:stransky] (ni? me)
	Comment 7 • 9 years ago

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);

	Trevor Saunders (:tbsaunde)
	Comment 8 • 9 years ago

(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.

	Martin Stránský [:stransky] (ni? me)
	Comment 9 • 9 years ago

(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.

	Martin Stránský [:stransky] (ni? me)
	Comment 10 • 9 years ago

> 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;

	Trevor Saunders (:tbsaunde)
	Comment 11 • 9 years ago

> 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.

	Sylvestre Ledru [:Sylvestre]
	Comment 12 • 9 years ago

Seems like an e10s-only bug, not tracking anymore

	David Bolter [:davidb] (NeedInfo me for attention)
	Comment 13 • 9 years ago

This is still around. Is mchildren garbage?
https://crash-stats.mozilla.com/report/index/223a7259-4338-4069-b111-b0ac02150416

	(Away) Reporter
	Comment 14 • 9 years ago

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?)

	Trevor Saunders (:tbsaunde)
	Comment 15 • 9 years ago

(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.

	David Bolter [:davidb] (NeedInfo me for attention)
	Comment 16 • 9 years ago

(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.

	pedrogfrancisco
	Comment 17 • 9 years ago

I have a screenshot tool installed ( EasyScreenshot 1.2.5.1 ).
Apparently someone else above can get it with SnagIt .

Is it a coincidence?

	Jonathan Howard
	Comment 18 • 9 years ago

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.

	(Away) Reporter
	Comment 19 • 9 years ago

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?

	Jonathan Howard
	Comment 20 • 9 years ago

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

	David Bolter [:davidb] (NeedInfo me for attention)
	Comment 21 • 9 years ago

(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?

	Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)
	Comment 22 • 9 years ago

Added a tracking flag for FF40 as KaiRo mentioned this as a top issue in Dev edition in today's channel meeting.

	(Away) Reporter
	Comment 23 • 9 years ago

This topcrash bug has been sitting idle for a long time. davidb could you help find an owner to actively investigate?

	David Bolter [:davidb] (NeedInfo me for attention)
	Comment 24 • 9 years ago

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.

	Trevor Saunders (:tbsaunde)
	Comment 25 • 9 years ago

(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.

	David Bolter [:davidb] (NeedInfo me for attention)
	Comment 26 • 9 years ago

I think we should turn e10s off for a11y in 40 at least. Jim what is the preferred way to do that?

	Jim Mathies [:jimm]
	Comment 27 • 9 years ago

(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?

	Jim Mathies [:jimm]
	Comment 28 • 9 years ago

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.

	Jim Mathies [:jimm]
	Comment 30 • 9 years ago

Hmm, am I looking at the wrong signature here? ProxyAccessible vs. DocAccessible?

	David Bolter [:davidb] (NeedInfo me for attention)
	Comment 31 • 9 years ago

(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).

	Jim Mathies [:jimm]
	Comment 32 • 9 years ago

(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?

	(Away) Reporter
	Comment 33 • 9 years ago

> 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.

	David Bolter [:davidb] (NeedInfo me for attention)
	Comment 34 • 9 years ago

(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?

	Jonathan Howard
	Comment 35 • 9 years ago

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.)

	Sylvestre Ledru [:Sylvestre]
	Comment 36 • 9 years ago

Stop tracking for 40 as it will move to beta soon and e10s is disabled in beta.

	Tracy Walker [:tracy]
	Comment 37 • 9 years ago

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

	Jim Mathies [:jimm]
	Comment 38 • 9 years ago

top crasher on aurora

	Bill McCloskey [inactive unless it's an emergency] (:billm)
	Comment 39 • 9 years ago

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).

	Trevor Saunders (:tbsaunde)
	Comment 40 • 9 years ago

(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.

	Bill McCloskey [inactive unless it's an emergency] (:billm)
	Comment 41 • 9 years ago

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.

	Bill McCloskey [inactive unless it's an emergency] (:billm)
	Comment 42 • 9 years ago

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.

	Trevor Saunders (:tbsaunde)
	Comment 43 • 9 years ago

> 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 :(

	Bill McCloskey [inactive unless it's an emergency] (:billm)
	Comment 44 • 9 years ago

> 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.

	Trevor Saunders (:tbsaunde)
	Comment 45 • 9 years ago

(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.

	Lorien Hu (:lsocks) Assignee
	Comment 46 • 9 years ago

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.

	Trevor Saunders (:tbsaunde)
	Comment 47 • 9 years ago

> 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.

	Bill McCloskey [inactive unless it's an emergency] (:billm)
	Comment 51 • 9 years ago

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.

	Trevor Saunders (:tbsaunde)
	Comment 52 • 9 years ago

> 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.

	Pulsebot
	Comment 53 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/5f190b448f83

	Ryan VanderMeulen [:RyanVM]
	Comment 54 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/5f190b448f83

	Bill McCloskey [inactive unless it's an emergency] (:billm)
	Comment 55 • 9 years ago

Oops. Still have to land my patch.

	Pulsebot
	Comment 56 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/d72b754ac74f

	Ryan VanderMeulen [:RyanVM]
	Comment 57 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/d72b754ac74f

	Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)
	Comment 58 • 9 years ago

Bill, this is a top-crasher on Aurora. Do you think the patch is stable and should be uplifted to m-a?

	Bill McCloskey [inactive unless it's an emergency] (:billm)
	Comment 59 • 9 years ago

That's up to Trevor, but I think it should be uplifted.

	Tracy Walker [:tracy]
	Comment 60 • 9 years ago

There haven't been any reports of this against Nightly(Fx42) after the fix was landed there on 20150716.

	Trevor Saunders (:tbsaunde)
	Comment 61 • 9 years ago

(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.

	Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)
	Comment 62 • 9 years ago

Untracking e10s issue for Aurora given that it is not enabled by default. Tracked for 42 instead.