Bug 17431: Clean house in form and form control destructors to allow reframing r=harishd a=choffman

layout/forms/nsComboboxControlFrame.cpp

@@ -118,7 +118,10 @@ nsComboboxControlFrame::~nsComboboxControlFrame()
   nsCOMPtr<nsIDOMEventReceiver> displayReciever(do_QueryInterface(mDisplayContent));
   displayReciever->RemoveEventListenerByIID((nsIDOMMouseListener *)this, kIDOMMouseListenerIID);
 
-  mFormFrame = nsnull;
+  if (mFormFrame) {
+    mFormFrame->RemoveFormControlFrame(*this);
+    mFormFrame = nsnull;
+  }
   NS_IF_RELEASE(mPresContext);
   NS_IF_RELEASE(mDisplayContent);
   NS_IF_RELEASE(mButtonContent);

layout/forms/nsFileControlFrame.cpp

@@ -88,6 +88,10 @@ nsFileControlFrame::~nsFileControlFrame()
     delete mCachedState;
     mCachedState = nsnull;
   }
+  if (mFormFrame) {
+    mFormFrame->RemoveFormControlFrame(*this);
+    mFormFrame = nsnull;
+  }
 }
 
 NS_IMETHODIMP

layout/forms/nsFormControlFrame.cpp

@@ -71,7 +71,12 @@ nsFormControlFrame::nsFormControlFrame()
 
 nsFormControlFrame::~nsFormControlFrame()
 {
-  mFormFrame = nsnull;
+  if (mFormFrame) {
+    mFormFrame->RemoveFormControlFrame(*this);
+    // This method only removes from radio lists if we are a radio input
+    mFormFrame->RemoveRadioControlFrame(this);
+    mFormFrame = nsnull;
+  }
 }
 
 // Frames are not refcounted, no need to AddRef

layout/forms/nsGfxRadioControlFrame.cpp

@@ -53,7 +53,6 @@ nsGfxRadioControlFrame::nsGfxRadioControlFrame()
 
 nsGfxRadioControlFrame::~nsGfxRadioControlFrame()
 {
- mFormFrame->RemoveRadioControlFrame((nsIFormControlFrame *)this);
  NS_IF_RELEASE(mRadioButtonFaceStyle);
 }
 

layout/forms/nsHTMLButtonControlFrame.cpp

@@ -80,6 +80,14 @@ nsHTMLButtonControlFrame::nsHTMLButtonControlFrame()
   mRenderer.SetNameSpace(kNameSpaceID_None);
 }
 
+nsHTMLButtonControlFrame::~nsHTMLButtonControlFrame()
+{
+  if (mFormFrame) {
+    mFormFrame->RemoveFormControlFrame(*this);
+    mFormFrame = nsnull;
+  }
+}
+
 NS_IMETHODIMP
 nsHTMLButtonControlFrame::Init(nsIPresContext&  aPresContext,
               nsIContent*      aContent,

layout/forms/nsHTMLButtonControlFrame.h

@@ -56,6 +56,7 @@ class nsHTMLButtonControlFrame : public nsHTMLContainerFrame,
 {
 public:
   nsHTMLButtonControlFrame();
+  ~nsHTMLButtonControlFrame();
 
 
 

layout/forms/nsImageControlFrame.cpp

@@ -62,6 +62,7 @@ class nsImageControlFrame : public nsImageControlFrameSuper,
 {
 public:
   nsImageControlFrame();
+  ~nsImageControlFrame();
 
   NS_IMETHOD  QueryInterface(const nsIID& aIID, void** aInstancePtr);
 
@@ -147,6 +148,14 @@ nsImageControlFrame::nsImageControlFrame()
   mFormFrame      = nsnull;
 }
 
+nsImageControlFrame::~nsImageControlFrame()
+{
+  if (mFormFrame) {
+    mFormFrame->RemoveFormControlFrame(*this);
+    mFormFrame = nsnull;
+  }
+}
+
 nsresult
 NS_NewImageControlFrame(nsIFrame** aNewFrame)
 {

layout/forms/nsListControlFrame.cpp

@@ -129,7 +129,10 @@ nsListControlFrame::~nsListControlFrame()
   reciever->RemoveEventListenerByIID((nsIDOMKeyListener *)this, kIDOMKeyListenerIID);
 
   mComboboxFrame = nsnull;
-  mFormFrame = nsnull;
+  if (mFormFrame) {
+    mFormFrame->RemoveFormControlFrame(*this);
+    mFormFrame = nsnull;
+  }
   NS_IF_RELEASE(mPresContext);
   if (mSelectionCache) {
     delete[] mSelectionCache;

layout/html/forms/src/nsComboboxControlFrame.cpp

@@ -118,7 +118,10 @@ nsComboboxControlFrame::~nsComboboxControlFrame()
   nsCOMPtr<nsIDOMEventReceiver> displayReciever(do_QueryInterface(mDisplayContent));
   displayReciever->RemoveEventListenerByIID((nsIDOMMouseListener *)this, kIDOMMouseListenerIID);
 
-  mFormFrame = nsnull;
+  if (mFormFrame) {
+    mFormFrame->RemoveFormControlFrame(*this);
+    mFormFrame = nsnull;
+  }
   NS_IF_RELEASE(mPresContext);
   NS_IF_RELEASE(mDisplayContent);
   NS_IF_RELEASE(mButtonContent);

layout/html/forms/src/nsFileControlFrame.cpp

@@ -88,6 +88,10 @@ nsFileControlFrame::~nsFileControlFrame()
     delete mCachedState;
     mCachedState = nsnull;
   }
+  if (mFormFrame) {
+    mFormFrame->RemoveFormControlFrame(*this);
+    mFormFrame = nsnull;
+  }
 }
 
 NS_IMETHODIMP

layout/html/forms/src/nsFormControlFrame.cpp

@@ -71,7 +71,12 @@ nsFormControlFrame::nsFormControlFrame()
 
 nsFormControlFrame::~nsFormControlFrame()
 {
-  mFormFrame = nsnull;
+  if (mFormFrame) {
+    mFormFrame->RemoveFormControlFrame(*this);
+    // This method only removes from radio lists if we are a radio input
+    mFormFrame->RemoveRadioControlFrame(this);
+    mFormFrame = nsnull;
+  }
 }
 
 // Frames are not refcounted, no need to AddRef

layout/html/forms/src/nsFormFrame.cpp

@@ -148,6 +148,14 @@ nsFormFrame::nsFormFrame()
 nsFormFrame::~nsFormFrame()
 {
   RemoveRadioGroups();
+  PRInt32 numControls = mFormControls.Count();
+  PRInt32 i;
+  // Traverse list from end to 0 -> void array remove method does less work
+  for (i = (numControls-1); i>=0; i--) {
+    nsIFormControlFrame* fcFrame = (nsIFormControlFrame*) mFormControls.ElementAt(i);
+    fcFrame->SetFormFrame(nsnull);
+    mFormControls.RemoveElement(fcFrame);
+  }
 }
 
 PRBool 
@@ -271,12 +279,14 @@ void nsFormFrame::RemoveRadioGroups()
 
 void nsFormFrame::AddFormControlFrame(nsIPresContext& aPresContext, nsIFrame& aFrame)
 {
+  // Make sure we have a form control
   nsIFormControlFrame* fcFrame = nsnull;
   nsresult result = aFrame.QueryInterface(kIFormControlFrameIID, (void**)&fcFrame);
   if ((NS_OK != result) || (nsnull == fcFrame)) {
     return;
   }
 
+  // Get this control's form frame and add this control to it
   nsCOMPtr<nsIContent> iContent;
   result = aFrame.GetContent(getter_AddRefs(iContent));
   if (NS_SUCCEEDED(result) && iContent) {
@@ -306,6 +316,12 @@ void nsFormFrame::AddFormControlFrame(nsIPresContext& aPresContext, nsIFrame& aF
   }
 }
 
+void nsFormFrame::RemoveFormControlFrame(nsIFormControlFrame& aFrame)
+{
+  // Remove form control from array
+  mFormControls.RemoveElement(&aFrame);
+}
+
 nsresult nsFormFrame::GetRadioInfo(nsIFormControlFrame* aFrame,
                                    nsString& aName,
                                    nsRadioControlGroup *& aGroup)
@@ -343,6 +359,7 @@ void nsFormFrame::AddFormControlFrame(nsIPresContext* aPresContext, nsIFormContr
   aFrame.GetType(&type);
 
   // a solo text control can be a submitter (if return is hit)
+  // XXX pollmann this logic is flawed - three text boxes?
   if ((NS_FORM_INPUT_TEXT == type) || (NS_FORM_INPUT_PASSWORD == type)) {
     mTextSubmitter = (nsnull == mTextSubmitter) ? &aFrame : nsnull;
     return;
@@ -372,14 +389,9 @@ void nsFormFrame::AddFormControlFrame(nsIPresContext* aPresContext, nsIFormContr
     }
   }
 }
-  
+
 void nsFormFrame::RemoveRadioControlFrame(nsIFormControlFrame * aFrame)
 {
-
-  // determine which radio buttons belong to which radio groups, unnamed radio buttons
-  // don't go into any group since they can't be submitted. Determine which controls
-  // are capable of form submission.
-
   PRInt32 type;
   aFrame->GetType(&type);
 

layout/html/forms/src/nsFormFrame.h

@@ -60,6 +60,7 @@ public:
     
   void AddFormControlFrame(nsIPresContext* aPresContext, nsIFormControlFrame& aFrame);
   static void AddFormControlFrame(nsIPresContext& aPresContext, nsIFrame& aFrame);
+  void RemoveFormControlFrame(nsIFormControlFrame& aFrame);
   void RemoveRadioControlFrame(nsIFormControlFrame * aFrame);
   nsresult GetRadioInfo(nsIFormControlFrame* aFrame, nsString& aName, nsRadioControlGroup *& aGroup);
 

layout/html/forms/src/nsGfxRadioControlFrame.cpp

@@ -53,7 +53,6 @@ nsGfxRadioControlFrame::nsGfxRadioControlFrame()
 
 nsGfxRadioControlFrame::~nsGfxRadioControlFrame()
 {
- mFormFrame->RemoveRadioControlFrame((nsIFormControlFrame *)this);
  NS_IF_RELEASE(mRadioButtonFaceStyle);
 }
 

layout/html/forms/src/nsHTMLButtonControlFrame.cpp

@@ -80,6 +80,14 @@ nsHTMLButtonControlFrame::nsHTMLButtonControlFrame()
   mRenderer.SetNameSpace(kNameSpaceID_None);
 }
 
+nsHTMLButtonControlFrame::~nsHTMLButtonControlFrame()
+{
+  if (mFormFrame) {
+    mFormFrame->RemoveFormControlFrame(*this);
+    mFormFrame = nsnull;
+  }
+}
+
 NS_IMETHODIMP
 nsHTMLButtonControlFrame::Init(nsIPresContext&  aPresContext,
               nsIContent*      aContent,

layout/html/forms/src/nsHTMLButtonControlFrame.h

@@ -56,6 +56,7 @@ class nsHTMLButtonControlFrame : public nsHTMLContainerFrame,
 {
 public:
   nsHTMLButtonControlFrame();
+  ~nsHTMLButtonControlFrame();
 
 
 

layout/html/forms/src/nsImageControlFrame.cpp

@@ -62,6 +62,7 @@ class nsImageControlFrame : public nsImageControlFrameSuper,
 {
 public:
   nsImageControlFrame();
+  ~nsImageControlFrame();
 
   NS_IMETHOD  QueryInterface(const nsIID& aIID, void** aInstancePtr);
 
@@ -147,6 +148,14 @@ nsImageControlFrame::nsImageControlFrame()
   mFormFrame      = nsnull;
 }
 
+nsImageControlFrame::~nsImageControlFrame()
+{
+  if (mFormFrame) {
+    mFormFrame->RemoveFormControlFrame(*this);
+    mFormFrame = nsnull;
+  }
+}
+
 nsresult
 NS_NewImageControlFrame(nsIFrame** aNewFrame)
 {

layout/html/forms/src/nsListControlFrame.cpp

@@ -129,7 +129,10 @@ nsListControlFrame::~nsListControlFrame()
   reciever->RemoveEventListenerByIID((nsIDOMKeyListener *)this, kIDOMKeyListenerIID);
 
   mComboboxFrame = nsnull;
-  mFormFrame = nsnull;
+  if (mFormFrame) {
+    mFormFrame->RemoveFormControlFrame(*this);
+    mFormFrame = nsnull;
+  }
   NS_IF_RELEASE(mPresContext);
   if (mSelectionCache) {
     delete[] mSelectionCache;