337586 - Crash [@ PresShell::InitialReflow] when window gets destroyed duren beforeunload event handler, pressing reload

Status: RESOLVED FIXED

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

Description

[Martijn Wargers (dead)]

See upcoming testcase, which crashes Mozilla when pressing the reload button in a popup window.

Talkback ID: TB18548174M
PresShell::InitialReflow   nsContentSink::StartLayout  

I haven't figured out the regression range, yet.

Comment 1

[Martijn Wargers (dead)]

Attachment: popup window needed for testcase

Comment 2

[Martijn Wargers (dead)]

Attachment: testcase

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Attachment: proposed patch

I think this should be enough.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 221789 [details] [diff] [review]
proposed patch

bah, no. The PresShell gets deleted during InitialReflow. :(
Will do a better patch

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Attachment: better patch

Keeping the presshell alive for a bit longer.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 221798 [details] [diff] [review]
better patch

>Index: layout/base/nsPresShell.cpp

>+  nsCOMPtr<nsIPresShell> kungFuDeathGrip(this);

File a followup bug on undoing this and make it dependent on roc's bug about running script at safe times, please?
>+    // Something in the mFrameConstructor->ContentInserted 

s/the//

>@@ -2802,16 +2807,17 @@ PresShell::InitialReflow(nscoord 
> #endif
>+
>     nsRect                bounds = 

Please undo that change?

r+sr=bzbarsky with that.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

Checking in content/base/src/nsContentSink.cpp;
/cvsroot/mozilla/content/base/src/nsContentSink.cpp,v  <--  nsContentSink.cpp
new revision: 1.53; previous revision: 1.52
done
Checking in content/html/document/src/nsMediaDocument.cpp;
/cvsroot/mozilla/content/html/document/src/nsMediaDocument.cpp,v  <--  nsMediaDocument.cpp
new revision: 1.34; previous revision: 1.33
done
Checking in content/xul/document/src/nsXULDocument.cpp;
/cvsroot/mozilla/content/xul/document/src/nsXULDocument.cpp,v  <--  nsXULDocument.cpp
new revision: 1.715; previous revision: 1.714
done
Checking in layout/base/nsPresShell.cpp;
/cvsroot/mozilla/layout/base/nsPresShell.cpp,v  <--  nsPresShell.cpp
new revision: 3.917; previous revision: 3.916
done

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Smaug: I'm trying to reproduce this, do you remember how ContentInserted could end up firing beforeunload?

Trying to figure out what types of protections I need to add in bug 401155

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

(In reply to comment #8)
> Smaug: I'm trying to reproduce this, do you remember how ContentInserted could
> end up firing beforeunload?
I don't remember that now. And this was fixed long time ago. So many other
things have changed since then.