From d9338f796cd947f5ce09f70b431bba00892960e5 Mon Sep 17 00:00:00 2001 From: Mats Palmgren Date: Mon, 8 Dec 2008 00:11:37 +0100 Subject: [PATCH] Don't crash if views were destroyed while flushing notifications. b=421839 r+sr=roc --- layout/generic/nsFrameSelection.h | 4 +++- layout/generic/nsSelection.cpp | 25 ++++++++++++++++++++++--- view/src/nsViewManager.cpp | 20 ++++++++++++++++---- 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/layout/generic/nsFrameSelection.h b/layout/generic/nsFrameSelection.h index a4dbfd617f6..844ec62873d 100644 --- a/layout/generic/nsFrameSelection.h +++ b/layout/generic/nsFrameSelection.h @@ -297,6 +297,7 @@ public: * @param aPoint is relative to the view. * @param aDelay is the timer's interval. */ + /*unsafe*/ nsresult StartAutoScrollTimer(nsIView *aView, nsPoint aPoint, PRUint32 aDelay); @@ -352,6 +353,7 @@ public: * @param aIsSynchronous when PR_TRUE, scrolls the selection into view * at some point after the method returns.request which is processed */ + /*unsafe*/ nsresult ScrollSelectionIntoView(SelectionType aType, SelectionRegion aRegion, PRBool aIsSynchronous) const; @@ -558,7 +560,7 @@ public: nsIPresShell *GetShell()const { return mShell; } - void DisconnectFromPresShell() { mShell = nsnull; } + void DisconnectFromPresShell() { StopAutoScrollTimer(); mShell = nsnull; } private: nsresult TakeFocus(nsIContent *aNewFocus, PRUint32 aContentOffset, diff --git a/layout/generic/nsSelection.cpp b/layout/generic/nsSelection.cpp index 17add3f9565..99b5d765e83 100644 --- a/layout/generic/nsSelection.cpp +++ b/layout/generic/nsSelection.cpp @@ -246,6 +246,7 @@ public: SelectionDetails **aReturnDetails, SelectionType aType, PRBool aSlowCheck); NS_IMETHOD Repaint(nsPresContext* aPresContext); + // Note: StartAutoScrollTimer might destroy arbitrary frames, views etc. nsresult StartAutoScrollTimer(nsPresContext *aPresContext, nsIView *aView, nsPoint& aPoint, @@ -257,12 +258,15 @@ public: private: friend class nsAutoScrollTimer; + // Note: DoAutoScrollView might destroy arbitrary frames, views etc. nsresult DoAutoScrollView(nsPresContext *aPresContext, nsIView *aView, nsPoint& aPoint, PRBool aScrollParentViews); + // Note: ScrollPointIntoClipView might destroy arbitrary frames, views etc. nsresult ScrollPointIntoClipView(nsPresContext *aPresContext, nsIView *aView, nsPoint& aPoint, PRBool *aDidScroll); + // Note: ScrollPointIntoView might destroy arbitrary frames, views etc. nsresult ScrollPointIntoView(nsPresContext *aPresContext, nsIView *aView, nsPoint& aPoint, PRBool aScrollParentViews, PRBool *aDidScroll); nsresult GetViewAncestorOffset(nsIView *aView, nsIView *aAncestorView, nscoord *aXOffset, nscoord *aYOffset); @@ -5084,10 +5088,25 @@ nsTypedSelection::ScrollPointIntoClipView(nsPresContext *aPresContext, nsIView * if (dx != 0 || dy != 0) { - // Make sure latest bits are available before we scroll them. - aPresContext->GetViewManager()->Composite(); + nsCOMPtr presShell; + GetPresShell(getter_AddRefs(presShell)); + NS_ASSERTION(presShell, "no pres shell"); - // Now scroll the view! + nsWeakView weakView = scrollableView->View(); + + // Make sure latest bits are available before we scroll them. This flushes + // pending notifications and thus might destroy stuff (bug 421839). + // We need to hold a strong ref on the view manager to keep it alive. + nsCOMPtr viewManager = aPresContext->GetViewManager(); + viewManager->Composite(); + + if (!weakView.IsAlive()) { + return NS_ERROR_NULL_POINTER; + } + + if (presShell->IsDestroying()) { + return NS_ERROR_NULL_POINTER; + } result = scrollableView->ScrollTo(bounds.x + dx, bounds.y + dy, 0); diff --git a/view/src/nsViewManager.cpp b/view/src/nsViewManager.cpp index f93029acf8e..16f6b6ccec9 100644 --- a/view/src/nsViewManager.cpp +++ b/view/src/nsViewManager.cpp @@ -1764,14 +1764,26 @@ void nsViewManager::UpdateWidgetsForView(nsView* aView) { NS_PRECONDITION(aView, "Must have view!"); + nsWeakView parentWeakView = aView; if (aView->HasWidget()) { - aView->GetWidget()->Update(); + aView->GetWidget()->Update(); // Flushes Layout! + if (!parentWeakView.IsAlive()) { + return; + } } - for (nsView* childView = aView->GetFirstChild(); - childView; - childView = childView->GetNextSibling()) { + nsView* childView = aView->GetFirstChild(); + while (childView) { + nsWeakView childWeakView = childView; UpdateWidgetsForView(childView); + if (NS_LIKELY(childWeakView.IsAlive())) { + childView = childView->GetNextSibling(); + } + else { + // The current view was destroyed - restart at the first child if the + // parent is still alive. + childView = parentWeakView.IsAlive() ? aView->GetFirstChild() : nsnull; + } } }