Bug 413292. Make Begin/EndUpdateViewBatch be sure to remove the batch-count from the same viewmanager we added one to. r+sr=bzbarsky

editor/libeditor/base/nsEditor.cpp

@@ -4353,9 +4353,7 @@ nsresult nsEditor::BeginUpdateViewBatch()
     }
 
     // Turn off view updating.
-
-    if (mViewManager)
-      mViewManager->BeginUpdateViewBatch();
+    mBatch.BeginUpdateViewBatch(mViewManager);
   }
 
   mUpdateCount++;
@@ -4411,7 +4409,7 @@ nsresult nsEditor::EndUpdateViewBatch()
         // have selection code that does sync caret scrolling in this case.
         presShell->FlushPendingNotifications(Flush_Layout);
       }
-      mViewManager->EndUpdateViewBatch(updateFlag);
+      mBatch.EndUpdateViewBatch(updateFlag);
     }
 
     // Turn selection updating and notifications back on.

editor/libeditor/base/nsEditor.h

@@ -65,11 +65,11 @@
 #include "nsIInlineSpellChecker.h"
 #include "nsPIDOMEventTarget.h"
 #include "nsStubMutationObserver.h"
+#include "nsIViewManager.h"
 
 class nsIDOMCharacterData;
 class nsIDOMRange;
 class nsIPresShell;
-class nsIViewManager;
 class ChangeAttributeTxn;
 class CreateElementTxn;
 class InsertElementTxn;
@@ -597,6 +597,7 @@ protected:
   nsWeakPtr       mSelConWeak;   // weak reference to the nsISelectionController
   nsIViewManager *mViewManager;
   PRInt32         mUpdateCount;
+  nsIViewManager::UpdateViewBatch mBatch;
 
   // Spellchecking
   enum Tristate {

editor/libeditor/text/Makefile.in

@@ -58,6 +58,7 @@ REQUIRES	= xpcom \
 		  necko \
 		  pref \
 		  lwbrk \
+		  view \
 		  gfx \
 		  thebes \
 		  widget \

layout/base/nsCSSFrameConstructor.cpp

@@ -9779,7 +9779,7 @@ ApplyRenderingChangeToTree(nsPresContext* aPresContext,
 
   // XXX this needs to detect the need for a view due to an opacity change and deal with it...
 
-  viewManager->BeginUpdateViewBatch();
+  nsIViewManager::UpdateViewBatch batch(viewManager);
 
 #ifdef DEBUG
   gInApplyRenderingChangeToTree = PR_TRUE;
@@ -9790,7 +9790,7 @@ ApplyRenderingChangeToTree(nsPresContext* aPresContext,
   gInApplyRenderingChangeToTree = PR_FALSE;
 #endif
   
-  viewManager->EndUpdateViewBatch(NS_VMREFRESH_NO_SYNC);
+  batch.EndUpdateViewBatch(NS_VMREFRESH_NO_SYNC);
 }
 
 /**
@@ -9850,11 +9850,10 @@ InvalidateCanvasIfNeeded(nsIFrame* aFrame)
     // Wrap this in a DEFERRED view update batch so we don't try to
     // flush out layout here
 
-    nsIViewManager* viewManager = presContext->GetViewManager();
-    viewManager->BeginUpdateViewBatch();  
+    nsIViewManager::UpdateViewBatch batch(presContext->GetViewManager());  
     ApplyRenderingChangeToTree(presContext, ancestor,
                                nsChangeHint_RepaintFrame);
-    viewManager->EndUpdateViewBatch(NS_VMREFRESH_DEFERRED);
+    batch.EndUpdateViewBatch(NS_VMREFRESH_DEFERRED);
   }
 }
 

layout/base/nsDocumentViewer.cpp

@@ -2664,10 +2664,8 @@ DocumentViewerImpl::SetTextZoom(float aTextZoom)
   if (!GetIsPrintPreview()) {
     mTextZoom = aTextZoom;
   }
-  nsCOMPtr<nsIViewManager> vm = GetViewManager();
-  if (vm) {
-    vm->BeginUpdateViewBatch();
-  }
+
+  nsIViewManager::UpdateViewBatch batch(GetViewManager());
       
   // Set the text zoom on all children of mContainer (even if our zoom didn't
   // change, our children's zoom may be different, though it would be unusual).
@@ -2682,9 +2680,7 @@ DocumentViewerImpl::SetTextZoom(float aTextZoom)
       pc->SetTextZoom(aTextZoom);
   }
 
-  if (vm) {
-    vm->EndUpdateViewBatch(NS_VMREFRESH_NO_SYNC);
-  }
+  batch.EndUpdateViewBatch(NS_VMREFRESH_NO_SYNC);
   
   return NS_OK;
 }
@@ -2705,10 +2701,7 @@ DocumentViewerImpl::SetFullZoom(float aFullZoom)
     mPageZoom = aFullZoom;
   }
 
-  nsCOMPtr<nsIViewManager> vm = GetViewManager();
-  if (vm) {
-    vm->BeginUpdateViewBatch();
-  }
+  nsIViewManager::UpdateViewBatch batch(GetViewManager());
 
   struct ZoomInfo ZoomInfo = { aFullZoom };
   CallChildren(SetChildFullZoom, &ZoomInfo);
@@ -2718,9 +2711,7 @@ DocumentViewerImpl::SetFullZoom(float aFullZoom)
     pc->SetFullZoom(aFullZoom);
   }
 
-  if (vm) {
-    vm->EndUpdateViewBatch(NS_VMREFRESH_NO_SYNC);
-  }
+  batch.EndUpdateViewBatch(NS_VMREFRESH_NO_SYNC);
 
   return NS_OK;
 }

layout/base/nsPresShell.cpp

@@ -2502,8 +2502,8 @@ PresShell::ResizeReflow(nscoord aWidth, nscoord aHeight)
     return NS_OK;
 
   NS_ASSERTION(mViewManager, "Must have view manager");
-  nsCOMPtr<nsIViewManager> viewManager = mViewManager;
-  viewManager->BeginUpdateViewBatch();
+  nsCOMPtr<nsIViewManager> viewManagerDeathGrip = mViewManager;
+  nsIViewManager::UpdateViewBatch batch(mViewManager);
 
   // Take this ref after viewManager so it'll make sure to go away first
   nsCOMPtr<nsIPresShell> kungFuDeathGrip(this);
@@ -2531,7 +2531,7 @@ PresShell::ResizeReflow(nscoord aWidth, nscoord aHeight)
     DidDoReflow();
   }
 
-  viewManager->EndUpdateViewBatch(NS_VMREFRESH_NO_SYNC);
+  batch.EndUpdateViewBatch(NS_VMREFRESH_NO_SYNC);
 
   if (!mIsDestroying) {
     CreateResizeEventTimer();
@@ -3340,7 +3340,7 @@ PresShell::RecreateFramesFor(nsIContent* aContent)
   // to keep the number of entrypoints down.
 
   NS_ASSERTION(mViewManager, "Should have view manager");
-  mViewManager->BeginUpdateViewBatch();
+  nsIViewManager::UpdateViewBatch batch(mViewManager);
 
   // Have to make sure that the content notifications are flushed before we
   // start messing with the frame model; otherwise we can get content doubling.
@@ -3354,7 +3354,7 @@ PresShell::RecreateFramesFor(nsIContent* aContent)
   nsresult rv = mFrameConstructor->ProcessRestyledFrames(changeList);
   --mChangeNestCount;
   
-  mViewManager->EndUpdateViewBatch(NS_VMREFRESH_NO_SYNC);
+  batch.EndUpdateViewBatch(NS_VMREFRESH_NO_SYNC);
 #ifdef ACCESSIBILITY
   InvalidateAccessibleSubtree(aContent);
 #endif
@@ -4456,10 +4456,8 @@ PresShell::DoFlushPendingNotifications(mozFlushType aType,
 
   NS_ASSERTION(!isSafeToFlush || mViewManager, "Must have view manager");
   // Make sure the view manager stays alive while batching view updates.
-  // XXX FIXME: If viewmanager hierarchy is modified while we're in update
-  //            batch... We need to address that somehow.  See bug 369165.
-  nsCOMPtr<nsIViewManager> viewManager = mViewManager;
-  if (isSafeToFlush && viewManager) {
+  nsCOMPtr<nsIViewManager> viewManagerDeathGrip = mViewManager;
+  if (isSafeToFlush && mViewManager) {
     // Processing pending notifications can kill us, and some callers only
     // hold weak refs when calling FlushPendingNotifications().  :(
     nsCOMPtr<nsIPresShell> kungFuDeathGrip(this);
@@ -4467,7 +4465,7 @@ PresShell::DoFlushPendingNotifications(mozFlushType aType,
     // Style reresolves not in conjunction with reflows can't cause
     // painting or geometry changes, so don't bother with view update
     // batching if we only have style reresolve
-    viewManager->BeginUpdateViewBatch();
+    nsIViewManager::UpdateViewBatch batch(mViewManager);
 
     // Force flushing of any pending content notifications that might have
     // queued up while our event was pending.  That will ensure that we don't
@@ -4519,7 +4517,7 @@ PresShell::DoFlushPendingNotifications(mozFlushType aType,
       // at the end of this view batch.
       updateFlags = NS_VMREFRESH_DEFERRED;
     }
-    viewManager->EndUpdateViewBatch(updateFlags);
+    batch.EndUpdateViewBatch(updateFlags);
   }
 
   return NS_OK;
@@ -6420,7 +6418,7 @@ PresShell::Observe(nsISupports* aSubject,
     // at interesting times during startup.
     if (rootFrame) {
       NS_ASSERTION(mViewManager, "View manager must exist");
-      mViewManager->BeginUpdateViewBatch();
+      nsIViewManager::UpdateViewBatch batch(mViewManager);
 
       WalkFramesThroughPlaceholders(mPresContext, rootFrame,
                                     &ReResolveMenusAndTrees, nsnull);
@@ -6436,7 +6434,7 @@ PresShell::Observe(nsISupports* aSubject,
       mFrameConstructor->ProcessRestyledFrames(changeList);
       --mChangeNestCount;
 
-      mViewManager->EndUpdateViewBatch(NS_VMREFRESH_NO_SYNC);
+      batch.EndUpdateViewBatch(NS_VMREFRESH_NO_SYNC);
 #ifdef ACCESSIBILITY
       InvalidateAccessibleSubtree(nsnull);
 #endif

view/public/nsIViewManager.h

@@ -322,13 +322,34 @@ public:
    */
   NS_IMETHOD EnableRefresh(PRUint32 aUpdateFlags) = 0;
 
+  class UpdateViewBatch {
+  public:
+    UpdateViewBatch() {}
   /**
    * prevents the view manager from refreshing. allows UpdateView()
    * to notify widgets of damaged regions that should be repainted
-   * when the batch is ended.
+   * when the batch is ended. Call EndUpdateViewBatch on this object
+   * before it is destroyed
    * @return error status
    */
-  NS_IMETHOD BeginUpdateViewBatch(void) = 0;
+    UpdateViewBatch(nsIViewManager* aVM) {
+      if (aVM) {
+        mRootVM = aVM->BeginUpdateViewBatch();
+      }
+    }
+    ~UpdateViewBatch() {
+      NS_ASSERTION(!mRootVM, "Someone forgot to call EndUpdateViewBatch!");
+    }
+    
+    /**
+     * See the constructor, this lets you "fill in" a blank UpdateViewBatch.
+     */
+    void BeginUpdateViewBatch(nsIViewManager* aVM) {
+      NS_ASSERTION(!mRootVM, "already started a batch!");
+      if (aVM) {
+        mRootVM = aVM->BeginUpdateViewBatch();
+      }
+    }
 
   /**
    * allow the view manager to refresh any damaged areas accumulated
@@ -352,8 +373,28 @@ public:
    * @param aUpdateFlags see bottom of nsIViewManager.h for
    * description @return error status
    */
+    void EndUpdateViewBatch(PRUint32 aUpdateFlags) {
+      if (!mRootVM)
+        return;
+      mRootVM->EndUpdateViewBatch(aUpdateFlags);
+      mRootVM = nsnull;
+    }
+
+  private:
+    UpdateViewBatch(const UpdateViewBatch& aOther) : mRootVM(aOther.mRootVM);
+    const UpdateViewBatch& operator=(const UpdateViewBatch& aOther);
+
+    nsCOMPtr<nsIViewManager> mRootVM;
+  };
+  
+private:
+  friend class UpdateViewBatch;
+
+  virtual nsIViewManager* BeginUpdateViewBatch(void) = 0;
   NS_IMETHOD EndUpdateViewBatch(PRUint32 aUpdateFlags) = 0;
 
+public:
+
   /**
    * set the view that is is considered to be the root scrollable
    * view for the document.

view/src/nsViewManager.cpp

@@ -1045,9 +1045,9 @@ NS_IMETHODIMP nsViewManager::DispatchEvent(nsGUIEvent *aEvent, nsEventStatus *aS
                 // refresh will be disabled it won't be able to do the paint.
                 // We should really sort out the rules on our synch painting
                 // api....
-                BeginUpdateViewBatch();
+                UpdateViewBatch batch(this);
                 observer->WillPaint();
-                EndUpdateViewBatch(NS_VMREFRESH_NO_SYNC);
+                batch.EndUpdateViewBatch(NS_VMREFRESH_NO_SYNC);
 
                 // Get the view pointer again since the code above might have
                 // destroyed it (bug 378273).
@@ -1843,7 +1843,7 @@ NS_IMETHODIMP nsViewManager::EnableRefresh(PRUint32 aUpdateFlags)
   return NS_OK;
 }
 
-NS_IMETHODIMP nsViewManager::BeginUpdateViewBatch(void)
+nsIViewManager* nsViewManager::BeginUpdateViewBatch(void)
 {
   if (!IsRootVM()) {
     return RootViewManager()->BeginUpdateViewBatch();
@@ -1859,14 +1859,12 @@ NS_IMETHODIMP nsViewManager::BeginUpdateViewBatch(void)
   if (NS_SUCCEEDED(result))
     ++mUpdateBatchCnt;
 
-  return result;
+  return this;
 }
 
 NS_IMETHODIMP nsViewManager::EndUpdateViewBatch(PRUint32 aUpdateFlags)
 {
-  if (!IsRootVM()) {
-    return RootViewManager()->EndUpdateViewBatch(aUpdateFlags);
-  }
+  NS_ASSERTION(IsRootVM(), "Should only be called on root");
   
   nsresult result = NS_OK;
 

view/src/nsViewManager.h

@@ -164,7 +164,7 @@ public:
   NS_IMETHOD  DisableRefresh(void);
   NS_IMETHOD  EnableRefresh(PRUint32 aUpdateFlags);
 
-  NS_IMETHOD  BeginUpdateViewBatch(void);
+  virtual nsIViewManager* BeginUpdateViewBatch(void);
   NS_IMETHOD  EndUpdateViewBatch(PRUint32 aUpdateFlags);
 
   NS_IMETHOD  SetRootScrollableView(nsIScrollableView *aScrollable);