Bug 455314. nsSVGRenderingObserver::GetReferencedFrame shouldn't try to look up the frame on a cache miss, if we're currently destroying frames. r+sr=mats
authorRobert O'Callahan <robert@ocallahan.org>
Fri, 12 Dec 2008 21:25:16 +1300
changeset 22726 b2355b2d980104fc6aaf05db3be0e90957dbe282
parent 22725 b77c1308a6d62321e30c5db3b03e4c25a1118da5
child 22727 b7d995725e1469269e01e1ae3f9197746446f558
push id1
push usershaver@mozilla.com
push dateTue, 04 Jan 2011 17:58:04 +0000
bugs455314
milestone1.9.2a1pre
Bug 455314. nsSVGRenderingObserver::GetReferencedFrame shouldn't try to look up the frame on a cache miss, if we're currently destroying frames. r+sr=mats
layout/base/nsFrameManager.cpp
layout/base/nsFrameManagerBase.h
layout/svg/base/src/nsSVGEffects.cpp
layout/svg/base/src/nsSVGIntegrationUtils.cpp
layout/svg/crashtests/455314-1.xhtml
layout/svg/crashtests/crashtests.list
--- a/layout/base/nsFrameManager.cpp
+++ b/layout/base/nsFrameManager.cpp
@@ -687,32 +687,31 @@ nsFrameManager::InsertFrames(nsIFrame*  
   return aParentFrame->InsertFrames(aListName, aPrevFrame, aFrameList);
 }
 
 nsresult
 nsFrameManager::RemoveFrame(nsIFrame*       aParentFrame,
                             nsIAtom*        aListName,
                             nsIFrame*       aOldFrame)
 {
-#ifdef DEBUG  
   PRBool wasDestroyingFrames = mIsDestroyingFrames;
   mIsDestroyingFrames = PR_TRUE;
-#endif
+
   // In case the reflow doesn't invalidate anything since it just leaves
   // a gap where the old frame was, we invalidate it here.  (This is
   // reasonably likely to happen when removing a last child in a way
   // that doesn't change the size of the parent.)
   // This has to sure to invalidate the entire overflow rect; this
   // is important in the presence of absolute positioning
   aOldFrame->Invalidate(aOldFrame->GetOverflowRect());
 
   nsresult rv = aParentFrame->RemoveFrame(aListName, aOldFrame);
-#ifdef DEBUG  
+
   mIsDestroyingFrames = wasDestroyingFrames;
-#endif
+
   return rv;
 }
 
 //----------------------------------------------------------------------
 
 void
 nsFrameManager::NotifyDestroyingFrame(nsIFrame* aFrame)
 {
--- a/layout/base/nsFrameManagerBase.h
+++ b/layout/base/nsFrameManagerBase.h
@@ -62,26 +62,27 @@ class nsPlaceholderFrame;
 class nsIFrame;
 class nsStyleContext;
 class nsIAtom;
 class nsStyleChangeList;
 class nsILayoutHistoryState;
 
 class nsFrameManagerBase
 {
+public:
+  PRBool IsDestroyingFrames() { return mIsDestroyingFrames; }
+
 protected:
   class UndisplayedMap;
 
   // weak link, because the pres shell owns us
   nsIPresShell*                   mPresShell;
   // the pres shell owns the style set
   nsStyleSet*                     mStyleSet;
   nsIFrame*                       mRootFrame;
   PLDHashTable                    mPrimaryFrameMap;
   PLDHashTable                    mPlaceholderMap;
   UndisplayedMap*                 mUndisplayedMap;
   PRPackedBool                    mIsDestroying;        // The frame manager is being destroyed.
-#ifdef DEBUG
   PRPackedBool                    mIsDestroyingFrames;  // The frame manager is destroying some frame(s).
-#endif
 };
 
 #endif
--- a/layout/svg/base/src/nsSVGEffects.cpp
+++ b/layout/svg/base/src/nsSVGEffects.cpp
@@ -1,8 +1,9 @@
+
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* ***** 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/
@@ -38,16 +39,17 @@
 #include "nsSVGEffects.h"
 #include "nsISupportsImpl.h"
 #include "nsSVGOuterSVGFrame.h"
 #include "nsSVGFilterFrame.h"
 #include "nsSVGClipPathFrame.h"
 #include "nsSVGMaskFrame.h"
 #include "nsSVGTextPathFrame.h"
 #include "nsCSSFrameConstructor.h"
+#include "nsFrameManager.h"
 
 NS_IMPL_ISUPPORTS1(nsSVGRenderingObserver, nsIMutationObserver)
 
 nsSVGRenderingObserver::nsSVGRenderingObserver(nsIURI *aURI,
                                                nsIFrame *aFrame)
   : mElement(this), mFrame(aFrame),
     mFramePresShell(aFrame->PresContext()->PresShell()),
     mReferencedFrame(nsnull),
@@ -76,23 +78,26 @@ nsSVGRenderingObserver::GetReferencedFra
   if (mReferencedFrame && !mReferencedFramePresShell->IsDestroying()) {
     NS_ASSERTION(mElement.get() &&
                  static_cast<nsGenericElement*>(mElement.get())->GetPrimaryFrame() == mReferencedFrame,
                  "Cached frame is incorrect!");
     return mReferencedFrame;
   }
 
   if (mElement.get()) {
-    nsIFrame *frame =
-      static_cast<nsGenericElement*>(mElement.get())->GetPrimaryFrame();
-    if (frame) {
-      mReferencedFrame = frame;
-      mReferencedFramePresShell = mReferencedFrame->PresContext()->PresShell();
-      nsSVGEffects::AddRenderingObserver(mReferencedFrame, this);
-      return frame;
+    nsIDocument* doc = mElement.get()->GetCurrentDoc();
+    nsIPresShell* shell = doc ? doc->GetPrimaryShell() : nsnull;
+    if (shell && !shell->FrameManager()->IsDestroyingFrames()) {
+      nsIFrame* frame = shell->GetPrimaryFrameFor(mElement.get());
+      if (frame) {
+        mReferencedFrame = frame;
+        mReferencedFramePresShell = shell;
+        nsSVGEffects::AddRenderingObserver(mReferencedFrame, this);
+        return frame;
+      }
     }
   }
   return nsnull;
 }
 
 nsIFrame*
 nsSVGRenderingObserver::GetReferencedFrame(nsIAtom* aFrameType, PRBool* aOK)
 {
--- a/layout/svg/base/src/nsSVGIntegrationUtils.cpp
+++ b/layout/svg/base/src/nsSVGIntegrationUtils.cpp
@@ -136,19 +136,27 @@ nsSVGIntegrationUtils::ComputeFrameEffec
 nsRect
 nsSVGIntegrationUtils::GetInvalidAreaForChangedSource(nsIFrame* aFrame,
                                                       const nsRect& aInvalidRect)
 {
   // Don't bother calling GetEffectProperties; the filter property should
   // already have been set up during reflow/ComputeFrameEffectsRect
   nsIFrame* firstFrame =
     nsLayoutUtils::GetFirstContinuationOrSpecialSibling(aFrame);
+  nsSVGEffects::EffectProperties effectProperties =
+    nsSVGEffects::GetEffectProperties(firstFrame);
+  if (!effectProperties.mFilter)
+    return aInvalidRect;
   nsSVGFilterFrame* filterFrame = nsSVGEffects::GetFilterFrame(firstFrame);
-  if (!filterFrame)
-    return aInvalidRect;
+  if (!filterFrame) {
+    // The frame is either not there or not currently available,
+    // perhaps because we're in the middle of tearing stuff down.
+    // Be conservative.
+    return aFrame->GetOverflowRect();
+  }
 
   PRInt32 appUnitsPerDevPixel = aFrame->PresContext()->AppUnitsPerDevPixel();
   nsRect userSpaceRect = GetNonSVGUserSpace(firstFrame);
   nsPoint offset = aFrame->GetOffsetTo(firstFrame) - userSpaceRect.TopLeft();
   nsRect r = aInvalidRect + offset;
   r.ScaleRoundOutInverse(appUnitsPerDevPixel);
   r = filterFrame->GetInvalidationBBox(firstFrame, r);
   r.ScaleRoundOut(appUnitsPerDevPixel);
new file mode 100644
--- /dev/null
+++ b/layout/svg/crashtests/455314-1.xhtml
@@ -0,0 +1,16 @@
+<html xmlns="http://www.w3.org/1999/xhtml"><head>
+<script>
+function doe() {
+document.getElementById('a').appendChild(document.body);
+}
+setTimeout(doe, 100);
+</script>
+</head>
+<body>
+<div style="position: absolute;  -moz-appearance: button; filter: url(#b);  "></div>
+<pre style="position: absolute;">
+<table id="b"></table>
+</pre>
+</body>
+<div id="a"/>
+</html>
\ No newline at end of file
--- a/layout/svg/crashtests/crashtests.list
+++ b/layout/svg/crashtests/crashtests.list
@@ -48,9 +48,10 @@ load 386475-1.xhtml
 load 386566-1.svg
 load 387290-1.svg
 load 402408-1.svg
 load 404677-1.xhtml
 load 409565-1.xhtml
 load 409573-1.svg
 load 429774-1.svg
 load 441368-1.svg
+load 455314-1.xhtml
 load extref-test-1.xhtml