76715 - crash activating window

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect, P1)

Description

[Chris Waterson]

SUMMARY

Browser crashes activating a window (e.g., via ALT+TAB). To reproduce.

1. Start mozilla and open two browser windows.

2. In window #1, go to http://bugzilla.mozilla.org/enter_bug.cgi
   and set focus to an entryfield.

3. In window #2, go to www.m-w.com, and look up a word, say
   ``star-chamber''.

4. Return to window #1.

5. Return to window #2.

Result: crash. Seen in 2001-04-19 builds. Hyatt says this a regression from a
fix that he put in for google.com or something.

Comment 1

[Chris Waterson]

Attachment: bail in SetFocus() if element doesn't have a document

Comment 2

[Johnny Stenback (:jst)]

I think this is related to something I saw on the XPCDOM branch (due to other
reasons tho), the presshell destructor tears itself down in the wrong order IMO,
on the branch, the presshell is torn down while the content nodes still know
their document and thus tearing down the frame hierarchy caused focus events to
be fired and the content nodes ended up finding deleted frames through the
presshell that was being torn down. Here's what I did to fix a crasher on the
branch:

Index: layout/html/base/src/nsPresShell.cpp
===================================================================
RCS file: /cvsroot/mozilla/layout/html/base/src/nsPresShell.cpp,v
retrieving revision 3.381
retrieving revision 3.381.2.1
diff -u -r3.381 -r3.381.2.1
--- nsPresShell.cpp     2001/03/27 23:59:56     3.381
+++ nsPresShell.cpp     2001/04/19 06:16:34     3.381.2.1
@@ -1403,14 +1403,17 @@
     mViewManager = nsnull;
   }

+  // This shell must be removed from the document before the frame
+  // hierarchy is torn down to avoid finding releted frames through
+  // this presshell while the frames are being torn down
+  if (mDocument) {
+    mDocument->DeleteShell(this);
+  }
+
   // Destroy the frame manager. This will destroy the frame hierarchy
   if (mFrameManager) {
     mFrameManager->Destroy();
     NS_RELEASE(mFrameManager);
-  }
-
-  if (mDocument) {
-    mDocument->DeleteShell(this);
   }

   // We hold a reference to the pres context, and it holds a weak link back

Would this change fix this crasher too? (I don't see a stacktrace so I can't say
for sure.)

If not, don't we need to make your change in a bunch of places, like
nsHTMLSelectElement, nsHTMLTextAreaElement, ...?

Comment 3

[Johnny Stenback (:jst)]

s/releted/deleted/

Comment 4

[Chris Waterson]

Not sure, but we should get this in. hyatt, could you slam dunk this? I am now 
officially in meeting hell.

Comment 5

[Johnny Stenback (:jst)]

r/sr=jst for Chris's fix.

Comment 6

[David Hyatt]

[s]r=hyatt

Comment 7

[Peter Trudelle]

critical for 0.9/p1

Comment 8

[Chris Waterson]

comment

Comment 9

[Asa Dotzler [:asa]]

a=asa (on behalf of drivers) for checkin to 0.9

Comment 10

[Chris Waterson]

Fix checked in, but I incorrectly marked it as for bug 75591. :-(

Comment 11

[Ben Goodger (use ben at mozilla dot org for email)]

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

Comment 12

[lchiang]

verified fixed for win32 2001-05-08 build.
Need to verify for Mac and Linux, or is this a Win32 fix only?

Comment 13

[Sivakiran Tummala]

verified on mac and linux. does not crash. adding the vtrunk keyword

Comment 14

[Sivakiran Tummala]

verified 2001-12-20-10