65243 - nsEventQueueImpl::RevokeEvents does not always revoke pending PL_events

Status: VERIFIED FIXED

(Core :: DOM: UI Events & Focus Handling, defect, P1)

Description

[Kevin McCluskey (gone)]

If the pending event was placed in a mElderQueue, RevokeEvents will not revoke
the event.

This may causing some seemly random crashes because reflow events may come
through for nsPresShell instances that have been destroyed.

This bug is causing periodic failures in a patch I have created to delay
invalidate events using PL_events.

Comment 1

[Kevin McCluskey (gone)]

Added crash keyword.

Comment 2

[Dan M]

Just an unfortunate oversight. This should fix it. Kevin: is there a somewhat 
reliable way to verify?

Index: nsEventQueue.cpp
===================================================================
RCS file: /cvsroot/mozilla/xpcom/threads/nsEventQueue.cpp,v
retrieving revision 3.27
diff -u -r3.27 nsEventQueue.cpp
--- nsEventQueue.cpp	2000/09/13 23:56:13	3.27
+++ nsEventQueue.cpp	2001/01/16 18:49:08
@@ -303,6 +303,11 @@
 nsEventQueueImpl::RevokeEvents(void* owner)
 {
   PL_RevokeEvents(mEventQueue, owner);
+  if (mElderQueue) {
+    nsCOMPtr<nsIEventQueue> elder(do_QueryInterface(mElderQueue));
+    if (elder)
+      elder->RevokeEvents(owner);
+  }
   return NS_OK;
 }

Comment 3

[Kevin McCluskey (gone)]

I don't have an easy and reliable way to verify your fix.

You can install the patch in bug 36849
You should never see the following assertion:
NS_ASSERTION(PR_FALSE, "bad view manager asked to process invalidate event");

In the past, I was able to trigger the assertion by doing a query in bugzilla
then clicking on a bug in the list of bugs displayed by the query. If I hit the
forward and back buttons the assertion was triggered - sometimes. You may have
to run with the patch for several hours to verify that it is no longer a problem.

Comment 4

[Kevin McCluskey (gone)]

The patch looks good.
I'll install it and run with it today. I'll let you know if there are any problems. 

Comment 5

[Kevin McCluskey (gone)]

I applied the patch and I haven't seen any problems with it. I no longer trigger
the assert which used to occur when RevokeEvents was failing.

Comment 6

[Kevin McCluskey (gone)]

The patch for this bug should fix a crash in the FrameManager with this stack trace:

FrameManager::HandlePLEvent(CantRenderReplacedElementEvent * 0x0694ef70) line
939 + 19 bytes
PL_HandleEvent(PLEvent * 0x0694ef70) line 576 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x00ac54e0) line 509 + 9 bytes
_md_EventReceiverProc(HWND__ * 0x007d0286, unsigned int 49331, unsigned int 0,
long 11293920) line 1054 + 9 bytes
USER32! 77e7124c()
00ac54e0()

CC'ing rickg since he added the null check to nsFrameManager in an attempt to
fix the crasher.

//adding a ptr check since talkback is complaining about a crash here.
//I suspect that if the event->owner is really null, bad things will happen
//elsewhere.

Comment 7

[Kevin McCluskey (gone)]

Patch looks good. Works for me. I haven't seen any of the crashes that could be
attributed to RevokeEvents since I installed the patch.
r=[e-mail]

Comment 8

[buster]

sr=buster
Please get this in asap.  I suspect it may be the root cause of other bugs that
people are spending lots of time on, like bug 65800.

Comment 9

[Dan M]

patch is in

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

Bug 66370 may be this same bug - this one may need reopening (it appears the
patch had been applied, and I got the Assert() "bad view manager asked to
process invalidate event" when closing a window.

Comment 11

[Scott Gifford (old account)]

FYI, the patch from this bug, along with the patches from bug 61386 and bug
61756, seem to fix bug 66164.

Comment 12

[Hixie (not reading bugmail)]

Reassigning QA Contact for all open and unverified bugs previously under Lorca's
care to Gerardo as per phone conversation this morning.

Comment 13

[Chris Waterson]

*** Bug 59906 has been marked as a duplicate of this bug. ***

Comment 14

[Madhur Bhatia]

QA contact updated

Comment 15

[jerone]

verified on build 2001-07-24-06-trunk