Bug 1388638 - Use RAII class for StartBatchChanges/EndBatchChanges. r?masayuki draft
authorMakoto Kato <m_kato@ga2.so-net.ne.jp>
Wed, 09 Aug 2017 18:06:36 +0900
changeset 643098 cd2a5a3606843da36f60acde93b452c2ae581e19
parent 642916 1d042bcb2632ea6a38fa08dbe21a6e8a0ee46961
child 725209 fa26bc216f51c17715b837c417d34df871bfee3b
push id72994
push userbmo:m_kato@ga2.so-net.ne.jp
push dateWed, 09 Aug 2017 09:17:27 +0000
reviewersmasayuki
bugs1388638
milestone57.0a1
Bug 1388638 - Use RAII class for StartBatchChanges/EndBatchChanges. r?masayuki CompositionTransaction forgets to call EndBatchChanges when GetSelectionController returns error, so we should use RAII class instead. MozReview-Commit-ID: HI9kutRVzx6
editor/libeditor/CompositionTransaction.cpp
editor/libeditor/EditorBase.cpp
--- a/editor/libeditor/CompositionTransaction.cpp
+++ b/editor/libeditor/CompositionTransaction.cpp
@@ -193,31 +193,31 @@ CompositionTransaction::SetIMESelection(
                                         Text* aTextNode,
                                         uint32_t aOffsetInNode,
                                         uint32_t aLengthOfCompositionString,
                                         const TextRangeArray* aRanges)
 {
   RefPtr<Selection> selection = aEditorBase.GetSelection();
   NS_ENSURE_TRUE(selection, NS_ERROR_NOT_INITIALIZED);
 
-  nsresult rv = selection->StartBatchChanges();
-  NS_ENSURE_SUCCESS(rv, rv);
+  SelectionBatcher selectionBatcher(selection);
 
   // First, remove all selections of IME composition.
   static const RawSelectionType kIMESelections[] = {
     nsISelectionController::SELECTION_IME_RAWINPUT,
     nsISelectionController::SELECTION_IME_SELECTEDRAWTEXT,
     nsISelectionController::SELECTION_IME_CONVERTEDTEXT,
     nsISelectionController::SELECTION_IME_SELECTEDCONVERTEDTEXT
   };
 
   nsCOMPtr<nsISelectionController> selCon;
   aEditorBase.GetSelectionController(getter_AddRefs(selCon));
   NS_ENSURE_TRUE(selCon, NS_ERROR_NOT_INITIALIZED);
 
+  nsresult rv = NS_OK;
   for (uint32_t i = 0; i < ArrayLength(kIMESelections); ++i) {
     nsCOMPtr<nsISelection> selectionOfIME;
     if (NS_FAILED(selCon->GetSelection(kIMESelections[i],
                                        getter_AddRefs(selectionOfIME)))) {
       continue;
     }
     rv = selectionOfIME->RemoveAllRanges();
     NS_ASSERTION(NS_SUCCEEDED(rv),
@@ -330,15 +330,12 @@ CompositionTransaction::SetIMESelection(
     // If caret range isn't specified explicitly, we should hide the caret.
     // Hiding the caret benefits a Windows build (see bug 555642 comment #6).
     // However, when there is no range, we should keep showing caret.
     if (countOfRanges) {
       aEditorBase.HideCaret(true);
     }
   }
 
-  rv = selection->EndBatchChangesInternal();
-  NS_ASSERTION(NS_SUCCEEDED(rv), "Failed to end batch changes");
-
   return rv;
 }
 
 } // namespace mozilla
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -718,33 +718,30 @@ EditorBase::DoTransaction(Selection* aSe
     // XXX: we will need to make sure that they are disabled during
     // XXX: the init of the editor for text widgets to avoid layout
     // XXX: re-entry during initial reflow. - kin
 
     // get the selection and start a batch change
     RefPtr<Selection> selection = aSelection ? aSelection : GetSelection();
     NS_ENSURE_TRUE(selection, NS_ERROR_NULL_POINTER);
 
-    selection->StartBatchChanges();
+    SelectionBatcher selectionBatcher(selection);
 
     nsresult rv;
     if (mTxnMgr) {
       RefPtr<nsTransactionManager> txnMgr = mTxnMgr;
       rv = txnMgr->DoTransaction(aTxn);
     } else {
       rv = aTxn->DoTransaction();
     }
-    if (NS_SUCCEEDED(rv)) {
-      DoAfterDoTransaction(aTxn);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
     }
 
-    // no need to check rv here, don't lose result of operation
-    selection->EndBatchChanges();
-
-    NS_ENSURE_SUCCESS(rv, rv);
+    DoAfterDoTransaction(aTxn);
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 EditorBase::EnableUndo(bool aEnable)
 {