From 82e20136835b08e21041623ecc908023abba810f Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Thu, 9 Dec 2021 07:35:09 +0000 Subject: [PATCH] Bug 1735446 - part 4: Make `Selection::GetRangeAt()` take `uint32_t` instead of `int32_t` r=smaug It's an internal API corresponding to `Selection.getRangeAt` DOM API. I think that it should use `uint32_t` rather than `size_t` because of the consistency with the DOM API and `Selection::RangeCount()`. This patch fixes all callers of `GetRangeAt()`, and rewrites it with ranged- loops unless original ones do not refer `RangeCount()` every time and may run script in the loop. Differential Revision: https://phabricator.services.mozilla.com/D128848 --- accessible/base/TextRange.cpp | 8 ++++-- accessible/generic/HyperTextAccessible.cpp | 20 +++++++++---- dom/base/Document.cpp | 7 +++-- dom/base/Selection.cpp | 7 +++-- dom/base/Selection.h | 2 +- dom/base/SelectionChangeEventDispatcher.cpp | 7 +++-- dom/base/nsCopySupport.cpp | 1 + dom/events/TextComposition.cpp | 17 ++++++++--- dom/serializers/nsDocumentEncoder.cpp | 12 ++++---- editor/libeditor/EditorBase.cpp | 20 +++++++++---- editor/libeditor/EditorUtils.cpp | 8 ++++-- editor/libeditor/EditorUtils.h | 8 ++++-- editor/libeditor/HTMLEditHelpers.h | 1 + editor/libeditor/HTMLEditSubActionHandler.cpp | 18 ++++++++---- editor/libeditor/HTMLEditUtils.h | 8 ++++-- editor/libeditor/HTMLEditor.cpp | 19 +++++++++---- editor/libeditor/HTMLTableEditor.cpp | 1 + editor/libeditor/SelectionState.cpp | 10 +++++-- editor/spellchecker/TextServicesDocument.cpp | 21 ++++++++------ .../spellcheck/src/mozInlineSpellChecker.cpp | 28 ++++++++++--------- layout/base/AccessibleCaretManager.cpp | 1 + layout/base/PresShell.cpp | 9 +++--- layout/base/nsLayoutUtils.cpp | 6 ++-- layout/generic/nsFrameSelection.cpp | 11 +++++--- layout/printing/nsPrintJob.cpp | 7 +++-- toolkit/components/find/nsWebBrowserFind.cpp | 8 +++--- 26 files changed, 175 insertions(+), 90 deletions(-) diff --git a/accessible/base/TextRange.cpp b/accessible/base/TextRange.cpp index 940110e3f0e8..5c4a78061675 100644 --- a/accessible/base/TextRange.cpp +++ b/accessible/base/TextRange.cpp @@ -8,6 +8,7 @@ #include "LocalAccessible-inl.h" #include "HyperTextAccessible-inl.h" +#include "mozilla/IntegerRange.h" #include "mozilla/dom/Selection.h" #include "nsAccUtils.h" @@ -263,7 +264,7 @@ bool TextRange::SetSelectionAt(int32_t aSelectionNum) const { if (aSelectionNum == static_cast(rangeCount)) { range = nsRange::Create(mRoot->GetContent()); } else { - range = domSel->GetRangeAt(aSelectionNum); + range = domSel->GetRangeAt(AssertedCast(aSelectionNum)); } if (!range) { @@ -400,8 +401,11 @@ void TextRange::TextRangesFromSelection(dom::Selection* aSelection, aRanges->SetCapacity(aSelection->RangeCount()); - for (uint32_t idx = 0; idx < aSelection->RangeCount(); idx++) { + const uint32_t rangeCount = aSelection->RangeCount(); + for (const uint32_t idx : IntegerRange(rangeCount)) { + MOZ_ASSERT(aSelection->RangeCount() == rangeCount); const nsRange* DOMRange = aSelection->GetRangeAt(idx); + MOZ_ASSERT(DOMRange); HyperTextAccessible* startContainer = nsAccUtils::GetTextContainer(DOMRange->GetStartContainer()); HyperTextAccessible* endContainer = diff --git a/accessible/generic/HyperTextAccessible.cpp b/accessible/generic/HyperTextAccessible.cpp index 9be0d2a3f267..c452ba4b460b 100644 --- a/accessible/generic/HyperTextAccessible.cpp +++ b/accessible/generic/HyperTextAccessible.cpp @@ -40,6 +40,7 @@ #include "mozilla/EditorBase.h" #include "mozilla/EventStates.h" #include "mozilla/HTMLEditor.h" +#include "mozilla/IntegerRange.h" #include "mozilla/MathAlgorithms.h" #include "mozilla/PresShell.h" #include "mozilla/StaticPrefs_accessibility.h" @@ -1610,8 +1611,12 @@ nsresult HyperTextAccessible::SetSelectionRange(int32_t aStartPos, NS_ENSURE_STATE(domSel); // Set up the selection. - for (int32_t idx = domSel->RangeCount() - 1; idx > 0; idx--) { + for (const uint32_t idx : Reversed(IntegerRange(1u, domSel->RangeCount()))) { + MOZ_ASSERT(domSel->RangeCount() == idx + 1); RefPtr range{domSel->GetRangeAt(idx)}; + if (!range) { + break; // The range count has been changed by somebody else. + } domSel->RemoveRangeAndUnselectFramesAndNotifyListeners(*range, IgnoreErrors()); } @@ -1921,7 +1926,8 @@ bool HyperTextAccessible::RemoveFromSelection(int32_t aSelectionNum) { return false; } - const RefPtr range{domSel->GetRangeAt(aSelectionNum)}; + const RefPtr range{ + domSel->GetRangeAt(static_cast(aSelectionNum))}; domSel->RemoveRangeAndUnselectFramesAndNotifyListeners(*range, IgnoreErrors()); return true; @@ -2315,12 +2321,16 @@ void HyperTextAccessible::GetSpellTextAttr(nsINode* aNode, int32_t aNodeOffset, dom::Selection* domSel = fs->GetSelection(SelectionType::eSpellCheck); if (!domSel) return; - int32_t rangeCount = domSel->RangeCount(); - if (rangeCount <= 0) return; + const uint32_t rangeCount = domSel->RangeCount(); + if (!rangeCount) { + return; + } uint32_t startOffset = 0, endOffset = 0; - for (int32_t idx = 0; idx < rangeCount; idx++) { + for (const uint32_t idx : IntegerRange(rangeCount)) { + MOZ_ASSERT(domSel->RangeCount() == rangeCount); const nsRange* range = domSel->GetRangeAt(idx); + MOZ_ASSERT(range); if (range->Collapsed()) continue; // See if the point comes after the range in which case we must continue in diff --git a/dom/base/Document.cpp b/dom/base/Document.cpp index 2c6dd2394106..184da8783d1a 100644 --- a/dom/base/Document.cpp +++ b/dom/base/Document.cpp @@ -13054,13 +13054,16 @@ static void CachePrintSelectionRanges(const Document& aSourceDoc, return; } - size_t rangeCount = + const uint32_t rangeCount = sourceDocIsStatic ? origRanges->Length() : origSelection->RangeCount(); auto printRanges = MakeUnique>>(rangeCount); - for (size_t i = 0; i < rangeCount; ++i) { + for (const uint32_t i : IntegerRange(rangeCount)) { + MOZ_ASSERT_IF(!sourceDocIsStatic, + origSelection->RangeCount() == rangeCount); const nsRange* range = sourceDocIsStatic ? origRanges->ElementAt(i).get() : origSelection->GetRangeAt(i); + MOZ_ASSERT(range); nsINode* startContainer = range->GetStartContainer(); nsINode* endContainer = range->GetEndContainer(); diff --git a/dom/base/Selection.cpp b/dom/base/Selection.cpp index 07005fbba9c4..7e6a4e4277c0 100644 --- a/dom/base/Selection.cpp +++ b/dom/base/Selection.cpp @@ -2264,7 +2264,7 @@ nsRange* Selection::GetRangeAt(uint32_t aIndex, ErrorResult& aRv) { return range; } -nsRange* Selection::GetRangeAt(int32_t aIndex) const { +nsRange* Selection::GetRangeAt(uint32_t aIndex) const { StyledRange empty(nullptr); return mStyledRanges.mRanges.SafeElementAt(aIndex, empty).mRange; } @@ -2748,8 +2748,11 @@ bool Selection::ContainsPoint(const nsPoint& aPoint) { return false; } PointInRectChecker checker(aPoint); - for (uint32_t i = 0; i < RangeCount(); i++) { + const uint32_t rangeCount = RangeCount(); + for (const uint32_t i : IntegerRange(rangeCount)) { + MOZ_ASSERT(RangeCount() == rangeCount); nsRange* range = GetRangeAt(i); + MOZ_ASSERT(range); nsRange::CollectClientRectsAndText( &checker, nullptr, range, range->GetStartContainer(), range->StartOffset(), range->GetEndContainer(), range->EndOffset(), diff --git a/dom/base/Selection.h b/dom/base/Selection.h index 5b19015d2a43..38904d167752 100644 --- a/dom/base/Selection.h +++ b/dom/base/Selection.h @@ -215,7 +215,7 @@ class Selection final : public nsSupportsWeakReference, /** * See mStyledRanges.mRanges. */ - nsRange* GetRangeAt(int32_t aIndex) const; + nsRange* GetRangeAt(uint32_t aIndex) const; // Get the anchor-to-focus range if we don't care which end is // anchor and which end is focus. diff --git a/dom/base/SelectionChangeEventDispatcher.cpp b/dom/base/SelectionChangeEventDispatcher.cpp index 213f8c30bf86..e1097bba8c2e 100644 --- a/dom/base/SelectionChangeEventDispatcher.cpp +++ b/dom/base/SelectionChangeEventDispatcher.cpp @@ -12,6 +12,7 @@ #include "mozilla/AsyncEventDispatcher.h" #include "mozilla/BasePrincipal.h" +#include "mozilla/IntegerRange.h" #include "mozilla/StaticPrefs_dom.h" #include "mozilla/dom/Document.h" #include "mozilla/dom/Selection.h" @@ -82,8 +83,8 @@ void SelectionChangeEventDispatcher::OnSelectionChange(Document* aDoc, !aSel->IsBlockingSelectionChangeEvents()) { bool changed = mOldDirection != aSel->GetDirection(); if (!changed) { - for (size_t i = 0; i < mOldRanges.Length(); i++) { - if (!mOldRanges[i].Equals(aSel->GetRangeAt(static_cast(i)))) { + for (const uint32_t i : IntegerRange(mOldRanges.Length())) { + if (!mOldRanges[i].Equals(aSel->GetRangeAt(i))) { changed = true; break; } @@ -97,7 +98,7 @@ void SelectionChangeEventDispatcher::OnSelectionChange(Document* aDoc, // The ranges have actually changed, update the mOldRanges array mOldRanges.ClearAndRetainStorage(); - for (size_t i = 0; i < aSel->RangeCount(); i++) { + for (const uint32_t i : IntegerRange(aSel->RangeCount())) { mOldRanges.AppendElement(RawRangeData(aSel->GetRangeAt(i))); } mOldDirection = aSel->GetDirection(); diff --git a/dom/base/nsCopySupport.cpp b/dom/base/nsCopySupport.cpp index 7c4ec7d0d556..2c73af7889dc 100644 --- a/dom/base/nsCopySupport.cpp +++ b/dom/base/nsCopySupport.cpp @@ -713,6 +713,7 @@ static bool IsInsideRuby(nsINode* aNode) { static bool IsSelectionInsideRuby(Selection* aSelection) { uint32_t rangeCount = aSelection->RangeCount(); for (auto i : IntegerRange(rangeCount)) { + MOZ_ASSERT(aSelection->RangeCount() == rangeCount); const nsRange* range = aSelection->GetRangeAt(i); if (!IsInsideRuby(range->GetClosestCommonInclusiveAncestor())) { return false; diff --git a/dom/events/TextComposition.cpp b/dom/events/TextComposition.cpp index 9e25ec41be24..9151b3d429b2 100644 --- a/dom/events/TextComposition.cpp +++ b/dom/events/TextComposition.cpp @@ -14,6 +14,7 @@ #include "mozilla/EditorBase.h" #include "mozilla/EventDispatcher.h" #include "mozilla/IMEStateManager.h" +#include "mozilla/IntegerRange.h" #include "mozilla/MiscEvents.h" #include "mozilla/PresShell.h" #include "mozilla/RangeBoundary.h" @@ -695,9 +696,13 @@ RawRangeBoundary TextComposition::GetStartRef() const { if (!selection) { continue; } - for (uint32_t i = 0; i < selection->RangeCount(); i++) { + const uint32_t rangeCount = selection->RangeCount(); + for (const uint32_t i : IntegerRange(rangeCount)) { + MOZ_ASSERT(selection->RangeCount() == rangeCount); const nsRange* range = selection->GetRangeAt(i); - if (NS_WARN_IF(!range) || NS_WARN_IF(!range->GetStartContainer())) { + MOZ_ASSERT(range); + if (MOZ_UNLIKELY(NS_WARN_IF(!range)) || + MOZ_UNLIKELY(NS_WARN_IF(!range->GetStartContainer()))) { continue; } if (!firstRange) { @@ -752,9 +757,13 @@ RawRangeBoundary TextComposition::GetEndRef() const { if (!selection) { continue; } - for (uint32_t i = 0; i < selection->RangeCount(); i++) { + const uint32_t rangeCount = selection->RangeCount(); + for (const uint32_t i : IntegerRange(rangeCount)) { + MOZ_ASSERT(selection->RangeCount() == rangeCount); const nsRange* range = selection->GetRangeAt(i); - if (NS_WARN_IF(!range) || NS_WARN_IF(!range->GetEndContainer())) { + MOZ_ASSERT(range); + if (MOZ_UNLIKELY(NS_WARN_IF(!range)) || + MOZ_UNLIKELY(NS_WARN_IF(!range->GetEndContainer()))) { continue; } if (!lastRange) { diff --git a/dom/serializers/nsDocumentEncoder.cpp b/dom/serializers/nsDocumentEncoder.cpp index 34c6ff4334c9..05734f74738d 100644 --- a/dom/serializers/nsDocumentEncoder.cpp +++ b/dom/serializers/nsDocumentEncoder.cpp @@ -44,6 +44,7 @@ #include "mozilla/dom/ShadowRoot.h" #include "mozilla/dom/Text.h" #include "mozilla/Encoding.h" +#include "mozilla/IntegerRange.h" #include "mozilla/Maybe.h" #include "mozilla/ScopeExit.h" #include "mozilla/UniquePtr.h" @@ -580,12 +581,12 @@ nsresult nsDocumentEncoder::SerializeSelection() { nsresult rv = NS_OK; const Selection* selection = mEncodingScope.mSelection; - uint32_t count = selection->RangeCount(); - nsCOMPtr node; nsCOMPtr prevNode; uint32_t firstRangeStartDepth = 0; - for (uint32_t i = 0; i < count; ++i) { + const uint32_t rangeCount = selection->RangeCount(); + for (const uint32_t i : IntegerRange(rangeCount)) { + MOZ_ASSERT(selection->RangeCount() == rangeCount); RefPtr range = selection->GetRangeAt(i); // Bug 236546: newlines not added when copying table cells into clipboard @@ -1575,7 +1576,7 @@ nsHTMLCopyEncoder::SetSelection(Selection* aSelection) { if (!aSelection) return NS_ERROR_NULL_POINTER; - uint32_t rangeCount = aSelection->RangeCount(); + const uint32_t rangeCount = aSelection->RangeCount(); // if selection is uninitialized return if (!rangeCount) { @@ -1630,7 +1631,8 @@ nsHTMLCopyEncoder::SetSelection(Selection* aSelection) { mEncodingScope.mSelection = new Selection(SelectionType::eNormal, nullptr); // loop thru the ranges in the selection - for (uint32_t rangeIdx = 0; rangeIdx < rangeCount; ++rangeIdx) { + for (const uint32_t rangeIdx : IntegerRange(rangeCount)) { + MOZ_ASSERT(aSelection->RangeCount() == rangeCount); range = aSelection->GetRangeAt(rangeIdx); NS_ENSURE_TRUE(range, NS_ERROR_FAILURE); RefPtr myRange = range->CloneRange(); diff --git a/editor/libeditor/EditorBase.cpp b/editor/libeditor/EditorBase.cpp index 5231313fc6d8..267c2b89373d 100644 --- a/editor/libeditor/EditorBase.cpp +++ b/editor/libeditor/EditorBase.cpp @@ -43,6 +43,7 @@ #include "mozilla/IMEContentObserver.h" // for IMEContentObserver #include "mozilla/IMEStateManager.h" // for IMEStateManager #include "mozilla/InputEventOptions.h" // for InputEventOptions +#include "mozilla/IntegerRange.h" // for IntegerRange #include "mozilla/InternalMutationEvent.h" // for NS_EVENT_BITS_MUTATION_CHARACTERDATAMODIFIED #include "mozilla/mozalloc.h" // for operator new, etc. #include "mozilla/mozInlineSpellChecker.h" // for mozInlineSpellChecker @@ -5870,12 +5871,15 @@ NS_IMETHODIMP EditorBase::SetNewlineHandling(int32_t aNewlineHandling) { bool EditorBase::IsSelectionRangeContainerNotContent() const { MOZ_ASSERT(IsEditActionDataAvailable()); - for (uint32_t i = 0; i < SelectionRef().RangeCount(); i++) { + const uint32_t rangeCount = SelectionRef().RangeCount(); + for (const uint32_t i : IntegerRange(rangeCount)) { + MOZ_ASSERT(SelectionRef().RangeCount() == rangeCount); const nsRange* range = SelectionRef().GetRangeAt(i); MOZ_ASSERT(range); - if (!range || !range->GetStartContainer() || - !range->GetStartContainer()->IsContent() || !range->GetEndContainer() || - !range->GetEndContainer()->IsContent()) { + if (MOZ_UNLIKELY(!range) || MOZ_UNLIKELY(!range->GetStartContainer()) || + MOZ_UNLIKELY(!range->GetStartContainer()->IsContent()) || + MOZ_UNLIKELY(!range->GetEndContainer()) || + MOZ_UNLIKELY(!range->GetEndContainer()->IsContent())) { return true; } } @@ -6259,9 +6263,13 @@ nsresult EditorBase::AutoEditActionDataSetter::MaybeDispatchBeforeInputEvent( else if (MayHaveTargetRangesOnHTMLEditor(inputType)) { if (uint32_t rangeCount = editorBase->SelectionRef().RangeCount()) { mTargetRanges.SetCapacity(rangeCount); - for (uint32_t i = 0; i < rangeCount; i++) { + for (const uint32_t i : IntegerRange(rangeCount)) { + MOZ_ASSERT(editorBase->SelectionRef().RangeCount() == rangeCount); const nsRange* range = editorBase->SelectionRef().GetRangeAt(i); - if (NS_WARN_IF(!range) || NS_WARN_IF(!range->IsPositioned())) { + MOZ_ASSERT(range); + MOZ_ASSERT(range->IsPositioned()); + if (MOZ_UNLIKELY(NS_WARN_IF(!range)) || + MOZ_UNLIKELY(NS_WARN_IF(!range->IsPositioned()))) { continue; } // Now, we need to fix the offset of target range because it may diff --git a/editor/libeditor/EditorUtils.cpp b/editor/libeditor/EditorUtils.cpp index d32f56fd2d4a..8e52f58b3f52 100644 --- a/editor/libeditor/EditorUtils.cpp +++ b/editor/libeditor/EditorUtils.cpp @@ -13,6 +13,7 @@ #include "gfxFontUtils.h" #include "mozilla/ComputedStyle.h" +#include "mozilla/IntegerRange.h" #include "mozilla/OwningNonNull.h" #include "mozilla/dom/Document.h" #include "mozilla/dom/HTMLBRElement.h" @@ -602,10 +603,11 @@ bool EditorUtils::IsPointInSelection(const Selection& aSelection, return false; } - uint32_t rangeCount = aSelection.RangeCount(); - for (uint32_t i = 0; i < rangeCount; i++) { + const uint32_t rangeCount = aSelection.RangeCount(); + for (const uint32_t i : IntegerRange(rangeCount)) { + MOZ_ASSERT(aSelection.RangeCount() == rangeCount); RefPtr range = aSelection.GetRangeAt(i); - if (!range) { + if (MOZ_UNLIKELY(NS_WARN_IF(!range))) { // Don't bail yet, iterate through them all continue; } diff --git a/editor/libeditor/EditorUtils.h b/editor/libeditor/EditorUtils.h index 1797713f787d..f806c70aa586 100644 --- a/editor/libeditor/EditorUtils.h +++ b/editor/libeditor/EditorUtils.h @@ -9,6 +9,7 @@ #include "mozilla/EditAction.h" #include "mozilla/EditorBase.h" #include "mozilla/EditorDOMPoint.h" +#include "mozilla/IntegerRange.h" #include "mozilla/RangeBoundary.h" #include "mozilla/Result.h" #include "mozilla/dom/Element.h" @@ -207,8 +208,8 @@ class MOZ_RAII AutoTransactionBatchExternal final { class MOZ_STACK_CLASS AutoSelectionRangeArray final { public: explicit AutoSelectionRangeArray(dom::Selection& aSelection) { - uint32_t rangeCount = aSelection.RangeCount(); - for (uint32_t i = 0; i < rangeCount; i++) { + for (const uint32_t i : IntegerRange(aSelection.RangeCount())) { + MOZ_ASSERT(aSelection.GetRangeAt(i)); mRanges.AppendElement(*aSelection.GetRangeAt(i)); } } @@ -231,7 +232,8 @@ class MOZ_STACK_CLASS AutoRangeArray final { void Initialize(const dom::Selection& aSelection) { mDirection = aSelection.GetDirection(); mRanges.Clear(); - for (uint32_t i = 0; i < aSelection.RangeCount(); i++) { + for (const uint32_t i : IntegerRange(aSelection.RangeCount())) { + MOZ_ASSERT(aSelection.GetRangeAt(i)); mRanges.AppendElement(aSelection.GetRangeAt(i)->CloneRange()); if (aSelection.GetRangeAt(i) == aSelection.GetAnchorFocusRange()) { mAnchorFocusRange = mRanges.LastElement(); diff --git a/editor/libeditor/HTMLEditHelpers.h b/editor/libeditor/HTMLEditHelpers.h index 62dead70ba5f..486876fc2cc5 100644 --- a/editor/libeditor/HTMLEditHelpers.h +++ b/editor/libeditor/HTMLEditHelpers.h @@ -16,6 +16,7 @@ #include "mozilla/Attributes.h" #include "mozilla/ContentIterator.h" #include "mozilla/EditorDOMPoint.h" +#include "mozilla/IntegerRange.h" #include "mozilla/RangeBoundary.h" #include "mozilla/dom/Element.h" #include "mozilla/dom/StaticRange.h" diff --git a/editor/libeditor/HTMLEditSubActionHandler.cpp b/editor/libeditor/HTMLEditSubActionHandler.cpp index 5f8e89324d7e..fc221cd7dfc8 100644 --- a/editor/libeditor/HTMLEditSubActionHandler.cpp +++ b/editor/libeditor/HTMLEditSubActionHandler.cpp @@ -21,6 +21,7 @@ #include "mozilla/Assertions.h" #include "mozilla/CheckedInt.h" #include "mozilla/ContentIterator.h" +#include "mozilla/IntegerRange.h" #include "mozilla/InternalMutationEvent.h" #include "mozilla/MathAlgorithms.h" #include "mozilla/OwningNonNull.h" @@ -6121,8 +6122,10 @@ void HTMLEditor::GetSelectionRangesExtendedToIncludeAdjuscentWhiteSpaces( MOZ_ASSERT(IsEditActionDataAvailable()); MOZ_ASSERT(aOutArrayOfRanges.IsEmpty()); - aOutArrayOfRanges.SetCapacity(SelectionRef().RangeCount()); - for (uint32_t i = 0; i < SelectionRef().RangeCount(); i++) { + const uint32_t rangeCount = SelectionRef().RangeCount(); + aOutArrayOfRanges.SetCapacity(rangeCount); + for (const uint32_t i : IntegerRange(rangeCount)) { + MOZ_ASSERT(SelectionRef().RangeCount() == rangeCount); const nsRange* selectionRange = SelectionRef().GetRangeAt(i); MOZ_ASSERT(selectionRange); @@ -6141,8 +6144,10 @@ void HTMLEditor::GetSelectionRangesExtendedToHardLineStartAndEnd( MOZ_ASSERT(IsEditActionDataAvailable()); MOZ_ASSERT(aOutArrayOfRanges.IsEmpty()); - aOutArrayOfRanges.SetCapacity(SelectionRef().RangeCount()); - for (uint32_t i = 0; i < SelectionRef().RangeCount(); i++) { + const uint32_t rangeCount = SelectionRef().RangeCount(); + aOutArrayOfRanges.SetCapacity(rangeCount); + for (const uint32_t i : IntegerRange(rangeCount)) { + MOZ_ASSERT(SelectionRef().RangeCount() == rangeCount); // Make a new adjusted range to represent the appropriate block content. // The basic idea is to push out the range endpoints to truly enclose the // blocks that we will affect. This call alters opRange. @@ -6626,8 +6631,11 @@ Element* HTMLEditor::GetParentListElementAtSelection() const { MOZ_ASSERT(IsEditActionDataAvailable()); MOZ_ASSERT(!IsSelectionRangeContainerNotContent()); - for (uint32_t i = 0; i < SelectionRef().RangeCount(); ++i) { + const uint32_t rangeCount = SelectionRef().RangeCount(); + for (const uint32_t i : IntegerRange(rangeCount)) { + MOZ_ASSERT(SelectionRef().RangeCount() == rangeCount); nsRange* range = SelectionRef().GetRangeAt(i); + MOZ_ASSERT(range); for (nsINode* parent = range->GetClosestCommonInclusiveAncestor(); parent; parent = parent->GetParentNode()) { if (HTMLEditUtils::IsAnyListElement(parent)) { diff --git a/editor/libeditor/HTMLEditUtils.h b/editor/libeditor/HTMLEditUtils.h index e96d537a2e64..709690b8eb9f 100644 --- a/editor/libeditor/HTMLEditUtils.h +++ b/editor/libeditor/HTMLEditUtils.h @@ -17,6 +17,7 @@ #include "mozilla/EditorDOMPoint.h" #include "mozilla/EditorUtils.h" #include "mozilla/EnumSet.h" +#include "mozilla/IntegerRange.h" #include "mozilla/Maybe.h" #include "mozilla/dom/AbstractRange.h" #include "mozilla/dom/AncestorIterator.h" @@ -2038,9 +2039,12 @@ class MOZ_STACK_CLASS SelectedTableCellScanner final { } mSelectedCellElements.SetCapacity(aSelection.RangeCount()); mSelectedCellElements.AppendElement(*firstSelectedCellElement); - for (uint32_t i = 1; i < aSelection.RangeCount(); i++) { + const uint32_t rangeCount = aSelection.RangeCount(); + for (const uint32_t i : IntegerRange(1u, rangeCount)) { + MOZ_ASSERT(aSelection.RangeCount() == rangeCount); nsRange* range = aSelection.GetRangeAt(i); - if (NS_WARN_IF(!range) || NS_WARN_IF(!range->IsPositioned())) { + if (MOZ_UNLIKELY(NS_WARN_IF(!range)) || + MOZ_UNLIKELY(NS_WARN_IF(!range->IsPositioned()))) { continue; // Shouldn't occur in normal conditions. } // Just ignore selection ranges which do not select only one table diff --git a/editor/libeditor/HTMLEditor.cpp b/editor/libeditor/HTMLEditor.cpp index 06d64d4c55fd..f0a5daa829a1 100644 --- a/editor/libeditor/HTMLEditor.cpp +++ b/editor/libeditor/HTMLEditor.cpp @@ -3490,9 +3490,11 @@ nsresult HTMLEditor::ReplaceTextWithTransaction( // Therefore, we might need to save/restore selection here. Maybe restoreSelection; if (!AllowsTransactionsToChangeSelection() && !ArePreservingSelection()) { - for (uint32_t i = 0; i < SelectionRef().RangeCount(); i++) { + const uint32_t rangeCount = SelectionRef().RangeCount(); + for (const uint32_t i : IntegerRange(rangeCount)) { + MOZ_ASSERT(SelectionRef().RangeCount() == rangeCount); const nsRange* range = SelectionRef().GetRangeAt(i); - if (!range) { + if (MOZ_UNLIKELY(!range)) { continue; } if ((range->GetStartContainer() == &aTextNode && @@ -4492,6 +4494,7 @@ SplitNodeResult HTMLEditor::DoSplitNode(const EditorDOMPoint& aStartOfRightNode, for (uint32_t j : IntegerRange(range.mSelection->RangeCount())) { const nsRange* r = range.mSelection->GetRangeAt(j); + MOZ_ASSERT(r); MOZ_ASSERT(r->IsPositioned()); // XXX Looks like that SavedRange should have mStart and mEnd which // are RangeBoundary. Then, we can avoid to compute offset here. @@ -4815,8 +4818,11 @@ nsresult HTMLEditor::DoJoinNodes(nsIContent& aContentToKeep, continue; } - for (uint32_t j = 0; j < range.mSelection->RangeCount(); ++j) { + const uint32_t rangeCount = range.mSelection->RangeCount(); + for (const uint32_t j : IntegerRange(rangeCount)) { + MOZ_ASSERT(range.mSelection->RangeCount() == rangeCount); const RefPtr r = range.mSelection->GetRangeAt(j); + MOZ_ASSERT(r); MOZ_ASSERT(r->IsPositioned()); range.mStartContainer = r->GetStartContainer(); range.mStartOffset = r->StartOffset(); @@ -5381,6 +5387,7 @@ nsresult HTMLEditor::SetCSSBackgroundColorWithTransaction( // XXX This is different from `SetInlinePropertyInternal()`. It uses // AutoSelectionRangeArray to store all ranges first. The result may be // different if mutation event listener changes the `Selection`. + // TODO: Store all selection ranges first since this updates the style. for (uint32_t i = 0; i < SelectionRef().RangeCount(); i++) { RefPtr range = SelectionRef().GetRangeAt(i); if (NS_WARN_IF(!range)) { @@ -5801,7 +5808,7 @@ Element* HTMLEditor::GetSelectionContainerElement() const { return nullptr; } } else { - uint32_t rangeCount = SelectionRef().RangeCount(); + const uint32_t rangeCount = SelectionRef().RangeCount(); MOZ_ASSERT(rangeCount, "If 0, Selection::IsCollapsed() should return true"); if (rangeCount == 1) { @@ -5831,8 +5838,10 @@ Element* HTMLEditor::GetSelectionContainerElement() const { } } } else { - for (uint32_t i = 0; i < rangeCount; i++) { + for (const uint32_t i : IntegerRange(rangeCount)) { + MOZ_ASSERT(SelectionRef().RangeCount() == rangeCount); const nsRange* range = SelectionRef().GetRangeAt(i); + MOZ_ASSERT(range); nsINode* startContainer = range->GetStartContainer(); if (!focusNode) { focusNode = startContainer; diff --git a/editor/libeditor/HTMLTableEditor.cpp b/editor/libeditor/HTMLTableEditor.cpp index 7fc5340d6b57..96063e7def89 100644 --- a/editor/libeditor/HTMLTableEditor.cpp +++ b/editor/libeditor/HTMLTableEditor.cpp @@ -2931,6 +2931,7 @@ NS_IMETHODIMP HTMLEditor::JoinTableCells(bool aMergeNonContiguousContents) { // Cleanup selection: remove ranges where cells were deleted uint32_t rangeCount = SelectionRef().RangeCount(); + // TODO: Rewriting this with reversed ranged-loop may make it simpler. RefPtr range; for (uint32_t i = 0; i < rangeCount; i++) { range = SelectionRef().GetRangeAt(i); diff --git a/editor/libeditor/SelectionState.cpp b/editor/libeditor/SelectionState.cpp index 400c70e813f3..c1c96acd3f55 100644 --- a/editor/libeditor/SelectionState.cpp +++ b/editor/libeditor/SelectionState.cpp @@ -8,7 +8,8 @@ #include "EditorUtils.h" // for EditorUtils #include "HTMLEditHelpers.h" // for JoinNodesDirection, SplitNodeDirection -#include "mozilla/Assertions.h" // for MOZ_ASSERT, etc. +#include "mozilla/Assertions.h" // for MOZ_ASSERT, etc. +#include "mozilla/IntegerRange.h" // for IntegerRange #include "mozilla/dom/RangeBinding.h" #include "mozilla/dom/Selection.h" // for Selection #include "nsAString.h" // for nsAString::Length @@ -53,9 +54,12 @@ void SelectionState::SaveSelection(Selection& aSelection) { } // now store the selection ranges - for (uint32_t i = 0; i < aSelection.RangeCount(); i++) { + const uint32_t rangeCount = aSelection.RangeCount(); + for (const uint32_t i : IntegerRange(rangeCount)) { + MOZ_ASSERT(aSelection.RangeCount() == rangeCount); const nsRange* range = aSelection.GetRangeAt(i); - if (NS_WARN_IF(!range)) { + MOZ_ASSERT(range); + if (MOZ_UNLIKELY(NS_WARN_IF(!range))) { continue; } mArray[i]->StoreRange(*range); diff --git a/editor/spellchecker/TextServicesDocument.cpp b/editor/spellchecker/TextServicesDocument.cpp index 9fc6847e5f8b..533c4e71a674 100644 --- a/editor/spellchecker/TextServicesDocument.cpp +++ b/editor/spellchecker/TextServicesDocument.cpp @@ -11,6 +11,7 @@ #include "mozilla/EditorUtils.h" // for AutoTransactionBatchExternal #include "mozilla/HTMLEditHelpers.h" // for JoinNodesDirection #include "mozilla/HTMLEditUtils.h" // for HTMLEditUtils +#include "mozilla/IntegerRange.h" // for IntegerRange #include "mozilla/mozalloc.h" // for operator new, etc #include "mozilla/OwningNonNull.h" #include "mozilla/UniquePtr.h" // for UniquePtr @@ -588,9 +589,10 @@ nsresult TextServicesDocument::LastSelectedBlock( // XXX: We may need to add some code here to make sure // the ranges are sorted in document appearance order! - for (uint32_t i = rangeCount; i > 0; i--) { - range = selection->GetRangeAt(i - 1, IgnoreErrors()); - if (!range) { + for (const uint32_t i : Reversed(IntegerRange(rangeCount))) { + MOZ_ASSERT(selection->RangeCount() == rangeCount); + range = selection->GetRangeAt(i); + if (MOZ_UNLIKELY(!range)) { return NS_OK; // XXX Really? } @@ -643,9 +645,7 @@ nsresult TextServicesDocument::LastSelectedBlock( // Create a range that extends from the end of the selection, // to the end of the document, then iterate forwards through // it till you find a text node! - - range = selection->GetRangeAt(rangeCount - 1, IgnoreErrors()); - + range = rangeCount > 0 ? selection->GetRangeAt(rangeCount - 1) : nullptr; if (!range) { return NS_ERROR_FAILURE; } @@ -2043,9 +2043,12 @@ nsresult TextServicesDocument::GetUncollapsedSelection( Maybe e1s2; Maybe e2s1; uint32_t startOffset, endOffset; - for (uint32_t i = 0; i < rangeCount; i++) { - range = selection->GetRangeAt(i, IgnoreErrors()); - NS_ENSURE_STATE(range); + for (const uint32_t i : IntegerRange(rangeCount)) { + MOZ_ASSERT(selection->RangeCount() == rangeCount); + range = selection->GetRangeAt(i); + if (MOZ_UNLIKELY(NS_WARN_IF(!range))) { + return NS_ERROR_FAILURE; + } nsresult rv = GetRangeEndPoints(range, getter_AddRefs(startContainer), &startOffset, diff --git a/extensions/spellcheck/src/mozInlineSpellChecker.cpp b/extensions/spellcheck/src/mozInlineSpellChecker.cpp index 19e74b1a9d7e..e84035ecc1b2 100644 --- a/extensions/spellcheck/src/mozInlineSpellChecker.cpp +++ b/extensions/spellcheck/src/mozInlineSpellChecker.cpp @@ -41,6 +41,7 @@ #include "mozilla/EditorUtils.h" #include "mozilla/EventListenerManager.h" #include "mozilla/HTMLEditor.h" +#include "mozilla/IntegerRange.h" #include "mozilla/Logging.h" #include "mozilla/RangeUtils.h" #include "mozilla/Services.h" @@ -1228,11 +1229,12 @@ nsresult mozInlineSpellChecker::DoSpellCheckSelection( // elements inside the selection nsTArray> ranges; - int32_t count = aSpellCheckSelection->RangeCount(); - - for (int32_t idx = 0; idx < count; idx++) { + const uint32_t rangeCount = aSpellCheckSelection->RangeCount(); + for (const uint32_t idx : IntegerRange(rangeCount)) { + MOZ_ASSERT(aSpellCheckSelection->RangeCount() == rangeCount); nsRange* range = aSpellCheckSelection->GetRangeAt(idx); - if (range) { + MOZ_ASSERT(range); + if (MOZ_LIKELY(range)) { ranges.AppendElement(range); } } @@ -1251,7 +1253,7 @@ nsresult mozInlineSpellChecker::DoSpellCheckSelection( mozInlineSpellStatus::CreateForRange(*this, nullptr); bool doneChecking; - for (int32_t idx = 0; idx < count; idx++) { + for (uint32_t idx : IntegerRange(rangeCount)) { // We can consider this word as "added" since we know it has no spell // check range over it that needs to be deleted. All the old ranges // were cleared above. We also need to clear the word count so that we @@ -1674,10 +1676,10 @@ nsresult mozInlineSpellChecker::ResumeCheck( ("%s: no active dictionary.", __FUNCTION__)); // no active dictionary - int32_t count = spellCheckSelection->RangeCount(); - for (int32_t index = count - 1; index >= 0; index--) { + for (const uint32_t index : + Reversed(IntegerRange(spellCheckSelection->RangeCount()))) { RefPtr checkRange = spellCheckSelection->GetRangeAt(index); - if (checkRange) { + if (MOZ_LIKELY(checkRange)) { RemoveRange(spellCheckSelection, checkRange); } } @@ -1737,11 +1739,11 @@ nsresult mozInlineSpellChecker::CleanupRangesInSelection( // can happen if the node containing a highlighted word was removed. if (!aSelection) return NS_ERROR_FAILURE; - int32_t count = aSelection->RangeCount(); - - for (int32_t index = 0; index < count; index++) { - nsRange* checkRange = aSelection->GetRangeAt(index); - if (checkRange) { + // TODO: Rewrite this with reversed ranged-loop, it might make this simpler. + int64_t count = aSelection->RangeCount(); + for (int64_t index = 0; index < count; index++) { + nsRange* checkRange = aSelection->GetRangeAt(static_cast(index)); + if (MOZ_LIKELY(checkRange)) { if (checkRange->Collapsed()) { RemoveRange(aSelection, checkRange); index--; diff --git a/layout/base/AccessibleCaretManager.cpp b/layout/base/AccessibleCaretManager.cpp index aa9060a5d1f4..980279e42634 100644 --- a/layout/base/AccessibleCaretManager.cpp +++ b/layout/base/AccessibleCaretManager.cpp @@ -1080,6 +1080,7 @@ nsIFrame* AccessibleCaretManager::GetFrameForFirstRangeStartOrLastRangeEnd( nodeOffset = range->StartOffset(); hint = CARET_ASSOCIATE_AFTER; } else { + MOZ_ASSERT(selection->RangeCount() > 0); range = selection->GetRangeAt(selection->RangeCount() - 1); startNode = range->GetEndContainer(); endNode = range->GetStartContainer(); diff --git a/layout/base/PresShell.cpp b/layout/base/PresShell.cpp index f5ddddc7bcd0..956656062f47 100644 --- a/layout/base/PresShell.cpp +++ b/layout/base/PresShell.cpp @@ -20,6 +20,7 @@ #include "mozilla/EventStates.h" #include "mozilla/GeckoMVMContext.h" #include "mozilla/IMEStateManager.h" +#include "mozilla/IntegerRange.h" #include "mozilla/MemoryReporting.h" #include "mozilla/dom/BrowserChild.h" #include "mozilla/Likely.h" @@ -5195,10 +5196,10 @@ already_AddRefed PresShell::RenderSelection( // iterate over each range and collect them into the rangeItems array. // This is done so that the size of selection can be determined so as // to allocate a surface area - uint32_t numRanges = aSelection->RangeCount(); - NS_ASSERTION(numRanges > 0, "RenderSelection called with no selection"); - - for (uint32_t r = 0; r < numRanges; r++) { + const uint32_t rangeCount = aSelection->RangeCount(); + NS_ASSERTION(rangeCount > 0, "RenderSelection called with no selection"); + for (const uint32_t r : IntegerRange(rangeCount)) { + MOZ_ASSERT(aSelection->RangeCount() == rangeCount); RefPtr range = aSelection->GetRangeAt(r); UniquePtr info = CreateRangePaintInfo(range, area, true); diff --git a/layout/base/nsLayoutUtils.cpp b/layout/base/nsLayoutUtils.cpp index ea08b0a7ecaf..04c757d9dced 100644 --- a/layout/base/nsLayoutUtils.cpp +++ b/layout/base/nsLayoutUtils.cpp @@ -61,6 +61,7 @@ #include "mozilla/gfx/2D.h" #include "mozilla/gfx/gfxVars.h" #include "mozilla/gfx/PathHelpers.h" +#include "mozilla/IntegerRange.h" #include "mozilla/layers/APZCCallbackHelper.h" #include "mozilla/layers/APZPublicUtils.h" // for apz::CalculatePendingDisplayPort #include "mozilla/layers/CompositorBridgeChild.h" @@ -9062,9 +9063,10 @@ nsRect nsLayoutUtils::GetSelectionBoundingRect(const Selection* aSel) { res = TransformFrameRectToAncestor(frame, res, relativeTo); } } else { - int32_t rangeCount = aSel->RangeCount(); RectAccumulator accumulator; - for (int32_t idx = 0; idx < rangeCount; ++idx) { + const uint32_t rangeCount = aSel->RangeCount(); + for (const uint32_t idx : IntegerRange(rangeCount)) { + MOZ_ASSERT(aSel->RangeCount() == rangeCount); nsRange* range = aSel->GetRangeAt(idx); nsRange::CollectClientRectsAndText( &accumulator, nullptr, range, range->GetStartContainer(), diff --git a/layout/generic/nsFrameSelection.cpp b/layout/generic/nsFrameSelection.cpp index f00c2bc4faeb..697dd6fd3b78 100644 --- a/layout/generic/nsFrameSelection.cpp +++ b/layout/generic/nsFrameSelection.cpp @@ -16,6 +16,7 @@ #include "mozilla/BasePrincipal.h" #include "mozilla/EventStates.h" #include "mozilla/HTMLEditor.h" +#include "mozilla/IntegerRange.h" #include "mozilla/Logging.h" #include "mozilla/PresShell.h" #include "mozilla/ScrollTypes.h" @@ -2488,7 +2489,7 @@ nsresult nsFrameSelection::TableSelection::HandleMouseUpOrDown( mDragSelectingCells, mStartSelectedCell.get()); #endif // First check if we are extending a block selection - uint32_t rangeCount = aNormalSelection.RangeCount(); + const uint32_t rangeCount = aNormalSelection.RangeCount(); if (rangeCount > 0 && aMouseEvent->IsShift() && mAppendStartSelectedCell && mAppendStartSelectedCell != aChildContent) { @@ -2539,11 +2540,12 @@ nsresult nsFrameSelection::TableSelection::HandleMouseUpOrDown( "rangeCount=%d\n", rangeCount); #endif - for (uint32_t i = 0; i < rangeCount; i++) { + for (const uint32_t i : IntegerRange(rangeCount)) { + MOZ_ASSERT(aNormalSelection.RangeCount() == rangeCount); // Strong reference, because sometimes we want to remove // this range, and then we might be the only owner. RefPtr range = aNormalSelection.GetRangeAt(i); - if (!range) { + if (MOZ_UNLIKELY(!range)) { return NS_ERROR_NULL_POINTER; } @@ -2984,7 +2986,8 @@ nsRange* nsFrameSelection::TableSelection::GetNextCellRange( const mozilla::dom::Selection& aNormalSelection) { MOZ_ASSERT(aNormalSelection.Type() == SelectionType::eNormal); - nsRange* range = aNormalSelection.GetRangeAt(mSelectedCellIndex); + nsRange* range = + aNormalSelection.GetRangeAt(AssertedCast(mSelectedCellIndex)); // Get first node in next range of selection - test if it's a cell if (!GetFirstCellNodeInRange(range)) { diff --git a/layout/printing/nsPrintJob.cpp b/layout/printing/nsPrintJob.cpp index e760dc0c55c3..61b9760ef2a5 100644 --- a/layout/printing/nsPrintJob.cpp +++ b/layout/printing/nsPrintJob.cpp @@ -23,6 +23,7 @@ #include "mozilla/dom/ContentChild.h" #include "mozilla/dom/HTMLCanvasElement.h" #include "mozilla/dom/ScriptSettings.h" +#include "mozilla/IntegerRange.h" #include "mozilla/PresShell.h" #include "mozilla/PresShellInlines.h" #include "mozilla/StaticPrefs_print.h" @@ -1635,9 +1636,9 @@ nsresult nsPrintJob::UpdateSelectionAndShrinkPrintObject( selectionPS->RemoveAllRanges(IgnoreErrors()); } if (selection && selectionPS) { - int32_t cnt = selection->RangeCount(); - int32_t inx; - for (inx = 0; inx < cnt; ++inx) { + const uint32_t rangeCount = selection->RangeCount(); + for (const uint32_t inx : IntegerRange(rangeCount)) { + MOZ_ASSERT(selection->RangeCount() == rangeCount); const RefPtr range{selection->GetRangeAt(inx)}; selectionPS->AddRangeAndSelectFramesAndNotifyListeners(*range, IgnoreErrors()); diff --git a/toolkit/components/find/nsWebBrowserFind.cpp b/toolkit/components/find/nsWebBrowserFind.cpp index 93ef711a02db..045b1dcf6235 100644 --- a/toolkit/components/find/nsWebBrowserFind.cpp +++ b/toolkit/components/find/nsWebBrowserFind.cpp @@ -429,8 +429,8 @@ nsresult nsWebBrowserFind::GetSearchLimits(nsRange* aSearchRange, NS_ENSURE_ARG_POINTER(aSel); // There is a selection. - uint32_t count = aSel->RangeCount(); - if (count < 1) { + const uint32_t rangeCount = aSel->RangeCount(); + if (rangeCount < 1) { return SetRangeAroundDocument(aSearchRange, aStartPt, aEndPt, aDoc); } @@ -458,7 +458,7 @@ nsresult nsWebBrowserFind::GetSearchLimits(nsRange* aSearchRange, if (!mFindBackwards && !aWrap) { // This isn't quite right, since the selection's ranges aren't // necessarily in order; but they usually will be. - range = aSel->GetRangeAt(count - 1); + range = aSel->GetRangeAt(rangeCount - 1); if (!range) { return NS_ERROR_UNEXPECTED; } @@ -496,7 +496,7 @@ nsresult nsWebBrowserFind::GetSearchLimits(nsRange* aSearchRange, } // Forward, wrapping: DocStart to SelEnd else if (!mFindBackwards && aWrap) { - range = aSel->GetRangeAt(count - 1); + range = aSel->GetRangeAt(rangeCount - 1); if (!range) { return NS_ERROR_UNEXPECTED; }