From c1fc16a11bd571be96ed862f0109c17d814f72ff Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Mon, 9 Mar 2020 07:14:19 +0000 Subject: [PATCH] Bug 1620504 - part 3: Clean up warnings in CompositionTransaction r=m_kato Depends on D65867 Differential Revision: https://phabricator.services.mozilla.com/D65869 --HG-- extra : moz-landing-system : lando --- editor/libeditor/CompositionTransaction.cpp | 140 ++++++++++++-------- 1 file changed, 85 insertions(+), 55 deletions(-) diff --git a/editor/libeditor/CompositionTransaction.cpp b/editor/libeditor/CompositionTransaction.cpp index 99a0f47698b5..36d3bfcac645 100644 --- a/editor/libeditor/CompositionTransaction.cpp +++ b/editor/libeditor/CompositionTransaction.cpp @@ -79,40 +79,45 @@ NS_INTERFACE_MAP_END_INHERITING(EditTransactionBase) NS_IMPL_ADDREF_INHERITED(CompositionTransaction, EditTransactionBase) NS_IMPL_RELEASE_INHERITED(CompositionTransaction, EditTransactionBase) -MOZ_CAN_RUN_SCRIPT_BOUNDARY -NS_IMETHODIMP +MOZ_CAN_RUN_SCRIPT_BOUNDARY NS_IMETHODIMP CompositionTransaction::DoTransaction() { if (NS_WARN_IF(!mEditorBase)) { return NS_ERROR_NOT_INITIALIZED; } // Fail before making any changes if there's no selection controller - nsCOMPtr selCon; - mEditorBase->GetSelectionController(getter_AddRefs(selCon)); - NS_ENSURE_TRUE(selCon, NS_ERROR_NOT_INITIALIZED); + if (NS_WARN_IF(!mEditorBase->GetSelectionController())) { + return NS_ERROR_NOT_INITIALIZED; + } RefPtr editorBase = mEditorBase; RefPtr textNode = mTextNode; // Advance caret: This requires the presentation shell to get the selection. if (mReplaceLength == 0) { - ErrorResult rv; - editorBase->DoInsertText(*textNode, mOffset, mStringToInsert, rv); - if (NS_WARN_IF(rv.Failed())) { - return rv.StealNSResult(); + ErrorResult error; + editorBase->DoInsertText(*textNode, mOffset, mStringToInsert, error); + if (error.Failed()) { + NS_WARNING("EditorBase::DoInsertText() failed"); + return error.StealNSResult(); } editorBase->RangeUpdaterRef().SelAdjInsertText(*textNode, mOffset, mStringToInsert); } else { uint32_t replaceableLength = textNode->TextLength() - mOffset; - ErrorResult rv; + ErrorResult error; editorBase->DoReplaceText(*textNode, mOffset, mReplaceLength, - mStringToInsert, rv); - if (NS_WARN_IF(rv.Failed())) { - return rv.StealNSResult(); + mStringToInsert, error); + if (error.Failed()) { + NS_WARNING("EditorBase::DoReplaceText() failed"); + return error.StealNSResult(); } - editorBase->RangeUpdaterRef().SelAdjDeleteText(textNode, mOffset, - mReplaceLength); + DebugOnly rvIgnored = + editorBase->RangeUpdaterRef().SelAdjDeleteText(textNode, mOffset, + mReplaceLength); + NS_WARNING_ASSERTION( + NS_SUCCEEDED(rvIgnored), + "RangeUpdater::SelAdjDeleteText() failed, but ignored"); editorBase->RangeUpdaterRef().SelAdjInsertText(*textNode, mOffset, mStringToInsert); @@ -123,10 +128,14 @@ CompositionTransaction::DoTransaction() { if (replaceableLength < mReplaceLength) { int32_t remainLength = mReplaceLength - replaceableLength; nsCOMPtr node = textNode->GetNextSibling(); + IgnoredErrorResult ignoredError; while (node && node->IsText() && remainLength > 0) { RefPtr textNode = static_cast(node.get()); uint32_t textLength = textNode->TextLength(); - editorBase->DoDeleteText(*textNode, 0, remainLength, IgnoreErrors()); + editorBase->DoDeleteText(*textNode, 0, remainLength, ignoredError); + NS_WARNING_ASSERTION(!ignoredError.Failed(), + "EditorBase::DoDeleteText() failed, but ignored"); + ignoredError.SuppressException(); editorBase->RangeUpdaterRef().SelAdjDeleteText(textNode, 0, remainLength); remainLength -= textLength; @@ -136,13 +145,13 @@ CompositionTransaction::DoTransaction() { } nsresult rv = SetSelectionForRanges(); - NS_ENSURE_SUCCESS(rv, rv); - - return NS_OK; + NS_WARNING_ASSERTION( + NS_SUCCEEDED(rv), + "CompositionTransaction::SetSelectionForRanges() failed"); + return rv; } -MOZ_CAN_RUN_SCRIPT_BOUNDARY -NS_IMETHODIMP +MOZ_CAN_RUN_SCRIPT_BOUNDARY NS_IMETHODIMP CompositionTransaction::UndoTransaction() { if (NS_WARN_IF(!mEditorBase)) { return NS_ERROR_NOT_INITIALIZED; @@ -151,28 +160,30 @@ CompositionTransaction::UndoTransaction() { // Get the selection first so we'll fail before making any changes if we // can't get it RefPtr selection = mEditorBase->GetSelection(); - NS_ENSURE_TRUE(selection, NS_ERROR_NOT_INITIALIZED); + if (NS_WARN_IF(!selection)) { + return NS_ERROR_NOT_INITIALIZED; + } RefPtr editorBase = mEditorBase; RefPtr textNode = mTextNode; - ErrorResult err; - editorBase->DoDeleteText(*textNode, mOffset, mStringToInsert.Length(), err); - if (NS_WARN_IF(err.Failed())) { - return err.StealNSResult(); + ErrorResult error; + editorBase->DoDeleteText(*textNode, mOffset, mStringToInsert.Length(), error); + if (error.Failed()) { + NS_WARNING("EditorBase::DoDeleteText() failed"); + return error.StealNSResult(); } // set the selection to the insertion point where the string was removed nsresult rv = selection->Collapse(textNode, mOffset); - NS_ASSERTION(NS_SUCCEEDED(rv), - "Selection could not be collapsed after undo of IME insert."); - NS_ENSURE_SUCCESS(rv, rv); - - return NS_OK; + NS_ASSERTION(NS_SUCCEEDED(rv), "Selection::Collapse() failed"); + return rv; } -NS_IMETHODIMP -CompositionTransaction::Merge(nsITransaction* aTransaction, bool* aDidMerge) { - NS_ENSURE_ARG_POINTER(aTransaction && aDidMerge); +NS_IMETHODIMP CompositionTransaction::Merge(nsITransaction* aTransaction, + bool* aDidMerge) { + if (NS_WARN_IF(!aTransaction) || NS_WARN_IF(!aDidMerge)) { + return NS_ERROR_INVALID_ARG; + } // Check to make sure we aren't fixed, if we are then nothing gets absorbed if (mFixed) { @@ -203,8 +214,11 @@ nsresult CompositionTransaction::SetSelectionForRanges() { if (NS_WARN_IF(!mEditorBase)) { return NS_ERROR_NOT_INITIALIZED; } - return SetIMESelection(*mEditorBase, mTextNode, mOffset, - mStringToInsert.Length(), mRanges); + nsresult rv = SetIMESelection(*mEditorBase, mTextNode, mOffset, + mStringToInsert.Length(), mRanges); + NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), + "CompositionTransaction::SetIMESelection() failed"); + return rv; } // static @@ -212,7 +226,9 @@ nsresult CompositionTransaction::SetIMESelection( EditorBase& aEditorBase, Text* aTextNode, uint32_t aOffsetInNode, uint32_t aLengthOfCompositionString, const TextRangeArray* aRanges) { RefPtr selection = aEditorBase.GetSelection(); - NS_ENSURE_TRUE(selection, NS_ERROR_NOT_INITIALIZED); + if (NS_WARN_IF(!selection)) { + return NS_ERROR_NOT_INITIALIZED; + } SelectionBatcher selectionBatcher(selection); @@ -223,17 +239,24 @@ nsresult CompositionTransaction::SetIMESelection( nsISelectionController::SELECTION_IME_CONVERTEDTEXT, nsISelectionController::SELECTION_IME_SELECTEDCONVERTEDTEXT}; - nsCOMPtr selCon; - aEditorBase.GetSelectionController(getter_AddRefs(selCon)); - NS_ENSURE_TRUE(selCon, NS_ERROR_NOT_INITIALIZED); + nsCOMPtr selectionController = + aEditorBase.GetSelectionController(); + if (NS_WARN_IF(!selectionController)) { + return NS_ERROR_NOT_INITIALIZED; + } - nsresult rv = NS_OK; + IgnoredErrorResult ignoredError; for (uint32_t i = 0; i < ArrayLength(kIMESelections); ++i) { - RefPtr selectionOfIME = selCon->GetSelection(kIMESelections[i]); + RefPtr selectionOfIME = + selectionController->GetSelection(kIMESelections[i]); if (!selectionOfIME) { + NS_WARNING("nsISelectionController::GetSelection() failed"); continue; } - selectionOfIME->RemoveAllRanges(IgnoreErrors()); + selectionOfIME->RemoveAllRanges(ignoredError); + NS_WARNING_ASSERTION(!ignoredError.Failed(), + "Selection::RemoveAllRanges() failed, but ignored"); + ignoredError.SuppressException(); } // Set caret position and selection of IME composition with TextRangeArray. @@ -248,6 +271,7 @@ nsresult CompositionTransaction::SetIMESelection( // NOTE: composition string may be truncated when it's committed and // maxlength attribute value doesn't allow input of all text of this // composition. + nsresult rv = NS_OK; for (uint32_t i = 0; i < countOfRanges; ++i) { const TextRange& textRange = aRanges->ElementAt(i); @@ -263,8 +287,11 @@ nsresult CompositionTransaction::SetIMESelection( MOZ_ASSERT(caretOffset >= 0 && static_cast(caretOffset) <= maxOffset); rv = selection->Collapse(aTextNode, caretOffset); + NS_WARNING_ASSERTION( + NS_SUCCEEDED(rv), + "Selection::Collapse() failed, but might be ignored"); setCaret = setCaret || NS_SUCCEEDED(rv); - if (NS_WARN_IF(!setCaret)) { + if (!setCaret) { continue; } // If caret range is specified explicitly, we should show the caret if @@ -293,30 +320,34 @@ nsresult CompositionTransaction::SetIMESelection( clauseRange = nsRange::Create(aTextNode, startOffset, aTextNode, endOffset, IgnoreErrors()); if (!clauseRange) { - NS_WARNING("Failed to create a DOM range for a clause of composition"); + NS_WARNING("nsRange::Create() failed, but might be ignored"); break; } // Set the range of the clause to selection. - RefPtr selectionOfIME = - selCon->GetSelection(ToRawSelectionType(textRange.mRangeType)); + RefPtr selectionOfIME = selectionController->GetSelection( + ToRawSelectionType(textRange.mRangeType)); if (!selectionOfIME) { - NS_WARNING("Failed to get IME selection"); + NS_WARNING( + "nsISelectionController::GetSelection() failed, but might be " + "ignored"); break; } - IgnoredErrorResult err; + IgnoredErrorResult ignoredError; selectionOfIME->AddRangeAndSelectFramesAndNotifyListeners(*clauseRange, - err); - if (err.Failed()) { - NS_WARNING("Failed to add selection range for a clause of composition"); + ignoredError); + if (ignoredError.Failed()) { + NS_WARNING( + "Selection::AddRangeAndSelectFramesAndNotifyListeners() failed, but " + "might be ignored"); break; } // Set the style of the clause. rv = selectionOfIME->SetTextRangeStyle(clauseRange, textRange.mRangeStyle); if (NS_FAILED(rv)) { - NS_WARNING("Failed to set selection style"); + NS_WARNING("Selection::SetTextRangeStyle() failed, but might be ignored"); break; // but this is unexpected... } } @@ -329,8 +360,7 @@ nsresult CompositionTransaction::SetIMESelection( MOZ_ASSERT(caretOffset >= 0 && static_cast(caretOffset) <= maxOffset); rv = selection->Collapse(aTextNode, caretOffset); - NS_ASSERTION(NS_SUCCEEDED(rv), - "Failed to set caret at the end of composition string"); + NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "Selection::Collapse() failed"); // If caret range isn't specified explicitly, we should hide the caret. // Hiding the caret benefits a Windows build (see bug 555642 comment #6).