Closed Bug 246712 Opened 17 years ago Closed 15 years ago

Right click "This Frame", but then quickly "View Page Source" causes crash [@ 0x00000ad7 - destroyTimerEvent ] [@ nsCachedChromeChannel::HandleStartLoadEvent ]

Categories

(Core :: DOM: Events, defect)

x86
Windows XP
defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: martijn.martijn, Unassigned)

References

()

Details

(Keywords: crash, fixed1.8.1)

Crash Data

Attachments

(3 files, 3 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040608
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040608

On that site (which is quite colorful), you can see an orange/brown iframe with
a scrollbar with the big words: "Giel Beelen".
Right click in this iframe, select "This frame".
Wait until the submenu comes up.
Now quickly click on "View Page Source" --> Crash

This happens in Mozilla1.7 RC3.
Talkback ID: TB94608X

It crashes afaik only on this site this way, although in other framed sites you
cannot access the "This Frame" submenu anymore after you followed previous steps.

Reproducible: Always
Steps to Reproduce:
1. See above
2.
3.

Actual Results:  
Crash

Expected Results:  
No crash
Another Talkback ID: TB95152Q
Severity: normal → critical
0x00690056
destroyTimerEvent 
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/xpcom/threads/nsTimerImpl.cpp,
line 456]
PL_DestroyEvent 
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/xpcom/threads/plevent.c,
line 713]
PL_HandleEvent 
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/xpcom/threads/plevent.c,
line 686]
PL_ProcessPendingEvents 
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/xpcom/threads/plevent.c,
line 612]
nsEventQueueImpl::ProcessPendingEvents 
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/xpcom/threads/nsEventQueue.cpp,
line 395]

Does this still happen on a newer build?  Try 2004-06-14-09, if you can.
It still happens with:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a2) Gecko/20040614
I don't get any Talkback ID though.
I don't seem to get a crash with a fairly recent debug build of Firefox
(althought the regular one still crashes).
I tried it some twenty times or so.
Also get a crash at http://www.webwereld.nl.
I think this crash happens for all sites with frames.
I believe the timing when clicking on "View page source" is important to get the
crash. I get the feeling I particularly get the crash, when I click on the "View
Page source" just before the submenu tends to close.
Here is another Talkback ID: TB180432X
Using in that case:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/2004-06-26
Firefox/0.9.1
Summary: Right click "This Frame", but then quickly "View Page Source" crashes on this site → Right click "This Frame", but then quickly "View Page Source" causes crash
TB180432 not too usefull:
0x01de8f71
SHELL32.dll + 0x520c24 (0x778b0c24)

Martijn Wargers: Try to delete compreg.dat fine in you Mozilla's components
directory before you start Mozilla. Then please try to reproduce and if TalkBack
appear, send incident and look for TalkBack incident ID.
Keywords: crash
(In reply to comment #6)
> Martijn Wargers: Try to delete compreg.dat fine in you Mozilla's components
> directory before you start Mozilla. Then please try to reproduce and if TalkBack
> appear, send incident and look for TalkBack incident ID.

Ok, done that. I've done three crashes, hth: TB227361K, TB227365M and TB227369W.
Using:
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a2) Gecko/20040629
Firefox/0.8.0+

Keywords: talkbackid
Martijn: Due to a recent Talkback database upgrade aren't older incidents
available. Could I ask you again for TalkBack incident ID? I do apologize for
complication.
Keywords: talkbackid
Ok, here are three new TB's: TB320459G, TB320495Q and TB320502Y.
TB320495Q and TB320502Y look like:
0x00000ad7
destroyTimerEvent 
[c:/builds/tinderbox/firefox-0.9.1/WINNT_5.0_Clobber/mozilla/xpcom/threads/nsTimerImpl.cpp,
line 456]
SETUPAPI.DLL + 0x30c24 (0x778b0c24)

TB320459G:
nsCachedChromeChannel::HandleStartLoadEvent 
[c:/builds/tinderbox/firefox-0.9.1/WINNT_5.0_Clobber/mozilla/chrome/src/nsChromeProtocolHandler.cpp,
line 447]
SETUPAPI.DLL + 0x30c24 (0x778b0c24)
Summary: Right click "This Frame", but then quickly "View Page Source" causes crash → Right click "This Frame", but then quickly "View Page Source" causes crash [@ 0x00000ad7 - destroyTimerEvent ] [@ nsCachedChromeChannel::HandleStartLoadEvent ]
please try again.  I could not recreate on 1.7.1, though if I tried this
multiple times eventually the "this frame" popup menu stopped appearing.  But no
crash.
Three new crashes: TB499344E,TB499325Z and TB499307X. Using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.1) Gecko/20040707
I admit, sometimes it can be very hard to get the crash. I have to repeat the
steps 10 times sometimes before I get the crash. Also, like you said, the "this
frame" popup menu sometimes stops working when trying to trigger the crash.

All new stacks look same:
0x00690056
xpcom.dll + 0x29025 (0x61d49025)
xpcom.dll + 0x29957 (0x61d49957)
xpcom.dll + 0x29928 (0x61d49928)
xpcom.dll + 0x2986b (0x61d4986b)
xpcom.dll + 0x27ad1 (0x61d47ad1)
Product: Browser → Seamonkey
Depends on: 268399
I have a better way to get the crash:
- Go to gmail (you've to be logged in and I have the setting 100 conversations
per page)
- Right Click somewhere in the page
- Hover over "This frame", wait until the submenu comes up.
- Now quickly choose "Select All".

Crashes quite reliably for me (50% of the cases) with 2005-09-04 trunk build.
Talkback ID's: TB9077106M TB9077132E
Component: General → DOM: Events
Product: Mozilla Application Suite → Core
Assignee: general → events
QA Contact: general → ian
I think it is a duplicate of Bug 268399: it is exactly the problem described in the bug.
Flags: blocking1.9a1?
Depends on: 279703
Depends on: 302584
anyone still see the problems in comment 0 and/or comment 14?  I was unable to reproduce using
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.2) Gecko/20060313 Firefox/1.5.0.2
I can still reproduce, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060311 Firefox/1.6a1
The problem is that nsMenuPopupFrame uses itself as a timer callback and doesn't cancel the timer in Destroy().  That's wrong.  Of course just using itself as a timer callback is wrong -- see bug 241733.

Martijn, do you want to take a stab at fixing this based on the patch in bug 241733?  If not, that's fine; I'll hunt someone down.  ;)

Changing deps to make this block the other similar-looking crashes.
Blocks: 268399, 302584
No longer depends on: 268399, 302584
Attached patch patch (obsolete) — Splinter Review
I really have no idea what I'm doing here, I more or less copied it from bug 241733 and then fixed all the compiler errors I got.
But it seems to fix the issue, I think.
Attachment #215050 - Flags: review?(bzbarsky)
Comment on attachment 215050 [details] [diff] [review]
patch

>Index: layout/xul/base/src/nsMenuPopupFrame.h
>+ * nsMenuPoupFrame, so that it will exist as long as the timer holds a reference

nsMenuPopupFrame

>+ * to it. The callback is delegated to the contained nsMenuFrame as long as

And here.

Also, I still think we should cancel that timer in Destroy() in addition to this patch (if it's non-null, etc).

r+sr=bzbarsky with that.

Thanks for doing this!
Attachment #215050 - Flags: superreview+
Attachment #215050 - Flags: review?(bzbarsky)
Attachment #215050 - Flags: review+
Attached patch updated patch, fixed comments (obsolete) — Splinter Review
So I guess this is ready for check-in then.
Oh wait, I should "cancel that timer in Destroy() in addition to this patch (if it's non-null, etc)".
Attached patch patchSplinter Review
With:
 nsMenuPopupFrame::Destroy(nsPresContext* aPresContext)
 {
+  if (mCloseTimer)
+    mCloseTimer->Cancel();

Boris, you meant this, right?
Attachment #215050 - Attachment is obsolete: true
Attachment #215060 - Attachment is obsolete: true
Pretty much; I'd call ClearFrame() before calling Cancel(), but other than that looks great.
Attached patch patchSplinter Review
Ok, calling ClearFrame() before calling Cancel() now.
This one is ready for check-in.
Checking in layout/xul/base/src/nsMenuPopupFrame.h;
/cvsroot/mozilla/layout/xul/base/src/nsMenuPopupFrame.h,v  <--  nsMenuPopupFrame
.h
new revision: 1.81; previous revision: 1.80
done
Checking in layout/xul/base/src/nsMenuPopupFrame.cpp;
/cvsroot/mozilla/layout/xul/base/src/nsMenuPopupFrame.cpp,v  <--  nsMenuPopupFra
me.cpp
new revision: 1.277; previous revision: 1.276
done
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Flags: blocking1.9a1?
A few notes, at least for the future:

Please wrap comments at 78 characters or less (I use 72).  Otherwise we'll all keep making our windows a little wider, and a little wider, and a little wider...  80 is a pretty common standard.

~nsMenuPopupTimerMediator should really be private, since it's non-virtual (and should be protected if it were).

It might have made sense to make the constructor and destructor inline; they're simple enough.
*** Bug 340045 has been marked as a duplicate of this bug. ***
I can confirm that this solves the problem, after pretty aggressive testing...

Any chance of this getting merged into the branch?

I'm getting these crashes with the latest Bon Echo 3 - it would be great if this could be solved in Firefox 2.0
Attachment #215050 - Flags: approval-branch-1.8.1?(alex)
Attachment #215050 - Flags: approval-branch-1.8.1?(alex) → approval-branch-1.8.1?(dbaron)
Martijn, is landing this on the branch ok with you?  Normally the patch author should request approval -- on the principle that two people need to agree, both the patch author and the approver.

Also, which patch is it that should go in, if so?
(In reply to comment #30)
> Martijn, is landing this on the branch ok with you? 

Yes, sure. It was on my to-do list, actually. But I was doubting a little, because of the suggestions you made in comment 27. I know those are merely suggestions to make the patch better, but I'm afraid I can't fulfill those suggestions, because I still don't understand how to do those.

> Also, which patch is it that should go in, if so?

The last one I posted.
Attachment #215050 - Flags: approval-branch-1.8.1?(dbaron) → approval1.8.1?
Attached patch patch for 1.8.1 branch (obsolete) — Splinter Review
Comment on attachment 215050 [details] [diff] [review]
patch

plussed approval request on correct patch; clearing this one
Attachment #215050 - Flags: approval1.8.1?
Attachment #226458 - Attachment is obsolete: true
Checking in nsMenuPopupFrame.cpp;
/cvsroot/mozilla/layout/xul/base/src/nsMenuPopupFrame.cpp,v  <--  nsMenuPopupFra
me.cpp
new revision: 1.259.2.5; previous revision: 1.259.2.4
done
Checking in nsMenuPopupFrame.h;
/cvsroot/mozilla/layout/xul/base/src/nsMenuPopupFrame.h,v  <--  nsMenuPopupFrame
.h
new revision: 1.70.24.2; previous revision: 1.70.24.1
done

Checked into 1.8.1 branch with help from dbaron.
Keywords: fixed1.8.1
This bug has returned, and should be reopened TB20252976M , TB20252961W , TB20252955Y
Crash Signature: [@ 0x00000ad7 - destroyTimerEvent ] [@ nsCachedChromeChannel::HandleStartLoadEvent ]
You need to log in before you can comment on or make changes to this bug.