281157 - [FIX]Deleting mails marked as spam crashes Mozilla [@ nsCOMPtr<nsIRequest>::assign_assuming_AddRef] [@ nsCOMPtr_base::assign_with_AddRef]

Status: VERIFIED FIXED

(Core :: Layout, defect, P1)

Description

[Frank Wein [:mcsmurf]]

To reproduce:
1. Move (or better copy, so you can reproduce it again) some mails (i used 5) to
a seperate folder and mark those as spam
2. Select the mail at the top and hold the Del key, so all mails get deleted.
3. Observe crash when last mail is deleted

Stacktrace (with debug build):
nsCOMPtr<nsIRequest>::assign_assuming_AddRef(nsIRequest * 0x00000000) line 568 +
3 bytes
nsCOMPtr<nsIRequest>::assign_with_AddRef(nsISupports * 0x00000000) line 1225
nsCOMPtr<nsIRequest>::operator=(nsIRequest * 0x00000000) line 714
PresShell::RemoveDummyLayoutRequest() line 6484
PresShell::DoneRemovingReflowCommands() line 6437
PresShell::ProcessReflowCommands(int 0x00000001) line 6345
PresShell::WillPaint(PresShell * const 0x05eb5410) line 6050
nsViewManager::DispatchEvent(nsViewManager * const 0x05e57378, nsGUIEvent *
0x0012f1f8, nsEventStatus * 0x0012f0dc) line 2010
HandleEvent(nsGUIEvent * 0x0012f1f8) line 174
nsWindow::DispatchEvent(nsWindow * const 0x05b8b4bc, nsGUIEvent * 0x0012f1f8,
nsEventStatus & nsEventStatus_eIgnore) line 1114 + 10 bytes
nsWindow::DispatchWindowEvent(nsGUIEvent * 0x0012f1f8, nsEventStatus &
nsEventStatus_eIgnore) line 1140
nsWindow::OnPaint(HDC__ * 0x00000000) line 5170 + 28 bytes
nsWindow::ProcessMessage(unsigned int 0x0000000f, unsigned int 0x00000000, long
0x00000000, long * 0x0012f734) line 3946 + 19 bytes
nsWindow::WindowProc(HWND__ * 0x001f01fc, unsigned int 0x0000000f, unsigned int
0x00000000, long 0x00000000) line 1400 + 27 bytes
USER32! 77e2a420()
USER32! 77e04750()
USER32! 77e055b0()
NTDLL! 7789ff57()
USER32! 77e0ded9()
USER32! 77e1e6c5()
nsWindow::ProcessMessage(unsigned int 0x00000100, unsigned int 0x0000002e, long
0x41530004, long * 0x0012fcd0) line 4056
nsWindow::WindowProc(HWND__ * 0x000c020e, unsigned int 0x00000100, unsigned int
0x0000002e, long 0x41530004) line 1400 + 27 bytes
USER32! 77e2a420()
USER32! 77e04605()
USER32! 77e0a7ba()
nsAppStartup::Run(nsAppStartup * const 0x010b2dc0) line 208
main1(int 0x00000001, char * * 0x00262638, nsISupports * 0x0105ce18) line 1324 +
32 bytes
main(int 0x00000001, char * * 0x00262638) line 1811 + 37 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 77e9893d()

Comment 1

[Boris Zbarsky [:bzbarsky]]

Is this still a problem in a current build?

Comment 2

[Frank Wein [:mcsmurf]]

(In reply to comment #1)
> Is this still a problem in a current build?

Yes, it does (just tested with CVS trunk build).

Comment 3

[Boris Zbarsky [:bzbarsky]]

OK....  I just tried reproducing this on Linux again, and it worksforme.  So
likely, this is Windows-only (which makes sense given the widget stuff on the
stack).

Is this a regression?  Does this crash a build from Jan 19, for example?  What
about Jan 20?

Comment 4

[Frank Wein [:mcsmurf]]

(In reply to comment #3)
> Is this a regression?  Does this crash a build from Jan 19, for example?  What
> about Jan 20?

Yes, this is a regression. And good guess :), doesn't crash  with Jan 19,
crashes with Jan 20.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Something to try

Does this fix the crash for you?

Comment 6

[Frank Wein [:mcsmurf]]

(In reply to comment #5)
> Created an attachment (id=174603) [edit]

Yes, fixes the crash.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 174603 [details] [diff] [review]
Something to try

OK, so the problem here is that the WillPaint() notification could end up
firing the onload event (via removing the dummy layout request).  Depending on
what onload did, this could mean trouble.

This patch just puts removal of the dummy layout request off on an event after
reflow is completed.  That should also prevent the onload event firing while
we're still in the tail end of ProcessReflowCommands (before DidDoReflow).

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 174603 [details] [diff] [review]
Something to try

Robert, if you get a chance to get to this before David does, could you just
review it too?

Comment 9

[Boris Zbarsky [:bzbarsky]]

Fixed.

Comment 10

[Simon Fraser [no longer active]]

Looks like this is causing crashes on Mac: bug 284354.

Comment 11

[Frank Wein [:mcsmurf]]

V. with a current CVS trunk build, it doesn't crash anymore.