Bug 765595 part 3 - Make PeekUndoStack/PeekRedoStack infallible; r=ehsan

editor/libeditor/base/nsEditor.cpp

@@ -644,9 +644,7 @@ nsEditor::DoTransaction(nsITransaction* aTxn)
     DoTransaction(theTxn);
 
     if (mTxnMgr) {
-      nsCOMPtr<nsITransaction> topTxn;
-      nsresult res = mTxnMgr->PeekUndoStack(getter_AddRefs(topTxn));
-      NS_ENSURE_SUCCESS(res, res);
+      nsCOMPtr<nsITransaction> topTxn = mTxnMgr->PeekUndoStack();
       if (topTxn) {
         plcTxn = do_QueryInterface(topTxn);
         if (plcTxn) {
@@ -1999,22 +1997,18 @@ nsEditor::BeginIMEComposition()
   return NS_OK;
 }
 
-nsresult
+void
 nsEditor::EndIMEComposition()
 {
-  NS_ENSURE_TRUE(mInIMEMode, NS_OK); // nothing to do
-
-  nsresult rv = NS_OK;
+  NS_ENSURE_TRUE(mInIMEMode, ); // nothing to do
 
   // commit the IME transaction..we can get at it via the transaction mgr.
   // Note that this means IME won't work without an undo stack!
   if (mTxnMgr) {
-    nsCOMPtr<nsITransaction> txn;
-    rv = mTxnMgr->PeekUndoStack(getter_AddRefs(txn));
-    NS_ASSERTION(NS_SUCCEEDED(rv), "PeekUndoStack() failed");
+    nsCOMPtr<nsITransaction> txn = mTxnMgr->PeekUndoStack();
     nsCOMPtr<nsIAbsorbingTransaction> plcTxn = do_QueryInterface(txn);
     if (plcTxn) {
-      rv = plcTxn->Commit();
+      DebugOnly<nsresult> rv = plcTxn->Commit();
       NS_ASSERTION(NS_SUCCEEDED(rv),
                    "nsIAbsorbingTransaction::Commit() failed");
     }
@@ -2029,8 +2023,6 @@ nsEditor::EndIMEComposition()
 
   // notify editor observers of action
   NotifyEditorObservers();
-
-  return rv;
 }
 
 

editor/libeditor/base/nsEditor.h

@@ -218,7 +218,7 @@ public:
   virtual nsresult BeginIMEComposition();
   virtual nsresult UpdateIMEComposition(const nsAString &aCompositionString,
                                         nsIPrivateTextRangeList *aTextRange)=0;
-  nsresult EndIMEComposition();
+  void EndIMEComposition();
 
   void SwitchTextDirectionTo(PRUint32 aDirection);
 

editor/libeditor/base/nsEditorEventListener.cpp

@@ -307,8 +307,10 @@ nsEditorEventListener::HandleEvent(nsIDOMEvent* aEvent)
     return HandleText(aEvent);
   if (eventType.EqualsLiteral("compositionstart"))
     return HandleStartComposition(aEvent);
-  if (eventType.EqualsLiteral("compositionend"))
-    return HandleEndComposition(aEvent);
+  if (eventType.EqualsLiteral("compositionend")) {
+    HandleEndComposition(aEvent);
+    return NS_OK;
+  }
 
   return NS_OK;
 }
@@ -822,19 +824,19 @@ nsEditorEventListener::HandleStartComposition(nsIDOMEvent* aCompositionEvent)
   return mEditor->BeginIMEComposition();
 }
 
-NS_IMETHODIMP
+void
 nsEditorEventListener::HandleEndComposition(nsIDOMEvent* aCompositionEvent)
 {
-  NS_ENSURE_TRUE(mEditor, NS_ERROR_NOT_AVAILABLE);
+  MOZ_ASSERT(mEditor);
   if (!mEditor->IsAcceptableInputEvent(aCompositionEvent)) {
-    return NS_OK;
+    return;
   }
 
   // Transfer the event's trusted-ness to our editor
   nsCOMPtr<nsIDOMNSEvent> NSEvent = do_QueryInterface(aCompositionEvent);
   nsEditor::HandlingTrustedAction operation(mEditor, NSEvent);
 
-  return mEditor->EndIMEComposition();
+  mEditor->EndIMEComposition();
 }
 
 NS_IMETHODIMP

editor/libeditor/base/nsEditorEventListener.h

@@ -46,7 +46,7 @@ public:
   NS_IMETHOD KeyPress(nsIDOMEvent* aKeyEvent);
   NS_IMETHOD HandleText(nsIDOMEvent* aTextEvent);
   NS_IMETHOD HandleStartComposition(nsIDOMEvent* aCompositionEvent);
-  NS_IMETHOD HandleEndComposition(nsIDOMEvent* aCompositionEvent);
+  void       HandleEndComposition(nsIDOMEvent* aCompositionEvent);
   NS_IMETHOD MouseDown(nsIDOMEvent* aMouseEvent);
   NS_IMETHOD MouseUp(nsIDOMEvent* aMouseEvent) { return NS_OK; }
   NS_IMETHOD MouseClick(nsIDOMEvent* aMouseEvent);

editor/txmgr/src/nsTransactionItem.cpp

@@ -82,14 +82,11 @@ nsTransactionItem::AddChild(nsTransactionItem *aTransactionItem)
   return NS_OK;
 }
 
-nsresult
-nsTransactionItem::GetTransaction(nsITransaction **aTransaction)
+already_AddRefed<nsITransaction>
+nsTransactionItem::GetTransaction()
 {
-  NS_ENSURE_TRUE(aTransaction, NS_ERROR_NULL_POINTER);
-
-  NS_IF_ADDREF(*aTransaction = mTransaction);
-
-  return NS_OK;
+  nsCOMPtr<nsITransaction> txn = mTransaction;
+  return txn.forget();
 }
 
 nsresult
@@ -227,13 +224,7 @@ nsTransactionItem::UndoChildren(nsTransactionManager *aTxMgr)
         return NS_ERROR_FAILURE;
       }
 
-      nsCOMPtr<nsITransaction> t;
-
-      result = item->GetTransaction(getter_AddRefs(t));
-
-      if (NS_FAILED(result)) {
-        return result;
-      }
+      nsCOMPtr<nsITransaction> t = item->GetTransaction();
 
       bool doInterrupt = false;
 
@@ -306,13 +297,7 @@ nsTransactionItem::RedoChildren(nsTransactionManager *aTxMgr)
       return NS_ERROR_FAILURE;
     }
 
-    nsCOMPtr<nsITransaction> t;
-
-    result = item->GetTransaction(getter_AddRefs(t));
-
-    if (NS_FAILED(result)) {
-      return result;
-    }
+    nsCOMPtr<nsITransaction> t = item->GetTransaction();
 
     bool doInterrupt = false;
 

editor/txmgr/src/nsTransactionItem.h

@@ -30,7 +30,7 @@ public:
   NS_DECL_CYCLE_COLLECTION_NATIVE_CLASS(nsTransactionItem)
 
   virtual nsresult AddChild(nsTransactionItem *aTransactionItem);
-  virtual nsresult GetTransaction(nsITransaction **aTransaction);
+  already_AddRefed<nsITransaction> GetTransaction();
   virtual nsresult GetIsBatch(bool *aIsBatch);
   virtual nsresult GetNumberOfChildren(PRInt32 *aNumChildren);
   virtual nsresult GetChild(PRInt32 aIndex, nsTransactionItem **aChild);

editor/txmgr/src/nsTransactionList.cpp

@@ -104,7 +104,9 @@ NS_IMETHODIMP nsTransactionList::GetItem(PRInt32 aIndex, nsITransaction **aItem)
 
   NS_ENSURE_TRUE(item, NS_ERROR_FAILURE);
 
-  return item->GetTransaction(aItem);
+  *aItem = item->GetTransaction().get();
+
+  return NS_OK;
 }
 
 /* long getNumChildrenForItem (in long aIndex); */

editor/txmgr/src/nsTransactionManager.cpp

@@ -13,6 +13,7 @@
 #include "nsTransactionList.h"
 #include "nsAutoPtr.h"
 #include "nsCOMPtr.h"
+#include "mozilla/Assertions.h"
 
 nsTransactionManager::nsTransactionManager(PRInt32 aMaxTransactionCount)
   : mMaxTransactionCount(aMaxTransactionCount)
@@ -111,13 +112,7 @@ nsTransactionManager::UndoTransaction()
     return NS_OK;
   }
 
-  nsCOMPtr<nsITransaction> t;
-
-  result = tx->GetTransaction(getter_AddRefs(t));
-
-  if (NS_FAILED(result)) {
-    return result;
-  }
+  nsCOMPtr<nsITransaction> t = tx->GetTransaction();
 
   bool doInterrupt = false;
 
@@ -170,13 +165,7 @@ nsTransactionManager::RedoTransaction()
     return NS_OK;
   }
 
-  nsCOMPtr<nsITransaction> t;
-
-  result = tx->GetTransaction(getter_AddRefs(t));
-
-  if (NS_FAILED(result)) {
-    return result;
-  }
+  nsCOMPtr<nsITransaction> t = tx->GetTransaction();
 
   bool doInterrupt = false;
 
@@ -272,8 +261,9 @@ nsTransactionManager::EndBatch()
 
   nsRefPtr<nsTransactionItem> tx = mDoStack.Peek();
 
-  if (tx)
-    tx->GetTransaction(getter_AddRefs(ti));
+  if (tx) {
+    ti = tx->GetTransaction();
+  }
 
   if (!tx || ti) {
     return NS_ERROR_FAILURE;
@@ -399,41 +389,41 @@ nsTransactionManager::SetMaxTransactionCount(PRInt32 aMaxCount)
 NS_IMETHODIMP
 nsTransactionManager::PeekUndoStack(nsITransaction **aTransaction)
 {
-  nsresult result;
-
-  NS_ENSURE_TRUE(aTransaction, NS_ERROR_NULL_POINTER);
-
-  *aTransaction = 0;
+  MOZ_ASSERT(aTransaction);
+  *aTransaction = PeekUndoStack().get();
+  return NS_OK;
+}
 
+already_AddRefed<nsITransaction>
+nsTransactionManager::PeekUndoStack()
+{
   nsRefPtr<nsTransactionItem> tx = mUndoStack.Peek();
 
   if (!tx) {
-    return NS_OK;
+    return nsnull;
   }
 
-  result = tx->GetTransaction(aTransaction);
-
-  return result;
+  return tx->GetTransaction();
 }
 
 NS_IMETHODIMP
-nsTransactionManager::PeekRedoStack(nsITransaction **aTransaction)
+nsTransactionManager::PeekRedoStack(nsITransaction** aTransaction)
 {
-  nsresult result;
-
-  NS_ENSURE_TRUE(aTransaction, NS_ERROR_NULL_POINTER);
-
-  *aTransaction = 0;
+  MOZ_ASSERT(aTransaction);
+  *aTransaction = PeekRedoStack().get();
+  return NS_OK;
+}
 
+already_AddRefed<nsITransaction>
+nsTransactionManager::PeekRedoStack()
+{
   nsRefPtr<nsTransactionItem> tx = mRedoStack.Peek();
 
   if (!tx) {
-    return NS_OK;
+    return nsnull;
   }
 
-  result = tx->GetTransaction(aTransaction);
-
-  return result;
+  return tx->GetTransaction();
 }
 
 NS_IMETHODIMP
@@ -749,7 +739,6 @@ nsTransactionManager::BeginTransaction(nsITransaction *aTransaction)
 nsresult
 nsTransactionManager::EndTransaction()
 {
-  nsCOMPtr<nsITransaction> tint;
   nsresult result              = NS_OK;
 
   nsRefPtr<nsTransactionItem> tx = mDoStack.Pop();
@@ -757,12 +746,7 @@ nsTransactionManager::EndTransaction()
   if (!tx)
     return NS_ERROR_FAILURE;
 
-  result = tx->GetTransaction(getter_AddRefs(tint));
-
-  if (NS_FAILED(result)) {
-    // XXX: What do we do with the transaction item at this point?
-    return result;
-  }
+  nsCOMPtr<nsITransaction> tint = tx->GetTransaction();
 
   if (!tint) {
     PRInt32 nc = 0;
@@ -819,9 +803,7 @@ nsTransactionManager::EndTransaction()
 
   if (tint && top) {
     bool didMerge = false;
-    nsCOMPtr<nsITransaction> topTransaction;
-
-    result = top->GetTransaction(getter_AddRefs(topTransaction));
+    nsCOMPtr<nsITransaction> topTransaction = top->GetTransaction();
 
     if (topTransaction) {
 

editor/txmgr/src/nsTransactionManager.h

@@ -52,6 +52,8 @@ public:
   /* nsTransactionManager specific methods. */
   virtual nsresult ClearUndoStack(void);
   virtual nsresult ClearRedoStack(void);
+  already_AddRefed<nsITransaction> PeekUndoStack();
+  already_AddRefed<nsITransaction> PeekRedoStack();
 
   virtual nsresult WillDoNotify(nsITransaction *aTransaction, bool *aInterrupt);
   virtual nsresult DidDoNotify(nsITransaction *aTransaction, nsresult aExecuteResult);