57044 - [FIX]crash deref-ing a null pointer in nsWindow::SetFocus

Status: VERIFIED FIXED

(Core :: XUL, defect, P2)

Description

[buster]

I've seen this crash several times in the past several days, using a variety of
debug builds on 2 different WinNT machines.  Sorry, I don't have a particular
URL or reproducable senario.

nsToolkit::IsGuiThread() line 52 + 18 bytes      <-- bogus call through null ptr
nsWindow::SetFocus() line 1607 + 8 bytes
nsEventStateManager::SendFocusBlur() line 2816
nsEventStateManager::SetContentState() line 2496
nsGenericHTMLElement::HandleDOMEventForAnchors() line 1106
nsHTMLAnchorElement::HandleDOMEvent() line 405
nsGenericDOMDataNode::HandleDOMEvent() line 689 + 39 bytes
nsTextNode::HandleDOMEvent() line 253
PresShell::HandleEventInternal() line 4894 + 41 bytes
PresShell::HandleEvent() line 4823 + 25 bytes
nsView::HandleEvent() line 379
nsView::HandleEvent() line 352
nsView::HandleEvent() line 352
nsViewManager2::DispatchEvent() line 1439
HandleEvent() line 68
nsWindow::DispatchEvent() line 682 + 10 bytes
nsWindow::DispatchWindowEvent() line 703
nsWindow::DispatchMouseEvent() line 3895 + 21 bytes
ChildWindow::DispatchMouseEvent() line 4105
nsWindow::ProcessMessage() line 2960 + 24 bytes
nsWindow::WindowProc() line 951 + 27 bytes
USER32! 77e71820()
GetUIEventProperty() line 78 + 3 bytes


The code could probably use a null pointer check:

NS_METHOD nsWindow::SetFocus(void)
{
    nsToolkit* toolkit = (nsToolkit *)mToolkit;     <--- mToolkit is not checked
    if (!toolkit->IsGuiThread()) {                  <--- crash
        MethodInfo info(this, nsWindow::SET_FOCUS);
        toolkit->CallMethod(&info);
        return NS_ERROR_FAILURE;
    }

Comment 1

[buster]

according to lxr, kevin actually added the offending line of code.  maybe the
bug should be assigned to you, kevin?

Comment 2

[rods (gone)]

I can take it.

I agree it would be good to put in a check, but I wonder why it is null, it 
really shouldn't be.

I think this would be a good one to fix for RTM, to prevent further crashes, if 
so, assign it to me.

Comment 3

[Peter Trudelle]

->rods.  thanks Rod!

Comment 4

[rods (gone)]

Here's the patch:

Index: src/windows/nsWindow.cpp
===================================================================
RCS file: /cvsroot/mozilla/widget/src/windows/nsWindow.cpp,v
retrieving revision 3.294.2.1
diff -u -r3.294.2.1 nsWindow.cpp
--- nsWindow.cpp        2000/10/11 21:47:48     3.294.2.1
+++ nsWindow.cpp        2000/10/17 20:34:18
@@ -1604,7 +1604,8 @@
     // be executed on the "gui thread"...
     //
     nsToolkit* toolkit = (nsToolkit *)mToolkit;
-    if (!toolkit->IsGuiThread()) {
+    NS_ASSERTION(toolkit != nsnull, "This should never be null!");
+    if (toolkit != nsnull && !toolkit->IsGuiThread()) {
         MethodInfo info(this, nsWindow::SET_FOCUS);
         toolkit->CallMethod(&info);
         return NS_ERROR_FAILURE;

Comment 5

[saari (gone)]

r = saari

Comment 6

[Marc Attinasi]

Adding crash kwd. for Rod.

Comment 7

[buster]

a=buster (I think I sent out an email already, but forgot to mark the bug)

Comment 8

[Kevin McCluskey (gone)]

Marked rtm+.  Simple check for null pointer that fixes crasher.

Comment 9

[rods (gone)]

checked into trunk

Comment 10

[Phil Peterson]

rtm++

Comment 11

[rods (gone)]

checked into branch

Comment 12

[rods (gone)]

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

Comment 13

[John Morrison]

verified fixed, 2000101909 win2k (after the checkin)