Don't flush out reflow while scrolling. Possible fix for crash bug 281173,

r+sr=roc, a=caillon.

view/src/nsScrollPortView.cpp

@@ -500,6 +500,7 @@ void nsScrollPortView::Scroll(nsView *aScrolledView, nsPoint aTwipsDelta, nsPoin
     
     if (!scrollWidget)
     {
+      NS_ASSERTION(!canBitBlit, "Someone screwed up");
       nsPoint offsetToWidget;
       GetNearestWidget(&offsetToWidget);
       // We're moving the child widgets because we are scrolling. But
@@ -527,7 +528,7 @@ void nsScrollPortView::Scroll(nsView *aScrolledView, nsPoint aTwipsDelta, nsPoin
       // Scroll the contents of the widget by the specfied amount, and scroll
       // the child widgets
       scrollWidget->Scroll(aPixDelta.x, aPixDelta.y, nsnull);
-      mViewManager->UpdateViewAfterScroll(this, aTwipsDelta.x, aTwipsDelta.y);
+      mViewManager->UpdateViewAfterScroll(this);
     }
   }
 }

view/src/nsViewManager.cpp

@@ -1701,6 +1701,9 @@ nsViewManager::WillBitBlit(nsView* aView, nsPoint aScrollAmount)
   NS_PRECONDITION(aView, "Must have a view");
   NS_PRECONDITION(aView->HasWidget(), "View must have a widget");
 
+  NS_PRECONDITION(!mInScroll, "Nested scrolls?");
+  mInScroll = PR_TRUE;
+  
   // Since the view is actually moving the widget by -aScrollAmount, that's the
   // offset we want to use when accumulating dirty rects.
   AccumulateIntersectionsIntoDirtyRegion(aView, GetRootView(), -aScrollAmount);
@@ -1708,27 +1711,28 @@ nsViewManager::WillBitBlit(nsView* aView, nsPoint aScrollAmount)
 
 // Invalidate all widgets which overlap the view, other than the view's own widgets.
 void
-nsViewManager::UpdateViewAfterScroll(nsIView *aView, PRInt32 aDX, PRInt32 aDY)
+nsViewManager::UpdateViewAfterScroll(nsView *aView)
 {
-  nsView* view = NS_STATIC_CAST(nsView*, aView);
+  NS_ASSERTION(RootViewManager()->mInScroll,
-
+               "Someone forgot to call WillBitBlit()");
   // Look at the view's clipped rect. It may be that part of the view is clipped out
   // in which case we don't need to worry about invalidating the clipped-out part.
-  nsRect damageRect = view->GetClippedRect();
+  nsRect damageRect = aView->GetClippedRect();
   if (damageRect.IsEmpty()) {
     return;
   }
-  damageRect.MoveBy(ComputeViewOffset(view));
+  damageRect.MoveBy(ComputeViewOffset(aView));
 
   // if this is a floating view, it isn't covered by any widgets other than
   // its children, which are handled by the widget scroller.
-  if (view->GetFloating()) {
+  if (aView->GetFloating()) {
     return;
   }
 
-  UpdateWidgetArea(RootViewManager()->GetRootView(), nsRegion(damageRect), view);
+  UpdateWidgetArea(RootViewManager()->GetRootView(), nsRegion(damageRect), aView);
 
   Composite();
+  mInScroll = PR_FALSE;
 }
 
 /**
@@ -1999,17 +2003,20 @@ NS_IMETHODIMP nsViewManager::DispatchEvent(nsGUIEvent *aEvent, nsEventStatus *aS
             // Just notify our own view observer that we're about to paint
             // XXXbz do we need to notify other view observers for viewmanagers
             // in our tree?
-            nsIViewObserver* observer = GetViewObserver();
+            // Make sure to not send WillPaint notifications while scrolling
-            if (observer) {
+            if (!mInScroll) {
-              // Do an update view batch, and make sure we don't process those
+              nsIViewObserver* observer = GetViewObserver();
-              // invalidates right now.  Note that the observer may try to
+              if (observer) {
-              // reenter this code from inside WillPaint() by trying to do a
+                // Do an update view batch, and make sure we don't process
-              // synchronous paint, but since refresh will be disabled it won't
+                // those invalidates right now.  Note that the observer may try
-              // be able to do the paint.  We should really sort out the rules
+                // to reenter this code from inside WillPaint() by trying to do
-              // on our synch painting api....
+                // a synchronous paint, but since refresh will be disabled it
-              BeginUpdateViewBatch();
+                // won't be able to do the paint.  We should really sort out
-              observer->WillPaint();
+                // the rules on our synch painting api....
-              EndUpdateViewBatch(NS_VMREFRESH_DEFERRED);
+                BeginUpdateViewBatch();
+                observer->WillPaint();
+                EndUpdateViewBatch(NS_VMREFRESH_DEFERRED);
+              }
             }
             Refresh(view, event->renderingContext, region,
                     NS_VMREFRESH_DOUBLE_BUFFER);
@@ -4220,35 +4227,39 @@ nsViewManager::FlushPendingInvalidates()
   // we don't go through two invalidate-processing cycles).
   NS_ASSERTION(gViewManagers, "Better have a viewmanagers array!");
 
-  // Disable refresh while we notify our view observers, so that if they do
+  // Make sure to not send WillPaint notifications while scrolling
-  // vie w update batches we don't reenter this code and so that we batch
+  if (!mInScroll) {
-  // all of them together.  We don't use
+    // Disable refresh while we notify our view observers, so that if they do
-  // BeginUpdateViewBatch/EndUpdateViewBatch, since that would reenter this
+    // view update batches we don't reenter this code and so that we batch
-  // exact code, but we want the effect of a single big update batch.
+    // all of them together.  We don't use
-  PRBool refreshEnabled = mRefreshEnabled;
+    // BeginUpdateViewBatch/EndUpdateViewBatch, since that would reenter this
-  mRefreshEnabled = PR_FALSE;
+    // exact code, but we want the effect of a single big update batch.
-  ++mUpdateBatchCnt;
+    PRBool refreshEnabled = mRefreshEnabled;
-  
+    mRefreshEnabled = PR_FALSE;
-  PRInt32 index;
+    ++mUpdateBatchCnt;
-  for (index = 0; index < mVMCount; index++) {
+    
-    nsViewManager* vm = (nsViewManager*)gViewManagers->ElementAt(index);
+    PRInt32 index;
-    if (vm->RootViewManager() == this) {
+    for (index = 0; index < mVMCount; index++) {
-      // One of our kids
+      nsViewManager* vm = (nsViewManager*)gViewManagers->ElementAt(index);
-      nsIViewObserver* observer = vm->GetViewObserver();
+      if (vm->RootViewManager() == this) {
-      if (observer) {
+        // One of our kids
-        observer->WillPaint();
+        nsIViewObserver* observer = vm->GetViewObserver();
-        NS_ASSERTION(mUpdateBatchCnt == 1, "Observer did not end view batch?");
+        if (observer) {
+          observer->WillPaint();
+          NS_ASSERTION(mUpdateBatchCnt == 1,
+                       "Observer did not end view batch?");
+        }
       }
     }
+    
+    --mUpdateBatchCnt;
+    // Someone could have called EnableRefresh on us from inside WillPaint().
+    // Only reset the old mRefreshEnabled value if the current value is false.
+    if (!mRefreshEnabled) {
+      mRefreshEnabled = refreshEnabled;
+    }
   }
-
+  
-  --mUpdateBatchCnt;
-  // Someone could have called EnableRefresh on us from inside WillPaint().
-  // Only reset the old mRefreshEnabled value if the current value is false.
-  if (!mRefreshEnabled) {
-    mRefreshEnabled = refreshEnabled;
-  }
-
   if (mHasPendingUpdates) {
     ProcessPendingUpdates(mRootView);
     mHasPendingUpdates = PR_FALSE;

view/src/nsViewManager.h

@@ -462,10 +462,10 @@ public: // NOT in nsIViewManager, so private to the view module
    * Called to inform the view manager that a view has scrolled.
    * The view manager will invalidate any widgets which may need
    * to be rerendered.
-   * @param aView view to paint. should be root view
+   * @param aView view to paint. should be the nsScrollPortView that
-   * @param aUpdateFlags see bottom of nsIViewManager.h for description
+   * got scrolled.
    */
-  void UpdateViewAfterScroll(nsIView *aView, PRInt32 aDX, PRInt32 aDY);
+  void UpdateViewAfterScroll(nsView *aView);
 
   PRBool CanScrollWithBitBlt(nsView* aView);
 
@@ -525,6 +525,7 @@ private:
   PRPackedBool      mPainting;
   PRPackedBool      mRecursiveRefreshPending;
   PRPackedBool      mHasPendingUpdates;
+  PRPackedBool      mInScroll;
 
   //from here to public should be static and locked... MMP
   static PRInt32           mVMCount;        //number of viewmanagers