21970 - [DOGFOOD][CRASH] in nsGlobalWindow.cpp when switching between mail folders

Status: VERIFIED FIXED

(MailNews Core :: Backend, defect, P3)

Description

[(not reading, please use [e-mail] instead)]

to reproduce do this:

I have two imap accounts in 5.0, remember password is off for the second.
log on to the first imap server, check the inbox.
switch the second server's inbox, and the password dialog comes up.
click cancel (you'll have to do this twice) click back on the first
server's inbox.  crash.

stack trace coming soon.

the following patch will fix it:
Index: dom/src/base/nsGlobalWindow.cpp
===================================================================
RCS file:
/cvsroot/mozilla/dom/src/base/nsGlobalWindow.cpp,v
retrieving revision 1.193
diff -p -r1.193 nsGlobalWindow.cpp
*** nsGlobalWindow.cpp  1999/12/15 03:59:38
  1.193
--- nsGlobalWindow.cpp  1999/12/16 21:44:05
***************
GlobalWindowImpl::GetPrivateParent(nsPID
*** 3535,3540 ****
--- 3535,3542 ----

         NS_ENSURE_SUCCESS(chromeElement->GetDocument(*getter_AddRefs(doc)),

          NS_ERROR_FAILURE);

+        if (!doc) return NS_ERROR_FAILURE;
+

         nsCOMPtr<nsIScriptGlobalObject> globalObject;

doc->GetScriptGlobalObject(getter_AddRefs(globalObject));

NS_ENSURE_TRUE(globalObject, NS_ERROR_FAILURE);

Comment 1

[(not reading, please use [e-mail] instead)]

Attachment: here's the stack trace.

Comment 2

[(not reading, please use [e-mail] instead)]

travis, this looks like another case of using NS_ENSURE_SUCCESS where rv is
NS_OK, but null is value of what comes back.

Comment 3

[travis]

Well really a case of checking the return of the function when all I really
cared about was getting a valid pointer back.  Since nsnull is a valid
document.  Is this an M12 holder or can this land when the tree opens?

Comment 4

[travis]

Actually, this function is broken in a few other ways.  null should be returned
from the function if there isn't a parent....  If the doc fails, then this
should not return failure but a null parent.

Comment 5

[(not reading, please use [e-mail] instead)]

I'll leave it to travis to fix the function.
it sounds like there is more problems.

but since there is a crasher here, any objections to checking in the crasher fix
now?

Comment 6

[travis]

The new function looks a little something like this....

NS_IMETHODIMP
GlobalWindowImpl::GetPrivateParent(nsPIDOMWindow** aParent)
{
   nsCOMPtr<nsIDOMWindow> parent;
   *aParent = nsnull; // Set to null so we can bail out later

   GetParent(getter_AddRefs(parent));

   if(NS_STATIC_CAST(nsIDOMWindow*, this) == parent.get())
      {
      nsCOMPtr<nsIContent> chromeElement(do_QueryInterface
(mChromeEventHandler));
      if(!chromeElement)
         return NS_OK; // This is ok, just means a null parent.

      nsCOMPtr<nsIDocument> doc;
      chromeElement->GetDocument(*getter_AddRefs(doc));
      if(!doc)
         return NS_OK; // This is ok, just means a null parent.

      nsCOMPtr<nsIScriptGlobalObject> globalObject;
      doc->GetScriptGlobalObject(getter_AddRefs(globalObject));
      if(!globalObject)
         return NS_OK; // This is ok, just means a null parent.

      parent = do_QueryInterface(globalObject);
      }

   if(parent)
      CallQueryInterface(parent.get(), aParent;

   return NS_OK;
}

Comment 7

[leger]

Putting on the PDT- radar.  Spoke with spitzer.  Cool with pdt-....also, he'd
like to see this on the trunk...not the M12 branch...can this be marked M13?

Comment 8

[travis]

Fixed

Comment 9

[laurel]

OK using:
 2000-02-09-08m14 commercial build on linux rh6.0
 2000-02-09-08m14 commercial build on NT 4.0

Comment 10

[laurel]

Can't verify on mac because remember password is broken.  See bug #27160.

Comment 11

[laurel]

I verified this for mac using the master password scheme.
One IMAP server remembered/listed in password manager.
One IMAP server not  remembered/listed in password manager.

OK using 2000-03-07-09m15 commercial build on mac OS 9.0