169595 - Keychain causes crash when importing bookmarks

Status: VERIFIED FIXED

(Camino Graveyard :: Bookmarks, defect)

Description

[Simon Fraser [no longer active]]

We do funky stuff when importing bookmarks, and that upset keychain:

Exception:  EXC_BAD_ACCESS (0x0001)
Codes:      KERN_INVALID_ADDRESS (0x0001) at 0x43300000

Thread 0 Crashed:
 #0   0x000427e8 in -[KeychainBrowserListener onLoadingCompleted:]
 #1   0x0003f8cc in
_ZN17CHBrowserListener13OnStateChangeEP14nsIWebProgressP10nsIRequestjj
 #2   0x0039f740 in
_ZN15nsDocLoaderImpl17FireOnStateChangeEP14nsIWebProgressP10nsIRequestij
 #3   0x0039ec20 in _ZN15nsDocLoaderImpl16DocLoaderIsEmptyEv
 #4   0x0039ea40 in _ZN15nsDocLoaderImpl13OnStopRequestEP10nsIRequestP11nsISupportsj
 #5   0x0013b880 in _ZN11nsLoadGroup13RemoveRequestEP10nsIRequestP11nsISupportsj
 #6   0x002e7da8 in _ZN9PresShell24RemoveDummyLayoutRequestEv
 #7   0x002e77c0 in _ZN9PresShell21ProcessReflowCommandsEi
 #8   0x0079da40 in _ZN11ReflowEvent11HandleEventEv

Comment 1

[Mike Pinkerton (not reading bugmail)]

the reason here is that we "destroy" the webshell, but don't release it so when
we later ask for its document, it gives us back garbage. i'm in the middle of a
patch that will clean this up, and also fixes several potential leaks.

Comment 2

[Mike Pinkerton (not reading bugmail)]

Attachment: diff -w, fix up _webBrowser in CHBrowserView

here's a patch that releases _webBrowser when we destroy the web browser rather
that waiting for dealloc. there was also a rather nasty situation lurking if
anyone called setWebBrowser as we didn't actually take ownership. It appears
like nobody calls it though.

the diff is -w which is why nothing lines up ;)

i'd like some scrutiny on this patch as it changes some fundamental behaviors.
i've verified in the debugger that we always destroy the web browser explicitly
so it does get released.

Comment 3

[Mike Pinkerton (not reading bugmail)]

slightly modified patch landed.

Comment 4

[Chris Petersen]

Verified in the 2002-09-20-04 NB.