Call RemoveMappingsForFrameSubtree() before destroying an "internal" popup frame. b=372685 r+sr=bzbarsky blocking1.9=dsicore
authormats.palmgren@bredband.net
Sat, 06 Oct 2007 06:53:05 -0700
changeset 6698 e974dbbbfd83c627179bbffa70b6c5fbdfe4a7c2
parent 6697 040bb27b1e41e41d152df8a48481549291b03572
child 6699 89b0919e55d13bb429bf91cd512fa8f51dc6ebda
push idunknown
push userunknown
push dateunknown
bugs372685
milestone1.9a9pre
Call RemoveMappingsForFrameSubtree() before destroying an "internal" popup frame. b=372685 r+sr=bzbarsky blocking1.9=dsicore
layout/xul/Makefile.in
layout/xul/base/src/nsMenuPopupFrame.cpp
layout/xul/base/src/nsPopupSetFrame.cpp
layout/xul/base/src/nsPopupSetFrame.h
layout/xul/test/Makefile.in
layout/xul/test/test_bug372685.xul
--- a/layout/xul/Makefile.in
+++ b/layout/xul/Makefile.in
@@ -39,10 +39,14 @@ DEPTH		= ../..
 topsrcdir	= @top_srcdir@
 srcdir		= @srcdir@
 VPATH		= @srcdir@
 
 include $(DEPTH)/config/autoconf.mk
 
 DIRS		= base
 
+ifdef MOZ_MOCHITEST
+DIRS		+= test
+endif
+
 include $(topsrcdir)/config/rules.mk
 
--- a/layout/xul/base/src/nsMenuPopupFrame.cpp
+++ b/layout/xul/base/src/nsMenuPopupFrame.cpp
@@ -81,26 +81,16 @@
 #include "nsIReflowCallback.h"
 #include "nsBindingManager.h"
 #ifdef XP_WIN
 #include "nsISound.h"
 #endif
 
 const PRInt32 kMaxZ = 0x7fffffff; //XXX: Shouldn't there be a define somewhere for MaxInt for PRInt32
 
-static nsPopupSetFrame*
-GetPopupSetFrame(nsPresContext* aPresContext)
-{
-  nsIRootBox* rootBox = nsIRootBox::GetRootBox(aPresContext->PresShell());
-  if (!rootBox)
-    return nsnull;
-
-  return rootBox->GetPopupSetFrame();
-}
-
 // NS_NewMenuPopupFrame
 //
 // Wrapper for creating a new menu popup container
 //
 nsIFrame*
 NS_NewMenuPopupFrame(nsIPresShell* aPresShell, nsStyleContext* aContext)
 {
   return new (aPresShell) nsMenuPopupFrame (aPresShell, aContext);
@@ -625,17 +615,16 @@ nsMenuPopupFrame::GetLayoutFlags(PRUint3
 //   Retrieves the view for the popup widget that contains the given frame. 
 //   If the given frame is not contained by a popup widget, return the
 //   root view of the root viewmanager.
 nsIView*
 nsMenuPopupFrame::GetRootViewForPopup(nsIFrame* aStartFrame)
 {
   nsIView* view = aStartFrame->GetClosestView();
   NS_ASSERTION(view, "frame must have a closest view!");
-  nsIView* rootView = nsnull;
   while (view) {
     // Walk up the view hierarchy looking for a view whose widget has a 
     // window type of eWindowType_popup - in other words a popup window
     // widget. If we find one, this is the view we want. 
     nsIWidget* widget = view->GetWidget();
     if (widget) {
       nsWindowType wtype;
       widget->GetWindowType(wtype);
--- a/layout/xul/base/src/nsPopupSetFrame.cpp
+++ b/layout/xul/base/src/nsPopupSetFrame.cpp
@@ -18,62 +18,47 @@
  * Netscape Communications Corporation.
  * Portions created by the Initial Developer are Copyright (C) 1998
  * the Initial Developer. All Rights Reserved.
  *
  * Contributor(s):
  *   Original Author: David W. Hyatt (hyatt@netscape.com)
  *   Pierre Phaneuf <pp@ludusdesign.com>
  *   Dean Tessman <dean_tessman@hotmail.com>
+ *   Mats Palmgren <mats.palmgren@bredband.net>
  *
  * Alternatively, the contents of this file may be used under the terms of
  * either of the GNU General Public License Version 2 or later (the "GPL"),
  * or the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
  * in which case the provisions of the GPL or the LGPL are applicable instead
  * of those above. If you wish to allow use of your version of this file only
  * under the terms of either the GPL or the LGPL, and not to allow others to
  * use your version of this file under the terms of the MPL, indicate your
  * decision by deleting the provisions above and replace them with the notice
  * and other provisions required by the GPL or the LGPL. If you do not delete
  * the provisions above, a recipient may use your version of this file under
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
+#include "nsPopupSetFrame.h"
 #include "nsGkAtoms.h"
-#include "nsPopupSetFrame.h"
-#include "nsIMenuParent.h"
-#include "nsMenuFrame.h"
-#include "nsBoxFrame.h"
+#include "nsCOMPtr.h"
 #include "nsIContent.h"
-#include "prtypes.h"
-#include "nsIAtom.h"
 #include "nsPresContext.h"
 #include "nsStyleContext.h"
 #include "nsCSSRendering.h"
 #include "nsINameSpaceManager.h"
-#include "nsMenuPopupFrame.h"
-#include "nsMenuBarFrame.h"
-#include "nsIView.h"
-#include "nsIWidget.h"
 #include "nsIDocument.h"
-#include "nsIDOMNSDocument.h"
-#include "nsIDOMDocument.h"
-#include "nsIDOMXULDocument.h"
-#include "nsIDOMElement.h"
-#include "nsISupportsArray.h"
-#include "nsIDOMText.h"
 #include "nsBoxLayoutState.h"
 #include "nsIScrollableFrame.h"
 #include "nsCSSFrameConstructor.h"
 #include "nsGUIEvent.h"
 #include "nsIRootBox.h"
 
-#define NS_MENU_POPUP_LIST_INDEX   0
-
 nsPopupFrameList::nsPopupFrameList(nsIContent* aPopupContent, nsPopupFrameList* aNext)
 :mNextPopup(aNext), 
  mPopupFrame(nsnull),
  mPopupContent(aPopupContent)
 {
 }
 
 //
@@ -99,16 +84,22 @@ nsPopupSetFrame::Init(nsIContent*      a
   NS_ASSERTION(NS_SUCCEEDED(res), "grandparent should be root box");
   if (NS_SUCCEEDED(res)) {
     rootBox->SetPopupSetFrame(this);
   }
 
   return rv;
 }
 
+nsIAtom*
+nsPopupSetFrame::GetType() const
+{
+  return nsGkAtoms::popupSetFrame;
+}
+
 NS_IMETHODIMP
 nsPopupSetFrame::AppendFrames(nsIAtom*        aListName,
                               nsIFrame*       aFrameList)
 {
   if (aListName == nsGkAtoms::popupList) {
     return AddPopupFrameList(aFrameList);
   }
   return nsBoxFrame::AppendFrames(aListName, aFrameList);
@@ -143,20 +134,22 @@ nsPopupSetFrame::SetInitialChildList(nsI
     return AddPopupFrameList(aChildList);
   }
   return nsBoxFrame::SetInitialChildList(aListName, aChildList);
 }
 
 void
 nsPopupSetFrame::Destroy()
 {
+  nsCSSFrameConstructor* frameConstructor =
+    PresContext()->PresShell()->FrameConstructor();
   // remove each popup from the list as we go.
   while (mPopupList) {
     if (mPopupList->mPopupFrame)
-      mPopupList->mPopupFrame->Destroy();
+      DestroyPopupFrame(frameConstructor, mPopupList->mPopupFrame);
 
     nsPopupFrameList* temp = mPopupList;
     mPopupList = mPopupList->mNextPopup;
     delete temp;
   }
 
   nsIRootBox *rootBox;
   nsresult res = CallQueryInterface(mParent->GetParent(), &rootBox);
@@ -240,17 +233,18 @@ nsPopupSetFrame::RemovePopupFrame(nsIFra
     if (currEntry->mPopupFrame == aPopup) {
       // Remove this entry.
       if (temp)
         temp->mNextPopup = currEntry->mNextPopup;
       else
         mPopupList = currEntry->mNextPopup;
       
       // Destroy the frame.
-      currEntry->mPopupFrame->Destroy();
+      DestroyPopupFrame(PresContext()->PresShell()->FrameConstructor(),
+                        currEntry->mPopupFrame);
 
       // Delete the entry.
       currEntry->mNextPopup = nsnull;
       delete currEntry;
 
       // Break out of the loop.
       break;
     }
@@ -297,8 +291,15 @@ nsPopupSetFrame::AddPopupFrame(nsIFrame*
     NS_ASSERTION(!entry->mPopupFrame, "Leaking a popup frame");
   }
 
   // Set the frame connection.
   entry->mPopupFrame = static_cast<nsMenuPopupFrame *>(aPopup);
   
   return NS_OK;
 }
+
+void
+nsPopupSetFrame::DestroyPopupFrame(nsCSSFrameConstructor* aFc, nsIFrame* aPopup)
+{
+  aFc->RemoveMappingsForFrameSubtree(aPopup);
+  aPopup->Destroy();
+}
--- a/layout/xul/base/src/nsPopupSetFrame.h
+++ b/layout/xul/base/src/nsPopupSetFrame.h
@@ -38,27 +38,22 @@
 
 //
 // nsPopupSetFrame
 //
 
 #ifndef nsPopupSetFrame_h__
 #define nsPopupSetFrame_h__
 
-#include "prtypes.h"
 #include "nsIAtom.h"
-#include "nsCOMPtr.h"
-#include "nsGkAtoms.h"
 
 #include "nsBoxFrame.h"
-#include "nsFrameList.h"
 #include "nsMenuPopupFrame.h"
-#include "nsIMenuParent.h"
-#include "nsITimer.h"
-#include "nsISupportsArray.h"
+
+class nsCSSFrameConstructor;
 
 nsIFrame* NS_NewPopupSetFrame(nsIPresShell* aPresShell, nsStyleContext* aContext);
 
 struct nsPopupFrameList {
   nsPopupFrameList* mNextPopup;  // The next popup in the list.
   nsMenuPopupFrame* mPopupFrame; // Our popup.
   nsIContent* mPopupContent;     // The content element for the <popup> itself.
 
@@ -88,28 +83,29 @@ public:
                                   nsIFrame*       aChildList);
 
     // nsIBox
   NS_IMETHOD DoLayout(nsBoxLayoutState& aBoxLayoutState);
 
   // Used to destroy our popup frames.
   virtual void Destroy();
 
-  virtual nsIAtom* GetType() const { return nsGkAtoms::popupSetFrame; }
+  virtual nsIAtom* GetType() const;
 
 #ifdef DEBUG
   NS_IMETHOD GetFrameName(nsAString& aResult) const
   {
       return MakeFrameName(NS_LITERAL_STRING("PopupSet"), aResult);
   }
 #endif
 
 protected:
 
   nsresult AddPopupFrameList(nsIFrame* aPopupFrameList);
   nsresult AddPopupFrame(nsIFrame* aPopup);
   nsresult RemovePopupFrame(nsIFrame* aPopup);
+  void DestroyPopupFrame(nsCSSFrameConstructor* aFc, nsIFrame* aPopup);
   
   nsPopupFrameList* mPopupList;
 
 }; // class nsPopupSetFrame
 
 #endif
new file mode 100644
--- /dev/null
+++ b/layout/xul/test/Makefile.in
@@ -0,0 +1,51 @@
+#
+# ***** BEGIN LICENSE BLOCK *****
+# Version: MPL 1.1/GPL 2.0/LGPL 2.1
+#
+# The contents of this file are subject to the Mozilla Public License Version
+# 1.1 (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+# http://www.mozilla.org/MPL/
+#
+# Software distributed under the License is distributed on an "AS IS" basis,
+# WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+# for the specific language governing rights and limitations under the
+# License.
+#
+# The Original Code is mozilla.org code.
+#
+# The Initial Developer of the Original Code is
+# Mozilla Foundation.
+# Portions created by the Initial Developer are Copyright (C) 2007
+# the Initial Developer. All Rights Reserved.
+#
+# Contributor(s):
+#
+# Alternatively, the contents of this file may be used under the terms of
+# either of the GNU General Public License Version 2 or later (the "GPL"),
+# or the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+# in which case the provisions of the GPL or the LGPL are applicable instead
+# of those above. If you wish to allow use of your version of this file only
+# under the terms of either the GPL or the LGPL, and not to allow others to
+# use your version of this file under the terms of the MPL, indicate your
+# decision by deleting the provisions above and replace them with the notice
+# and other provisions required by the GPL or the LGPL. If you do not delete
+# the provisions above, a recipient may use your version of this file under
+# the terms of any one of the MPL, the GPL or the LGPL.
+#
+# ***** END LICENSE BLOCK *****
+
+DEPTH		= ../../..
+topsrcdir	= @top_srcdir@
+srcdir		= @srcdir@
+VPATH		= @srcdir@
+relativesrcdir  = layout/xul/test
+
+include $(DEPTH)/config/autoconf.mk
+include $(topsrcdir)/config/rules.mk
+
+_TEST_FILES =	test_bug372685.xul \
+		$(NULL)
+
+libs:: $(_TEST_FILES)
+	$(INSTALL) $^ $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)
new file mode 100644
--- /dev/null
+++ b/layout/xul/test/test_bug372685.xul
@@ -0,0 +1,50 @@
+<?xml version="1.0"?>
+<?xml-stylesheet href="chrome://global/skin" type="text/css"?>
+<?xml-stylesheet href="/tests/SimpleTest/test.css" type="text/css"?>
+<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
+        xmlns:html="http://www.w3.org/1999/xhtml"
+        title="Test for Bug 372685">
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=372685
+-->
+
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+
+<menuitem id="a" style="display: -moz-stack;">
+<box id="b" style="display: -moz-popup; ">
+<box id="c" style="position: fixed;"></box>
+</box>
+</menuitem>
+
+<script class="testbody" type="application/javascript">
+<![CDATA[
+
+function removestyles(i){
+          document.getElementById('a').removeAttribute('style');
+	  var x=document.getElementById('html_body').offsetHeight;
+	  is(0, 0, "this is a crash test, so always ok if we survive this far");
+          SimpleTest.finish();
+}
+function do_test() {
+          setTimeout(removestyles,200);
+}
+
+SimpleTest.waitForExplicitFinish();
+
+]]>
+</script>
+
+<body  id="html_body" xmlns="http://www.w3.org/1999/xhtml">
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=372685">Mozilla Bug 372685</a>
+<p id="display"></p>
+
+<pre id="test">
+</pre>
+<script>
+addLoadEvent(do_test);
+</script>
+</body>
+
+
+</window>