Bug 606642 - Do not remove the [Excess]OverflowContainers properties before destroying its frames. Carefully check after destroying each frame that the property is still alive. r=roc

2013-02-11 03:28:50 +01:00 · 2013-02-11 03:28:50 +01:00 · 7f33878bb1
--- a/layout/generic/crashtests/606642.xhtml
+++ b/layout/generic/crashtests/606642.xhtml
@ -0,0 +1,16 @@
+<html xmlns="http://www.w3.org/1999/xhtml">
+<head>
+<script>
+
+function boom()
+{
+  document.documentElement.offsetHeight;
+  var r = document.getElementById("r");
+  r.parentNode.removeChild(r);
+}
+
+</script>
+</head>
+
+<body onload="boom();"><div style="-moz-column-count: 3; height: 2in;"><div style="position: relative;"><div id="r" style="position: absolute;"><div style="height: 0in;"><div style="height: 5in;"></div><div style="position: absolute; height: 5in;"></div></div></div></div></div></body>
+</html>
--- a/layout/generic/crashtests/crashtests.list
+++ b/layout/generic/crashtests/crashtests.list
@ -383,6 +383,7 @@ load 603510-1.html
 load 604314-1.html
 load 604843.html
 load 605340.html
+load 606642.xhtml
 load 621424-1.html
 load 621841-1.html
 load 645072-1.html
--- a/layout/generic/nsContainerFrame.cpp
+++ b/layout/generic/nsContainerFrame.cpp
@ -216,6 +216,27 @@ nsContainerFrame::DestroyAbsoluteFrames(nsIFrame* aDestructRoot)
  }
 }

+void
+nsContainerFrame::SafelyDestroyFrameListProp(nsIFrame* aDestructRoot,
+                                             FramePropertyTable* aPropTable,
+                                             const FramePropertyDescriptor* aProp)
+{
+  // Note that the last frame can be removed through another route and thus
+  // delete the property -- that's why we fetch the property again before
+  // removing each frame rather than fetching it once and iterating the list.
+  while (nsFrameList* frameList =
+           static_cast<nsFrameList*>(aPropTable->Get(this, aProp))) {
+    nsIFrame* frame = frameList->RemoveFirstChild();
+    if (MOZ_LIKELY(frame)) {
+      frame->DestroyFrom(aDestructRoot);
+    } else {
+      aPropTable->Remove(this, aProp);
+      delete frameList;
+      return;
+    }
+  }
+}
+
 void
 nsContainerFrame::DestroyFrom(nsIFrame* aDestructRoot)
 {
@ -235,15 +256,11 @@ nsContainerFrame::DestroyFrom(nsIFrame* aDestructRoot)
  DestroyOverflowList(prescontext, aDestructRoot);

  if (IsFrameOfType(nsIFrame::eCanContainOverflowContainers)) {
-    nsFrameList* frameList =
-      RemovePropTableFrames(prescontext, OverflowContainersProperty());
-    if (frameList)
-      frameList->DestroyFrom(aDestructRoot);
-
-    frameList = RemovePropTableFrames(prescontext,
-                                      ExcessOverflowContainersProperty());
-    if (frameList)
-      frameList->DestroyFrom(aDestructRoot);
+    FramePropertyTable* props = prescontext->PropertyTable();
+    SafelyDestroyFrameListProp(aDestructRoot, props,
+                               OverflowContainersProperty());
+    SafelyDestroyFrameListProp(aDestructRoot, props,
+                               ExcessOverflowContainersProperty());
  }

  // Destroy the frame and remove the flow pointers
--- a/layout/generic/nsContainerFrame.h
+++ b/layout/generic/nsContainerFrame.h
@ -25,6 +25,9 @@
 #define NS_FRAME_INVALIDATE_ON_MOVE   0x0010 

 class nsOverflowContinuationTracker;
+namespace mozilla {
+class FramePropertyTable;
+}

 // Some macros for container classes to do sanity checking on
 // width/height/x/y values computed during reflow.
@ -514,6 +517,16 @@ protected:
  nsresult SetPropTableFrames(nsPresContext*                 aPresContext,
                              nsFrameList*                   aFrameList,
                              const FramePropertyDescriptor* aProperty);
+
+  /**
+   * Safely destroy the frames on the nsFrameList stored on aProp for this
+   * frame then remove the property and delete the frame list.
+   * Nothing happens if the property doesn't exist.
+   */
+  void SafelyDestroyFrameListProp(nsIFrame* aDestructRoot,
+                                  mozilla::FramePropertyTable* aPropTable,
+                                  const FramePropertyDescriptor* aProp);
+
  // ==========================================================================

  nsFrameList mFrames;
--- a/layout/generic/nsIFrame.h
+++ b/layout/generic/nsIFrame.h
@ -618,6 +618,7 @@ protected:
  virtual void DestroyFrom(nsIFrame* aDestructRoot) = 0;
  friend class nsFrameList; // needed to pass aDestructRoot through to children
  friend class nsLineBox;   // needed to pass aDestructRoot through to children
+  friend class nsContainerFrame; // needed to pass aDestructRoot through to children
 public:

  /**