28544 - webshell leaked when type url in URL bar

Status: VERIFIED FIXED

(Core :: Layout: Form Controls, defect, P3)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

DESCRIPTION:  If I type a URL in to the URL bar, it adds a leaked webshell to
the final count of leaked webshells.

STEPS TO REPRODUCE:
 * start mozilla (preferably with XPCOM_MEM_LEAK_LOG=1)
 * type a URL into the URL bar
 * exit

ACTUAL RESULTS:
 * there's one leaked webshell.  This doesn't happen if a URL isn't typed in

EXPECTED RESULTS:
 * no leaked webshells

DOES NOT WORK CORRECTLY ON:
 * Linux, mozilla, my build (from 2000-02-18)

ADDITIONAL INFORMATION:
Looking at the refcount balancer log on the leaked webshell (which I will
attach), the part that seems most suspicious to me is near the end of the log:

1       nsEventListenerManager::HandleEvent(nsIPresContext *, nsEvent *,
nsIDOMEvent **, unsigned int, nsEventStatus *)+0x00000C98
 1       nsEnderEventListener::KeyPress(nsIDOMEvent *)+0x0000045F
  1       nsGfxTextControlFrame::HandleEvent(nsIPresContext *, nsGUIEvent *,
nsEventStatus *)+0x0000066F
   1       nsGfxTextControlFrame::EnterPressed(nsIPresContext *)+0x000000E7
    1       nsFormFrame::OnSubmit(nsIPresContext *, nsIFrame *)+0x0000055A
     1       nsPresContext::GetLinkHandler(nsILinkHandler **)+0x00000066
      1       unsigned int ns_if_addref<nsILinkHandler *>(nsILinkHandler
*)+0x00000023
       1       nsWebShell::AddRef(void)+0x00000066

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: refcount balancer log (with ignore balanced subtrees)

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: refcount balancer log (with ignore balanced subtrees), attached correctly

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I think I have a fix for this bug (which I'll attach).  The problem was (it
seems) that nsFormFrame::OnSubmit had 'return' in lots of places but only had a
release at the end.  I changed it to use nsCOMPtr.

If someone thinks this is the right thing to do, could you review the patch so
that I could check it in either:
 * after the tree opens for M15
 * if you think it's important enough, try to get approval and do it before then

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: proposed fix

Comment 5

[karnaze (gone)]

David, the change looks good to me. I'm adding the beta1 keyword, and if reaches 
PDT+ status you can check it in to M14. I'm also reassigning to you and CCing 
Jar and Rickg who have authority to make it PDT+. This fix should be risk free 
(unless somewhere else there is code doing one too many releases of the link 
handler), but you should run a few of the top 100 sites before checking in.

Comment 6

[leger]

Putting on PDT+ radar for beta1. 

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in 2000-02-21 20:06-0800.

Comment 8

[ckritzer (gone)]

David, would you please verify and comment?  Thank you! -Chris

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Marking verified, based on the "WEBSHELL=" counts in the text window.