tor-browser

The Tor Browser
git clone https://git.dasho.dev/tor-browser.git
Log | Files | Refs | README | LICENSE

commit ac500ea570dbad10924a4d8ccf47c61c17e7c442
parent ad197aca581f92f0e1954d808acfb2d54ac15881
Author: Norisz Fay <nfay@mozilla.com>
Date:   Mon, 22 Dec 2025 21:43:32 +0200

Revert "Bug 2006613 - Optimize front-removal of various tree-ordered-arrays. r=smaug" for causing multiple failures and bustages

This reverts commit 4d02f992d0fca541ae33ab4eaade954c76a154af.

Diffstat:
Mdom/base/ChildIterator.cpp | 17++++++++---------
Mdom/base/ContentIterator.cpp | 32++++++++++++++++++++------------
Mdom/base/DirectionalityUtils.cpp | 6+++---
Mdom/base/Document.cpp | 18+++++++++---------
Mdom/base/DocumentOrShadowRoot.h | 8+++-----
Ddom/base/FastFrontRemovableArray.h | 143-------------------------------------------------------------------------------
Mdom/base/IdentifierMapEntry.h | 7++-----
Mdom/base/RadioGroupContainer.cpp | 23++++++++++++-----------
Mdom/base/RadioGroupContainer.h | 2+-
Mdom/base/RangeBoundary.h | 28+++++++++++++++-------------
Mdom/base/RangeUtils.cpp | 3++-
Mdom/base/ShadowRoot.cpp | 42+++++++++++++++++++++---------------------
Mdom/base/ShadowRoot.h | 2+-
Mdom/base/TreeOrderedArray.h | 19++++++++++++++-----
Mdom/base/TreeOrderedArrayInlines.h | 17++++++++---------
Mdom/base/moz.build | 1-
Mdom/base/nsContentUtils.cpp | 46++++++++++++++++++----------------------------
Mdom/base/nsINode.cpp | 9+++++++--
Mdom/html/HTMLFormControlsCollection.cpp | 41++++++++++++++++++++++++++---------------
Mdom/html/HTMLFormElement.cpp | 116++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------
Mdom/html/HTMLFormElement.h | 8++++++++
Mdom/html/HTMLSlotElement.cpp | 12++++++++----
Mdom/html/HTMLSlotElement.h | 5++---
Mdom/serializers/nsDocumentEncoder.cpp | 14++++----------
Mdom/xul/XULPersist.cpp | 8++++----
Mlayout/style/GeckoBindings.cpp | 20++++++++------------
Mlayout/style/GeckoBindings.h | 15+++------------
Mmfbt/Span.h | 13-------------
Mservo/components/style/gecko/wrapper.rs | 64+++++++++++++++++++++++++++++++++++++++++-----------------------
29 files changed, 334 insertions(+), 405 deletions(-)

diff --git a/dom/base/ChildIterator.cpp b/dom/base/ChildIterator.cpp @@ -60,13 +60,10 @@ Maybe<uint32_t> FlattenedChildIterator::GetIndexOf( const nsINode* aParent, const nsINode* aPossibleChild) { if (const auto* element = Element::FromNode(aParent)) { if (const auto* slot = HTMLSlotElement::FromNode(element)) { - const Span assigned = slot->AssignedNodes(); - if (!assigned.IsEmpty()) { - auto index = assigned.IndexOf(aPossibleChild); - if (index == assigned.npos) { - return Nothing(); - } - return Some(index); + const auto& assignedNodes = slot->AssignedNodes(); + if (!assignedNodes.IsEmpty()) { + auto index = assignedNodes.IndexOf(aPossibleChild); + return index == assignedNodes.NoIndex ? Nothing() : Some(index); } } else if (auto* shadowRoot = element->GetShadowRoot()) { return shadowRoot->ComputeIndexOf(aPossibleChild); @@ -78,7 +75,8 @@ Maybe<uint32_t> FlattenedChildIterator::GetIndexOf( nsIContent* FlattenedChildIterator::GetNextChild() { // If we're already in the inserted-children array, look there first if (mParentAsSlot) { - const Span assignedNodes = mParentAsSlot->AssignedNodes(); + const nsTArray<RefPtr<nsINode>>& assignedNodes = + mParentAsSlot->AssignedNodes(); if (mIsFirst) { mIsFirst = false; MOZ_ASSERT(mIndexInInserted == 0); @@ -137,7 +135,8 @@ nsIContent* FlattenedChildIterator::GetPreviousChild() { return nullptr; } if (mParentAsSlot) { - const Span assignedNodes = mParentAsSlot->AssignedNodes(); + const nsTArray<RefPtr<nsINode>>& assignedNodes = + mParentAsSlot->AssignedNodes(); MOZ_ASSERT(mIndexInInserted <= assignedNodes.Length()); if (mIndexInInserted == 0) { mIsFirst = true; diff --git a/dom/base/ContentIterator.cpp b/dom/base/ContentIterator.cpp @@ -617,11 +617,13 @@ nsIContent* ContentIteratorBase<NodeType>::GetDeepLastChild( nsIContent* node = aRoot; while (HTMLSlotElement* slot = HTMLSlotElement::FromNode(node)) { - auto assigned = slot->AssignedNodes(); // The deep last child of a slot should be the last slotted element of it - if (!assigned.IsEmpty()) { - node = assigned[assigned.Length() - 1]->AsContent(); - continue; + if (!slot->AssignedNodes().IsEmpty()) { + if (nsIContent* content = + nsIContent::FromNode(slot->AssignedNodes().LastElement())) { + node = content; + continue; + } } break; } @@ -669,10 +671,13 @@ nsIContent* ContentIteratorBase<NodeType>::GetNextSibling( } // Next sibling of a slotted node should be the next slotted node - auto assigned = slot->AssignedNodes(); - auto cur = assigned.IndexOf(aNode); - if (cur != assigned.npos && cur + 1 < assigned.Length()) { - return assigned[cur + 1]->AsContent(); + auto currentIndex = slot->AssignedNodes().IndexOf(aNode); + if (currentIndex < slot->AssignedNodes().Length() - 1) { + nsINode* nextSlottedNode = + slot->AssignedNodes().ElementAt(currentIndex + 1); + if (nextSlottedNode->IsContent()) { + return nextSlottedNode->AsContent(); + } } // Move on to assigned slot's next sibling aNode = slot; @@ -734,10 +739,13 @@ nsIContent* ContentIteratorBase<NodeType>::GetPrevSibling( break; } // prev sibling of a slotted node should be the prev slotted node - auto assigned = slot->AssignedNodes(); - auto cur = assigned.IndexOf(aNode); - if (cur != assigned.npos && cur != 0) { - return assigned[cur - 1]->AsContent(); + auto currentIndex = slot->AssignedNodes().IndexOf(aNode); + if (currentIndex > 0) { + nsINode* prevSlottedNode = + slot->AssignedNodes().ElementAt(currentIndex - 1); + if (prevSlottedNode->IsContent()) { + return prevSlottedNode->AsContent(); + } } aNode = slot; } diff --git a/dom/base/DirectionalityUtils.cpp b/dom/base/DirectionalityUtils.cpp @@ -268,10 +268,10 @@ static Directionality ComputeAutoDirectionality(Element* aElement, * https://html.spec.whatwg.org/#auto-directionality step 2 */ Directionality ComputeAutoDirectionFromAssignedNodes( - HTMLSlotElement* aSlot, Span<const RefPtr<nsINode>> aAssignedNodes, + HTMLSlotElement* aSlot, const nsTArray<RefPtr<nsINode>>& assignedNodes, bool aNotify) { // Step 2.1. For each node child of element's assigned nodes: - for (const RefPtr<nsINode>& assignedNode : aAssignedNodes) { + for (const RefPtr<nsINode>& assignedNode : assignedNodes) { // Step 2.1.1. Let childDirection be null. Directionality childDirection = Directionality::Unset; @@ -316,7 +316,7 @@ static Directionality ComputeAutoDirectionality(Element* aElement, // Step 2. If element is a slot element whose root is a shadow root and // element's assigned nodes are not empty: if (auto* slot = HTMLSlotElement::FromNode(aElement)) { - const Span assignedNodes = slot->AssignedNodes(); + const nsTArray<RefPtr<nsINode>>& assignedNodes = slot->AssignedNodes(); if (!assignedNodes.IsEmpty()) { MOZ_ASSERT(slot->IsInShadowTree()); return ComputeAutoDirectionFromAssignedNodes(slot, assignedNodes, diff --git a/dom/base/Document.cpp b/dom/base/Document.cpp @@ -574,7 +574,7 @@ void IdentifierMapEntry::Traverse( } bool IdentifierMapEntry::IsEmpty() { - return mIdContentList.IsEmpty() && !mNameContentList && + return mIdContentList->IsEmpty() && !mNameContentList && !mDocumentNameContentList && !mChangeCallbacks && !mImageElement; } @@ -623,11 +623,11 @@ void IdentifierMapEntry::FireChangeCallbacks(Element* aOldElement, void IdentifierMapEntry::AddIdElement(Element* aElement) { MOZ_ASSERT(aElement, "Must have element"); - MOZ_ASSERT(!mIdContentList.Contains(nullptr), "Why is null in our list?"); + MOZ_ASSERT(!mIdContentList->Contains(nullptr), "Why is null in our list?"); size_t index = mIdContentList.Insert(*aElement); if (index == 0) { - Element* oldElement = mIdContentList.SafeElementAt(1, nullptr); + Element* oldElement = mIdContentList->SafeElementAt(1); FireChangeCallbacks(oldElement, aElement); } } @@ -642,16 +642,15 @@ void IdentifierMapEntry::RemoveIdElement(Element* aElement) { // Only assert this in HTML documents for now as XUL does all sorts of weird // crap. NS_ASSERTION(!aElement->OwnerDoc()->IsHTMLDocument() || - mIdContentList.Contains(aElement), + mIdContentList->Contains(aElement), "Removing id entry that doesn't exist"); // XXXbz should this ever Compact() I guess when all the content is gone // we'll just get cleaned up in the natural order of things... - Element* currentElement = mIdContentList.SafeElementAt(0, nullptr); + Element* currentElement = mIdContentList->SafeElementAt(0); mIdContentList.RemoveElement(*aElement); if (currentElement == aElement) { - FireChangeCallbacks(currentElement, - mIdContentList.SafeElementAt(0, nullptr)); + FireChangeCallbacks(currentElement, mIdContentList->SafeElementAt(0)); } } @@ -665,7 +664,7 @@ void IdentifierMapEntry::SetImageElement(Element* aElement) { } void IdentifierMapEntry::ClearAndNotify() { - Element* currentElement = mIdContentList.SafeElementAt(0, nullptr); + Element* currentElement = mIdContentList->SafeElementAt(0); mIdContentList.Clear(); if (currentElement) { FireChangeCallbacks(currentElement, nullptr); @@ -11709,7 +11708,8 @@ void Document::RemoveColorSchemeMeta(HTMLMetaElement& aMeta) { void Document::RecomputeColorScheme() { auto oldColorScheme = mColorSchemeBits; mColorSchemeBits = 0; - for (const HTMLMetaElement* el : mColorSchemeMetaTags.AsSpan()) { + const nsTArray<HTMLMetaElement*>& elements = mColorSchemeMetaTags; + for (const HTMLMetaElement* el : elements) { nsAutoString content; if (!el->GetAttr(nsGkAtoms::content, content)) { continue; diff --git a/dom/base/DocumentOrShadowRoot.h b/dom/base/DocumentOrShadowRoot.h @@ -102,12 +102,10 @@ class DocumentOrShadowRoot { * * This is useful for stuff like QuerySelector optimization and such. */ - Span<Element* const> GetAllElementsForId( + const nsTArray<Element*>* GetAllElementsForId( const IdentifierMapEntry::DependentAtomOrString& aElementId) const { - if (IdentifierMapEntry* entry = mIdentifierMap.GetEntry(aElementId)) { - return entry->GetIdElements(); - } - return {}; + IdentifierMapEntry* entry = mIdentifierMap.GetEntry(aElementId); + return entry ? &entry->GetIdElements() : nullptr; } already_AddRefed<nsContentList> GetElementsByTagName( diff --git a/dom/base/FastFrontRemovableArray.h b/dom/base/FastFrontRemovableArray.h @@ -1,143 +0,0 @@ -/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ -/* vim: set ts=8 sts=2 et sw=2 tw=80: */ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -#ifndef mozilla_dom_FastFrontRemovableArray_h -#define mozilla_dom_FastFrontRemovableArray_h - -// An nsTArray of pointers where removing from the front is amortized constant -// time. - -#include "mozilla/Span.h" -#include "nsTArray.h" - -namespace mozilla::dom { - -template <typename T, size_t InlineCapacity = 0> -class FastFrontRemovableArray { - using InternalList = AutoTArray<T, InlineCapacity>; - - public: - static const size_t NoIndex = InternalList::NoIndex; - - operator Span<const T>() const { return AsSpan(); } - operator Span<T>() { return AsSpan(); } - - Span<const T> AsSpan() const { - return Span<const T>(mList).From(mIndexOfFirstElement); - } - Span<T> AsSpan() { return Span<T>(mList).From(mIndexOfFirstElement); } - - size_t Length() const { return mList.Length() - mIndexOfFirstElement; } - - bool IsEmpty() const { return Length() == 0; } - - void RemoveElementAt(size_t aIndex) { - if (aIndex == 0) { - mList[mIndexOfFirstElement++] = nullptr; - if (mIndexOfFirstElement > std::max(size_t(4), mList.Length() / 4)) { - // Compact the list if it gets too big. This shifts all the elements, - // which is expensive, so only do it if we have more than 4 elements - // wasted at the front, and more than a quarter of the list is wasted - // space in the front. - mList.RemoveElementsAt(0, mIndexOfFirstElement); - mIndexOfFirstElement = 0; - } - return; - } - mList.RemoveElementAt(aIndex + mIndexOfFirstElement); - if (IsEmpty()) { - Clear(); - } - } - - template <typename U> - void InsertElementAt(size_t aIndex, U* aElem) { - if (mIndexOfFirstElement && aIndex == 0) { - mList[--mIndexOfFirstElement] = aElem; - return; - } - mList.InsertElementAt(aIndex + mIndexOfFirstElement, aElem); - } - - T& operator[](size_t aIndex) { return mList[aIndex + mIndexOfFirstElement]; } - - const T& operator[](size_t aIndex) const { - return mList[aIndex + mIndexOfFirstElement]; - } - T& ElementAt(size_t aIndex) { return mList[aIndex + mIndexOfFirstElement]; } - const T& ElementAt(size_t aIndex) const { - return mList[aIndex + mIndexOfFirstElement]; - } - - T& SafeElementAt(size_t aIndex, T& aDef) { - return mList.SafeElementAt(aIndex + mIndexOfFirstElement, aDef); - } - - const T& SafeElementAt(size_t aIndex, const T& aDef) const { - return mList.SafeElementAt(aIndex + mIndexOfFirstElement, aDef); - } - - const T& FirstElement() const { return ElementAt(0); } - const T& LastElement() const { return mList.LastElement(); } - T& FirstElement() { return ElementAt(0); } - T& LastElement() { return mList.LastElement(); } - - template <typename U> - void AppendElement(U* aElem) { - mList.AppendElement(aElem); - } - - template <typename Item> - bool RemoveElement(const Item& aItem) { - auto i = IndexOf(aItem); - if (i == NoIndex) { - return false; - } - RemoveElementAt(i); - return true; - } - - template <typename Item> - bool Contains(const Item& aItem) const { - return IndexOf(aItem) != NoIndex; - } - - void Clear() { - mList.Clear(); - mIndexOfFirstElement = 0; - } - - template <typename Item> - size_t IndexOf(const Item& aItem) const { - auto index = mList.IndexOf(aItem, mIndexOfFirstElement); - if (index == NoIndex || mIndexOfFirstElement == 0) { - return index; - } - return index - mIndexOfFirstElement; - } - - private: - AutoTArray<T, InlineCapacity> mList; - size_t mIndexOfFirstElement = 0; -}; - -template <typename T, size_t InlineCap> -inline void ImplCycleCollectionUnlink( - FastFrontRemovableArray<T, InlineCap>& aField) { - aField.Clear(); -} - -template <typename T, size_t InlineCap, typename Callback> -inline void ImplCycleCollectionIndexedContainer( - FastFrontRemovableArray<T, InlineCap>& aField, Callback&& aCallback) { - for (auto& value : aField.AsSpan()) { - aCallback(value); - } -} - -} // namespace mozilla::dom - -#endif diff --git a/dom/base/IdentifierMapEntry.h b/dom/base/IdentifierMapEntry.h @@ -127,15 +127,12 @@ class IdentifierMapEntry : public PLDHashEntryHdr { * Returns the element if we know the element associated with this * id. Otherwise returns null. */ - Element* GetIdElement() const { - auto span = mIdContentList.AsSpan(); - return span.IsEmpty() ? nullptr : span[0]; - } + Element* GetIdElement() const { return mIdContentList->SafeElementAt(0); } /** * Returns the list of all elements associated with this id. */ - Span<Element* const> GetIdElements() const { return mIdContentList.AsSpan(); } + const nsTArray<Element*>& GetIdElements() const { return mIdContentList; } /** * If this entry has a non-null image element set (using SetImageElement), diff --git a/dom/base/RadioGroupContainer.cpp b/dom/base/RadioGroupContainer.cpp @@ -33,7 +33,7 @@ RadioGroupContainer::RadioGroupContainer() = default; RadioGroupContainer::~RadioGroupContainer() { for (const auto& group : mRadioGroups) { - for (const auto& button : group.GetData()->mRadioButtons.AsSpan()) { + for (const auto& button : group.GetData()->mRadioButtons.AsList()) { // When the radio group container is being cycle-collected, any remaining // connected buttons will also be in the process of being cycle-collected. // Here, we unset the button's reference to the container so that when it @@ -53,10 +53,11 @@ void RadioGroupContainer::Traverse(RadioGroupContainer* tmp, cb, "mRadioGroups entry->mSelectedRadioButton"); cb.NoteXPCOMChild(ToSupports(radioGroup->mSelectedRadioButton)); - for (auto& button : radioGroup->mRadioButtons.AsSpan()) { + uint32_t i, count = radioGroup->mRadioButtons->Length(); + for (i = 0; i < count; ++i) { NS_CYCLE_COLLECTION_NOTE_EDGE_NAME( cb, "mRadioGroups entry->mRadioButtons[i]"); - cb.NoteXPCOMChild(ToSupports(button)); + cb.NoteXPCOMChild(ToSupports(radioGroup->mRadioButtons->ElementAt(i))); } } } @@ -95,12 +96,12 @@ nsresult RadioGroupContainer::GetNextRadioButton( return NS_ERROR_FAILURE; } } - int32_t index = radioGroup->mRadioButtons.IndexOf(currentRadio); + int32_t index = radioGroup->mRadioButtons->IndexOf(currentRadio); if (index < 0) { return NS_ERROR_FAILURE; } - int32_t numRadios = static_cast<int32_t>(radioGroup->mRadioButtons.Length()); + int32_t numRadios = static_cast<int32_t>(radioGroup->mRadioButtons->Length()); RefPtr<HTMLInputElement> radio; do { if (aPrevious) { @@ -110,7 +111,7 @@ nsresult RadioGroupContainer::GetNextRadioButton( } else if (++index >= numRadios) { index = 0; } - radio = radioGroup->mRadioButtons.ElementAt(index); + radio = radioGroup->mRadioButtons->ElementAt(index); } while ((radio->Disabled() || !radio->GetPrimaryFrame() || !radio->GetPrimaryFrame()->IsVisibleConsideringAncestors()) && radio != currentRadio); @@ -122,7 +123,7 @@ nsresult RadioGroupContainer::GetNextRadioButton( HTMLInputElement* RadioGroupContainer::GetFirstRadioButton( const nsAString& aName) { nsRadioGroupStruct* radioGroup = GetOrCreateRadioGroup(aName); - for (HTMLInputElement* radio : radioGroup->mRadioButtons.AsSpan()) { + for (HTMLInputElement* radio : radioGroup->mRadioButtons.AsList()) { if (!radio->Disabled() && radio->GetPrimaryFrame() && radio->GetPrimaryFrame()->IsVisibleConsideringAncestors()) { return radio; @@ -145,7 +146,7 @@ void RadioGroupContainer::RemoveFromRadioGroup(const nsAString& aName, HTMLInputElement* aRadio) { nsRadioGroupStruct* radioGroup = GetOrCreateRadioGroup(aName); MOZ_ASSERT( - radioGroup->mRadioButtons.Contains(aRadio), + radioGroup->mRadioButtons->Contains(aRadio), "Attempting to remove radio button from group it is not a part of!"); radioGroup->mRadioButtons.RemoveElement(*aRadio); @@ -199,9 +200,9 @@ nsRadioGroupStruct* RadioGroupContainer::GetOrCreateRadioGroup( return mRadioGroups.GetOrInsertNew(aName); } -Span<const RefPtr<HTMLInputElement>> RadioGroupContainer::GetButtonsInGroup( - nsRadioGroupStruct* aGroup) const { - return aGroup->mRadioButtons.AsSpan(); +const nsTArray<RefPtr<HTMLInputElement>>& +RadioGroupContainer::GetButtonsInGroup(nsRadioGroupStruct* aGroup) const { + return aGroup->mRadioButtons.AsList(); } } // namespace mozilla::dom diff --git a/dom/base/RadioGroupContainer.h b/dom/base/RadioGroupContainer.h @@ -55,7 +55,7 @@ class RadioGroupContainer final { nsRadioGroupStruct* GetOrCreateRadioGroup(const nsAString& aName); private: - Span<const RefPtr<HTMLInputElement>> GetButtonsInGroup( + const nsTArray<RefPtr<HTMLInputElement>>& GetButtonsInGroup( nsRadioGroupStruct* aGroup) const; nsClassHashtable<nsStringHashKey, nsRadioGroupStruct> mRadioGroups; diff --git a/dom/base/RangeBoundary.h b/dom/base/RangeBoundary.h @@ -449,10 +449,12 @@ class RangeBoundaryBase { if (mTreeKind == TreeKind::Flat) { if (const auto* slot = dom::HTMLSlotElement::FromNode(mParent)) { - const Span assigned = slot->AssignedNodes(); - const auto index = assigned.IndexOf(aCurrentNode); - if (index != assigned.npos && index + 1 < assigned.Length()) { - if (auto* nextSibling = RawRefType::FromNode(assigned[index + 1])) { + const auto& assignedNodes = slot->AssignedNodes(); + const auto index = assignedNodes.IndexOf(aCurrentNode); + if (index != assignedNodes.NoIndex && + index + 1 < assignedNodes.Length()) { + if (RawRefType* nextSibling = + RawRefType::FromNode(assignedNodes.ElementAt(index + 1))) { return nextSibling; } return nullptr; @@ -466,9 +468,9 @@ class RangeBoundaryBase { MOZ_ASSERT(aNode); if (mTreeKind == TreeKind::Flat) { if (const auto* slot = dom::HTMLSlotElement::FromNode(aNode)) { - const Span assigned = slot->AssignedNodes(); - if (!assigned.IsEmpty()) { - if (RawRefType* child = RawRefType::FromNode(assigned[0])) { + const auto& assignedNodes = slot->AssignedNodes(); + if (!assignedNodes.IsEmpty()) { + if (RawRefType* child = RawRefType::FromNode(assignedNodes[0])) { return child; } return nullptr; @@ -504,9 +506,9 @@ class RangeBoundaryBase { MOZ_ASSERT(aNode); if (mTreeKind == TreeKind::Flat) { if (const auto* slot = dom::HTMLSlotElement::FromNode(aNode)) { - const Span assigned = slot->AssignedNodes(); - if (!assigned.IsEmpty()) { - return assigned.Length(); + const auto& assignedNodes = slot->AssignedNodes(); + if (!assignedNodes.IsEmpty()) { + return assignedNodes.Length(); } } @@ -529,9 +531,9 @@ class RangeBoundaryBase { MOZ_ASSERT(aParent); if (mTreeKind == TreeKind::Flat) { if (const auto* slot = dom::HTMLSlotElement::FromNode(aParent)) { - const Span assigned = slot->AssignedNodes(); - if (!assigned.IsEmpty()) { - return RawRefType::FromNode(assigned[assigned.Length() - 1]); + const auto& assignedNodes = slot->AssignedNodes(); + if (!assignedNodes.IsEmpty()) { + return RawRefType::FromNode(assignedNodes.LastElement()); } } if (const auto* shadowRoot = aParent->GetShadowRoot()) { diff --git a/dom/base/RangeUtils.cpp b/dom/base/RangeUtils.cpp @@ -240,7 +240,8 @@ nsresult RangeUtils::CompareNodeToRangeBoundaries( parent = aNode; nodeStart = 0; nodeEnd = aNode->GetChildCount(); - } else if (const auto* slotAsParent = HTMLSlotElement::FromNode(parent); + } else if (const HTMLSlotElement* slotAsParent = + HTMLSlotElement::FromNode(parent); slotAsParent && aKind == TreeKind::Flat) { // aNode is a slotted content, use the index in the assigned nodes // to represent this node. diff --git a/dom/base/ShadowRoot.cpp b/dom/base/ShadowRoot.cpp @@ -259,17 +259,18 @@ void ShadowRoot::AddSlot(HTMLSlotElement* aSlot) { InvalidateStyleAndLayoutOnSubtree(aSlot); - HTMLSlotElement* oldSlot = currentSlots.SafeElementAt(1, nullptr); + HTMLSlotElement* oldSlot = currentSlots->SafeElementAt(1); if (SlotAssignment() == SlotAssignmentMode::Named) { if (oldSlot) { MOZ_DIAGNOSTIC_ASSERT(oldSlot != aSlot); // Move assigned nodes from old slot to new slot. InvalidateStyleAndLayoutOnSubtree(oldSlot); + const nsTArray<RefPtr<nsINode>>& assignedNodes = oldSlot->AssignedNodes(); bool doEnqueueSlotChange = false; - auto assignedNodes = - ToTArray<AutoTArray<nsINode*, 8>>(oldSlot->AssignedNodes()); - for (nsINode* assignedNode : assignedNodes) { + while (assignedNodes.Length() > 0) { + nsINode* assignedNode = assignedNodes[0]; + oldSlot->RemoveAssignedNode(*assignedNode->AsContent()); aSlot->AppendAssignedNode(*assignedNode->AsContent()); doEnqueueSlotChange = true; @@ -326,11 +327,11 @@ void ShadowRoot::RemoveSlot(HTMLSlotElement* aSlot) { MOZ_ASSERT(mSlotMap.Get(name)); SlotArray& currentSlots = *mSlotMap.Get(name); - MOZ_DIAGNOSTIC_ASSERT(currentSlots.Contains(aSlot), + MOZ_DIAGNOSTIC_ASSERT(currentSlots->Contains(aSlot), "Slot to de-register wasn't found?"); - if (currentSlots.Length() == 1) { + if (currentSlots->Length() == 1) { MOZ_ASSERT_IF(SlotAssignment() == SlotAssignmentMode::Named, - currentSlots.ElementAt(0) == aSlot); + currentSlots->ElementAt(0) == aSlot); InvalidateStyleAndLayoutOnSubtree(aSlot); @@ -350,7 +351,7 @@ void ShadowRoot::RemoveSlot(HTMLSlotElement* aSlot) { } } - const bool wasFirstSlot = currentSlots.ElementAt(0) == aSlot; + const bool wasFirstSlot = currentSlots->ElementAt(0) == aSlot; currentSlots.RemoveElement(*aSlot); if (!wasFirstSlot || SlotAssignment() == SlotAssignmentMode::Manual) { return; @@ -359,15 +360,16 @@ void ShadowRoot::RemoveSlot(HTMLSlotElement* aSlot) { // Move assigned nodes from removed slot to the next slot in // tree order with the same name. InvalidateStyleAndLayoutOnSubtree(aSlot); - HTMLSlotElement* replacementSlot = currentSlots.ElementAt(0); - auto assignedNodes = - ToTArray<AutoTArray<nsINode*, 8>>(aSlot->AssignedNodes()); + HTMLSlotElement* replacementSlot = currentSlots->ElementAt(0); + const nsTArray<RefPtr<nsINode>>& assignedNodes = aSlot->AssignedNodes(); if (assignedNodes.IsEmpty()) { return; } InvalidateStyleAndLayoutOnSubtree(replacementSlot); - for (auto* assignedNode : assignedNodes) { + while (!assignedNodes.IsEmpty()) { + nsINode* assignedNode = assignedNodes[0]; + aSlot->RemoveAssignedNode(*assignedNode->AsContent()); replacementSlot->AppendAssignedNode(*assignedNode->AsContent()); } @@ -657,7 +659,7 @@ ShadowRoot::SlotInsertionPoint ShadowRoot::SlotInsertionPointFor( if (!slots) { return {}; } - slot = (*slots).ElementAt(0); + slot = (*slots)->ElementAt(0); } MOZ_ASSERT(slot); @@ -689,7 +691,7 @@ ShadowRoot::SlotInsertionPoint ShadowRoot::SlotInsertionPointFor( return {slot, Some(index)}; } } else { - const Span assignedNodes = slot->AssignedNodes(); + const nsTArray<RefPtr<nsINode>>& assignedNodes = slot->AssignedNodes(); nsIContent* currentContent = GetHost()->GetFirstChild(); for (uint32_t i = 0; i < assignedNodes.Length(); i++) { // Seek through the host's explicit children until the @@ -761,13 +763,11 @@ void ShadowRoot::MaybeReassignMainSummary(SummaryChangeReason aReason) { if (aReason == SummaryChangeReason::Insertion) { // We've inserted a summary element, may need to remove the existing one. SlotArray* array = mSlotMap.Get(u"internal-main-summary"_ns); - MOZ_RELEASE_ASSERT(array && (*array).Length() == 1); - HTMLSlotElement* slot = (*array).ElementAt(0); - auto assigned = slot->AssignedNodes(); - if (assigned.IsEmpty()) { - return; - } - if (auto* summary = HTMLSummaryElement::FromNode(assigned[0])) { + MOZ_RELEASE_ASSERT(array && (*array)->Length() == 1); + HTMLSlotElement* slot = (*array)->ElementAt(0); + auto* summary = HTMLSummaryElement::FromNodeOrNull( + slot->AssignedNodes().SafeElementAt(0)); + if (summary) { MaybeReassignContent(*summary); } } else if (MOZ_LIKELY(GetHost())) { diff --git a/dom/base/ShadowRoot.h b/dom/base/ShadowRoot.h @@ -192,7 +192,7 @@ class ShadowRoot final : public DocumentFragment, public DocumentOrShadowRoot { bool HasSlots() const { return !mSlotMap.IsEmpty(); }; HTMLSlotElement* GetDefaultSlot() const { SlotArray* list = mSlotMap.Get(u""_ns); - return list ? (*list).AsSpan()[0] : nullptr; + return list ? (*list)->ElementAt(0) : nullptr; } void PartAdded(const Element&); diff --git a/dom/base/TreeOrderedArray.h b/dom/base/TreeOrderedArray.h @@ -7,7 +7,7 @@ #ifndef mozilla_dom_TreeOrderedArray_h #define mozilla_dom_TreeOrderedArray_h -#include "FastFrontRemovableArray.h" +#include "nsTArray.h" class nsINode; template <typename T> @@ -17,9 +17,7 @@ namespace mozilla::dom { // A sorted tree-ordered list of pointers (either raw or RefPtr) to nodes. template <typename NodePointer> -class TreeOrderedArray : public FastFrontRemovableArray<NodePointer, 1> { - using Base = FastFrontRemovableArray<NodePointer, 1>; - +class TreeOrderedArray { template <typename T> struct RawTypeExtractor {}; @@ -36,6 +34,10 @@ class TreeOrderedArray : public FastFrontRemovableArray<NodePointer, 1> { using Node = typename RawTypeExtractor<NodePointer>::type; public: + operator const nsTArray<NodePointer>&() const { return mList; } + const nsTArray<NodePointer>& AsList() const { return mList; } + const nsTArray<NodePointer>* operator->() const { return &mList; } + // Inserts a node into the list, and returns the new index in the array. // // All the nodes in the list should be in the same subtree, and debug builds @@ -47,7 +49,14 @@ class TreeOrderedArray : public FastFrontRemovableArray<NodePointer, 1> { // You can provide a potential common ancestor to speed up comparisons, see // nsContentUtils::CompareTreePosition. That's only a hint. inline size_t Insert(Node&, nsINode* aCommonAncestor = nullptr); - bool RemoveElement(Node& aNode) { return Base::RemoveElement(&aNode); } + + bool RemoveElement(Node& aNode) { return mList.RemoveElement(&aNode); } + void RemoveElementAt(size_t aIndex) { mList.RemoveElementAt(aIndex); } + + void Clear() { mList.Clear(); } + + private: + AutoTArray<NodePointer, 1> mList; }; } // namespace mozilla::dom diff --git a/dom/base/TreeOrderedArrayInlines.h b/dom/base/TreeOrderedArrayInlines.h @@ -19,10 +19,8 @@ template <typename Node> size_t TreeOrderedArray<Node>::Insert(Node& aNode, nsINode* aCommonAncestor) { static_assert(std::is_base_of_v<nsINode, Node>, "Should be a node"); - auto span = Base::AsSpan(); - auto len = span.Length(); - if (!len) { - Base::AppendElement(&aNode); + if (mList.IsEmpty()) { + mList.AppendElement(&aNode); return 0; } @@ -41,15 +39,16 @@ size_t TreeOrderedArray<Node>::Insert(Node& aNode, nsINode* aCommonAncestor) { }; PositionComparator cmp{aNode, aCommonAncestor}; - if (cmp(span[len - 1]) > 0) { + if (cmp(mList.LastElement()) > 0) { // Appending is a really common case, optimize for it. - Base::AppendElement(&aNode); - return len; + auto index = mList.Length(); + mList.AppendElement(&aNode); + return index; } size_t idx; - BinarySearchIf(span, 0, len, cmp, &idx); - Base::InsertElementAt(idx, &aNode); + BinarySearchIf(mList, 0, mList.Length(), cmp, &idx); + mList.InsertElementAt(idx, &aNode); return idx; } diff --git a/dom/base/moz.build b/dom/base/moz.build @@ -190,7 +190,6 @@ EXPORTS.mozilla.dom += [ "EventSource.h", "EventSourceEventService.h", "External.h", - "FastFrontRemovableArray.h", "FilteredNodeIterator.h", "FormData.h", "FragmentDirective.h", diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp @@ -951,17 +951,20 @@ class MOZ_STACK_CLASS CommonAncestors final { return; } - bool found = false; + Maybe<uint32_t> childIndex; if constexpr (aKind == TreeKind::Flat) { if (auto* slot = HTMLSlotElement::FromNode(mClosestCommonAncestor)) { - found = slot->AssignedNodes().IndexOf(child) != span.npos; + auto index = slot->AssignedNodes().IndexOf(child); + if (index != nsTArray<RefPtr<nsINode>>::NoIndex) { + childIndex = Some(index); + } } } - if (!found) { - found = mClosestCommonAncestor->ComputeIndexOf(child).isSome(); + if (childIndex.isNothing()) { + childIndex = mClosestCommonAncestor->ComputeIndexOf(child); } - if (MOZ_LIKELY(found)) { + if (MOZ_LIKELY(childIndex.isSome())) { return; } const Maybe<size_t> index = @@ -3370,27 +3373,11 @@ Maybe<int32_t> nsContentUtils::CompareChildNodes( const auto* slot = aChild1->AsContent()->GetAssignedSlot(); MOZ_ASSERT(slot); - constexpr auto NoIndex = size_t(-1); - auto child1Index = NoIndex; - auto child2Index = NoIndex; - size_t index = 0; - for (nsINode* node : slot->AssignedNodes()) { - if (node == aChild1) { - child1Index = index; - if (child2Index != NoIndex) { - break; - } - } else if (node == aChild2) { - child2Index = index; - if (child1Index != NoIndex) { - break; - } - } - index++; - } + auto child1Index = slot->AssignedNodes().IndexOf(aChild1); + auto child2Index = slot->AssignedNodes().IndexOf(aChild2); - MOZ_ASSERT(child1Index != NoIndex); - MOZ_ASSERT(child2Index != NoIndex); + MOZ_ASSERT(child1Index != nsTArray<RefPtr<nsINode>>::NoIndex); + MOZ_ASSERT(child2Index != nsTArray<RefPtr<nsINode>>::NoIndex); return Some(child1Index < child2Index ? -1 : 1); } @@ -3567,12 +3554,15 @@ Maybe<int32_t> nsContentUtils::CompareChildOffsetAndChildNode( return Some(!aOffset1 ? 0 : 1); } - MOZ_ASSERT_IF(!isFlatAndSlotted, parentNode->GetLastChild()); +#ifdef DEBUG + if (!isFlatAndSlotted) { + MOZ_ASSERT(parentNode->GetLastChild()); + } +#endif const nsIContent& lastChild = [parentNode]() -> const nsIContent& { if constexpr (aKind == TreeKind::Flat) { if (const HTMLSlotElement* slot = HTMLSlotElement::FromNode(parentNode)) { - auto assigned = slot->AssignedNodes(); - return *assigned[assigned.Length() - 1]->AsContent(); + return *slot->AssignedNodes().LastElement()->AsContent(); } } diff --git a/dom/base/nsINode.cpp b/dom/base/nsINode.cpp @@ -3385,11 +3385,16 @@ inline static Element* FindMatchingElementWithId( aRoot.IsInUncomposedDoc() || aRoot.IsInShadowTree(), "Don't call me if the root is not in the document or in a shadow tree"); - Span elements = aContainingDocOrShadowRoot.GetAllElementsForId(aId); + const nsTArray<Element*>* elements = + aContainingDocOrShadowRoot.GetAllElementsForId(aId); + if (!elements) { + // Nothing to do; we're done + return nullptr; + } // XXXbz: Should we fall back to the tree walk if |elements| is long, // for some value of "long"? - for (Element* element : elements) { + for (Element* element : *elements) { if (MOZ_UNLIKELY(element == &aRoot)) { continue; } diff --git a/dom/html/HTMLFormControlsCollection.cpp b/dom/html/HTMLFormControlsCollection.cpp @@ -86,14 +86,14 @@ void HTMLFormControlsCollection::DropFormReference() { void HTMLFormControlsCollection::Clear() { // Null out childrens' pointer to me. No refcounting here - for (nsGenericHTMLFormElement* element : Reversed(mElements.AsSpan())) { + for (nsGenericHTMLFormElement* element : Reversed(mElements.AsList())) { nsCOMPtr<nsIFormControl> formControl = nsIFormControl::FromNode(element); MOZ_ASSERT(formControl); formControl->ClearForm(false, false); } mElements.Clear(); - for (nsGenericHTMLFormElement* element : Reversed(mNotInElements.AsSpan())) { + for (nsGenericHTMLFormElement* element : Reversed(mNotInElements.AsList())) { nsCOMPtr<nsIFormControl> formControl = nsIFormControl::FromNode(element); MOZ_ASSERT(formControl); formControl->ClearForm(false, false); @@ -131,7 +131,7 @@ NS_IMPL_CYCLE_COLLECTING_RELEASE(HTMLFormControlsCollection) // nsIHTMLCollection interfac -uint32_t HTMLFormControlsCollection::Length() { return mElements.Length(); } +uint32_t HTMLFormControlsCollection::Length() { return mElements->Length(); } nsISupports* HTMLFormControlsCollection::NamedItemInternal( const nsAString& aName) { @@ -154,7 +154,7 @@ nsresult HTMLFormControlsCollection::IndexOfContent(nsIContent* aContent, // Note -- not a DOM method; callers should handle flushing themselves NS_ENSURE_ARG_POINTER(aIndex); - *aIndex = mElements.IndexOf(aContent); + *aIndex = mElements->IndexOf(aContent); return NS_OK; } @@ -171,14 +171,17 @@ nsresult HTMLFormControlsCollection::RemoveElementFromTable( nsresult HTMLFormControlsCollection::GetSortedControls( nsTArray<RefPtr<nsGenericHTMLFormElement>>& aControls) const { +#ifdef DEBUG + HTMLFormElement::AssertDocumentOrder(mElements, mForm); + HTMLFormElement::AssertDocumentOrder(mNotInElements, mForm); +#endif + aControls.Clear(); // Merge the elements list and the not in elements list. Both lists are // already sorted. - auto elements = mElements.AsSpan(); - auto notInElements = mNotInElements.AsSpan(); - uint32_t elementsLen = elements.Length(); - uint32_t notInElementsLen = notInElements.Length(); + uint32_t elementsLen = mElements->Length(); + uint32_t notInElementsLen = mNotInElements->Length(); aControls.SetCapacity(elementsLen + notInElementsLen); uint32_t elementsIdx = 0; @@ -193,7 +196,8 @@ nsresult HTMLFormControlsCollection::GetSortedControls( // Append the remaining mNotInElements elements // XXX(Bug 1631371) Check if this should use a fallible operation as it // pretended earlier. - aControls.AppendElements(notInElements.From(notInElementsIdx)); + aControls.AppendElements(mNotInElements->Elements() + notInElementsIdx, + notInElementsLen - notInElementsIdx); break; } // Check whether we're done with mNotInElements @@ -203,22 +207,25 @@ nsresult HTMLFormControlsCollection::GetSortedControls( // Append the remaining mElements elements // XXX(Bug 1631371) Check if this should use a fallible operation as it // pretended earlier. - aControls.AppendElements(elements.From(elementsIdx)); + aControls.AppendElements(mElements->Elements() + elementsIdx, + elementsLen - elementsIdx); break; } // Both lists have elements left. - NS_ASSERTION(elements[elementsIdx] && notInElements[notInElementsIdx], + NS_ASSERTION(mElements->ElementAt(elementsIdx) && + mNotInElements->ElementAt(notInElementsIdx), "Should have remaining elements"); // Determine which of the two elements should be ordered // first and add it to the end of the list. nsGenericHTMLFormElement* elementToAdd; if (nsContentUtils::CompareTreePosition<TreeKind::DOM>( - elements[elementsIdx], notInElements[notInElementsIdx], mForm, + mElements->ElementAt(elementsIdx), + mNotInElements->ElementAt(notInElementsIdx), mForm, &indexCache) < 0) { - elementToAdd = elements[elementsIdx]; + elementToAdd = mElements->ElementAt(elementsIdx); ++elementsIdx; } else { - elementToAdd = notInElements[notInElementsIdx]; + elementToAdd = mNotInElements->ElementAt(notInElementsIdx); ++notInElementsIdx; } // Add the first element to the list. @@ -229,11 +236,15 @@ nsresult HTMLFormControlsCollection::GetSortedControls( NS_ASSERTION(aControls.Length() == elementsLen + notInElementsLen, "Not all form controls were added to the sorted list"); +#ifdef DEBUG + HTMLFormElement::AssertDocumentOrder(aControls, mForm); +#endif + return NS_OK; } Element* HTMLFormControlsCollection::GetElementAt(uint32_t aIndex) { - return mElements.SafeElementAt(aIndex, nullptr); + return mElements->SafeElementAt(aIndex, nullptr); } /* virtual */ diff --git a/dom/html/HTMLFormElement.cpp b/dom/html/HTMLFormElement.cpp @@ -262,10 +262,10 @@ void HTMLFormElement::MaybeSubmit(Element* aSubmitter) { // FIXME: Should be specified, see: // https://github.com/whatwg/html/issues/10066 { - for (nsGenericHTMLFormElement* el : mControls->mElements.AsSpan()) { + for (nsGenericHTMLFormElement* el : mControls->mElements.AsList()) { el->SetUserInteracted(true); } - for (nsGenericHTMLFormElement* el : mControls->mNotInElements.AsSpan()) { + for (nsGenericHTMLFormElement* el : mControls->mNotInElements.AsList()) { el->SetUserInteracted(true); } } @@ -397,14 +397,15 @@ nsresult HTMLFormElement::BindToTree(BindContext& aContext, nsINode& aParent) { } template <typename T> -static void MarkOrphans(Span<T*> aArray) { - for (auto* element : aArray) { - element->SetFlags(MAYBE_ORPHAN_FORM_ELEMENT); +static void MarkOrphans(const nsTArray<T*>& aArray) { + uint32_t length = aArray.Length(); + for (uint32_t i = 0; i < length; ++i) { + aArray[i]->SetFlags(MAYBE_ORPHAN_FORM_ELEMENT); } } static void CollectOrphans(nsINode* aRemovalRoot, - TreeOrderedArray<nsGenericHTMLFormElement*>& aArray + const nsTArray<nsGenericHTMLFormElement*>& aArray #ifdef DEBUG , HTMLFormElement* aThisForm @@ -449,7 +450,7 @@ static void CollectOrphans(nsINode* aRemovalRoot, } static void CollectOrphans(nsINode* aRemovalRoot, - const TreeOrderedArray<HTMLImageElement*>& aArray + const nsTArray<HTMLImageElement*>& aArray #ifdef DEBUG , HTMLFormElement* aThisForm @@ -495,9 +496,9 @@ void HTMLFormElement::UnbindFromTree(UnbindContext& aContext) { RefPtr<Document> oldDocument = GetUncomposedDoc(); // Mark all of our controls as maybe being orphans - MarkOrphans(mControls->mElements.AsSpan()); - MarkOrphans(mControls->mNotInElements.AsSpan()); - MarkOrphans(mImageElements.AsSpan()); + MarkOrphans(mControls->mElements.AsList()); + MarkOrphans(mControls->mNotInElements.AsList()); + MarkOrphans(mImageElements.AsList()); nsGenericHTMLElement::UnbindFromTree(aContext); @@ -636,7 +637,8 @@ nsresult HTMLFormElement::PostHandleEvent(EventChainPostVisitor& aVisitor) { nsresult HTMLFormElement::DoReset() { // Make sure the presentation is up-to-date - if (Document* doc = GetComposedDoc()) { + Document* doc = GetComposedDoc(); + if (doc) { doc->FlushPendingNotifications(FlushType::ContentAndNotify); } @@ -644,11 +646,8 @@ nsresult HTMLFormElement::DoReset() { uint32_t numElements = mControls->Length(); for (uint32_t elementX = 0; elementX < numElements; ++elementX) { // Hold strong ref in case the reset does something weird - if (elementX >= mControls->mElements.Length()) { - continue; - } - nsCOMPtr<nsIFormControl> controlNode = - nsIFormControl::FromNode(mControls->mElements[elementX]); + nsCOMPtr<nsIFormControl> controlNode = nsIFormControl::FromNodeOrNull( + mControls->mElements->SafeElementAt(elementX, nullptr)); if (controlNode) { controlNode->Reset(); } @@ -1116,11 +1115,65 @@ NotNull<const Encoding*> HTMLFormElement::GetSubmitEncoding() { } Element* HTMLFormElement::IndexedGetter(uint32_t aIndex, bool& aFound) { - Element* element = mControls->mElements.SafeElementAt(aIndex, nullptr); + Element* element = mControls->mElements->SafeElementAt(aIndex, nullptr); aFound = element != nullptr; return element; } +#ifdef DEBUG +/** + * Checks that all form elements are in document order. Asserts if any pair of + * consecutive elements are not in increasing document order. + * + * @param aControls List of form controls to check. + * @param aForm Parent form of the controls. + */ +/* static */ +void HTMLFormElement::AssertDocumentOrder( + const nsTArray<nsGenericHTMLFormElement*>& aControls, nsIContent* aForm) { + // TODO: remove the if directive with bug 598468. + // This is done to prevent asserts in some edge cases. +# if 0 + // Only iterate if aControls is not empty, since otherwise + // |aControls.Length() - 1| will be a very large unsigned number... not what + // we want here. + if (!aControls.IsEmpty()) { + for (uint32_t i = 0; i < aControls.Length() - 1; ++i) { + NS_ASSERTION( + CompareFormControlPosition(aControls[i], aControls[i + 1], aForm) < 0, + "Form controls not ordered correctly"); + } + } +# endif +} + +/** + * Copy of the above function, but with RefPtrs. + * + * @param aControls List of form controls to check. + * @param aForm Parent form of the controls. + */ +/* static */ +void HTMLFormElement::AssertDocumentOrder( + const nsTArray<RefPtr<nsGenericHTMLFormElement>>& aControls, + nsIContent* aForm) { + // TODO: remove the if directive with bug 598468. + // This is done to prevent asserts in some edge cases. +# if 0 + // Only iterate if aControls is not empty, since otherwise + // |aControls.Length() - 1| will be a very large unsigned number... not what + // we want here. + if (!aControls.IsEmpty()) { + for (uint32_t i = 0; i < aControls.Length() - 1; ++i) { + NS_ASSERTION( + CompareFormControlPosition(aControls[i], aControls[i + 1], aForm) < 0, + "Form controls not ordered correctly"); + } + } +# endif +} +#endif + nsresult HTMLFormElement::AddElement(nsGenericHTMLFormElement* aChild, bool aUpdateValidity, bool aNotify) { // If an element has a @form, we can assume it *might* be able to not have @@ -1136,7 +1189,11 @@ nsresult HTMLFormElement::AddElement(nsGenericHTMLFormElement* aChild, childInElements ? mControls->mElements : mControls->mNotInElements; const size_t insertedIndex = controlList.Insert(*aChild, this); - const bool lastElement = controlList.Length() == insertedIndex + 1; + const bool lastElement = controlList->Length() == insertedIndex + 1; + +#ifdef DEBUG + AssertDocumentOrder(controlList, this); +#endif auto type = fc->ControlType(); @@ -1240,8 +1297,8 @@ nsresult HTMLFormElement::RemoveElement(nsGenericHTMLFormElement* aChild, // Find the index of the child. This will be used later if necessary // to find the default submit. - size_t index = controls.IndexOf(aChild); - NS_ENSURE_STATE(index != controls.NoIndex); + size_t index = controls->IndexOf(aChild); + NS_ENSURE_STATE(index != controls.AsList().NoIndex); controls.RemoveElementAt(index); @@ -1252,13 +1309,13 @@ nsresult HTMLFormElement::RemoveElement(nsGenericHTMLFormElement* aChild, *firstSubmitSlot = nullptr; // We are removing the first submit in this list, find the new first submit - uint32_t length = controls.Length(); + uint32_t length = controls->Length(); for (uint32_t i = index; i < length; ++i) { const auto* currentControl = - nsIFormControl::FromNode(controls.ElementAt(i)); + nsIFormControl::FromNode(controls->ElementAt(i)); MOZ_ASSERT(currentControl); if (currentControl->IsSubmitControl()) { - *firstSubmitSlot = controls.ElementAt(i); + *firstSubmitSlot = controls->ElementAt(i); break; } } @@ -1602,14 +1659,13 @@ nsGenericHTMLFormElement* HTMLFormElement::GetDefaultSubmitElement() const { bool HTMLFormElement::ImplicitSubmissionIsDisabled() const { // Input text controls are always in the elements list. uint32_t numDisablingControlsFound = 0; - for (auto* element : mControls->mElements.AsSpan()) { - const auto* fc = nsIFormControl::FromNode(element); + uint32_t length = mControls->mElements->Length(); + for (uint32_t i = 0; i < length && numDisablingControlsFound < 2; ++i) { + const auto* fc = + nsIFormControl::FromNode(mControls->mElements->ElementAt(i)); MOZ_ASSERT(fc); if (fc->IsSingleLineTextControl(false)) { numDisablingControlsFound++; - if (numDisablingControlsFound > 1) { - break; - } } } return numDisablingControlsFound != 1; @@ -1619,7 +1675,7 @@ bool HTMLFormElement::IsLastActiveElement( const nsGenericHTMLFormElement* aElement) const { MOZ_ASSERT(aElement, "Unexpected call"); - for (auto* element : Reversed(mControls->mElements.AsSpan())) { + for (auto* element : Reversed(mControls->mElements.AsList())) { const auto* fc = nsIFormControl::FromNode(element); MOZ_ASSERT(fc); // XXX How about date/time control? @@ -1744,7 +1800,7 @@ int32_t HTMLFormElement::IndexOfContent(nsIContent* aContent) { } void HTMLFormElement::Clear() { - for (HTMLImageElement* image : Reversed(mImageElements.AsSpan())) { + for (HTMLImageElement* image : Reversed(mImageElements.AsList())) { image->ClearForm(false); } mImageElements.Clear(); diff --git a/dom/html/HTMLFormElement.h b/dom/html/HTMLFormElement.h @@ -335,6 +335,14 @@ class HTMLFormElement final : public nsGenericHTMLElement { void GetSupportedNames(nsTArray<nsString>& aRetval); +#ifdef DEBUG + static void AssertDocumentOrder( + const nsTArray<nsGenericHTMLFormElement*>& aControls, nsIContent* aForm); + static void AssertDocumentOrder( + const nsTArray<RefPtr<nsGenericHTMLFormElement>>& aControls, + nsIContent* aForm); +#endif + JS::ExpandoAndGeneration mExpandoAndGeneration; protected: diff --git a/dom/html/HTMLSlotElement.cpp b/dom/html/HTMLSlotElement.cpp @@ -113,7 +113,7 @@ static void FlattenAssignedNodes(HTMLSlotElement* aSlot, return; } - const Span<const RefPtr<nsINode>> assignedNodes = aSlot->AssignedNodes(); + const nsTArray<RefPtr<nsINode>>& assignedNodes = aSlot->AssignedNodes(); // If assignedNodes is empty, use children of slot as fallback content. if (assignedNodes.IsEmpty()) { @@ -148,7 +148,7 @@ void HTMLSlotElement::AssignedNodes(const AssignedNodesOptions& aOptions, return FlattenAssignedNodes(this, aNodes); } - aNodes.AppendElements(mAssignedNodes.AsSpan()); + aNodes = mAssignedNodes.Clone(); } void HTMLSlotElement::AssignedElements(const AssignedNodesOptions& aOptions, @@ -162,6 +162,10 @@ void HTMLSlotElement::AssignedElements(const AssignedNodesOptions& aOptions, } } +const nsTArray<RefPtr<nsINode>>& HTMLSlotElement::AssignedNodes() const { + return mAssignedNodes; +} + const nsTArray<nsINode*>& HTMLSlotElement::ManuallyAssignedNodes() const { return mManuallyAssignedNodes; } @@ -315,7 +319,7 @@ void HTMLSlotElement::AppendAssignedNode(nsIContent& aNode) { void HTMLSlotElement::RecalculateHasSlottedState() { bool hasSlotted = false; // Find the first node that makes this a slotted element. - for (const RefPtr<nsINode>& assignedNode : mAssignedNodes.AsSpan()) { + for (const RefPtr<nsINode>& assignedNode : mAssignedNodes) { if (auto* slot = HTMLSlotElement::FromNode(assignedNode)) { if (slot->IsInShadowTree() && !slot->State().HasState(ElementState::HAS_SLOTTED)) { @@ -348,7 +352,7 @@ void HTMLSlotElement::RemoveAssignedNode(nsIContent& aNode) { } void HTMLSlotElement::ClearAssignedNodes() { - for (RefPtr<nsINode>& node : mAssignedNodes.AsSpan()) { + for (RefPtr<nsINode>& node : mAssignedNodes) { MOZ_ASSERT(!node->AsContent()->GetAssignedSlot() || node->AsContent()->GetAssignedSlot() == this, "How exactly?"); diff --git a/dom/html/HTMLSlotElement.h b/dom/html/HTMLSlotElement.h @@ -7,7 +7,6 @@ #ifndef mozilla_dom_HTMLSlotElement_h #define mozilla_dom_HTMLSlotElement_h -#include "mozilla/dom/FastFrontRemovableArray.h" #include "nsGenericHTMLElement.h" #include "nsTArray.h" @@ -53,7 +52,7 @@ class HTMLSlotElement final : public nsGenericHTMLElement { void Assign(const Sequence<OwningElementOrText>& aNodes); // Helper methods - Span<const RefPtr<nsINode>> AssignedNodes() const { return mAssignedNodes; } + const nsTArray<RefPtr<nsINode>>& AssignedNodes() const; const nsTArray<nsINode*>& ManuallyAssignedNodes() const; void InsertAssignedNode(uint32_t aIndex, nsIContent&); void AppendAssignedNode(nsIContent&); @@ -76,7 +75,7 @@ class HTMLSlotElement final : public nsGenericHTMLElement { virtual ~HTMLSlotElement(); JSObject* WrapNode(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) final; - FastFrontRemovableArray<RefPtr<nsINode>> mAssignedNodes; + nsTArray<RefPtr<nsINode>> mAssignedNodes; nsTArray<nsINode*> mManuallyAssignedNodes; // Whether we're in the signal slot list of our unit of related similar-origin diff --git a/dom/serializers/nsDocumentEncoder.cpp b/dom/serializers/nsDocumentEncoder.cpp @@ -987,11 +987,8 @@ nsresult nsDocumentEncoder::NodeSerializer::SerializeToStringRecursive( ++counter; if (allowCrossShadowBoundary) { if (const auto* slot = HTMLSlotElement::FromNode(node)) { - auto assigned = slot->AssignedNodes(); - if (size_t(counter) < assigned.Length()) { - return assigned[counter]; - } - return nullptr; + auto* next = slot->AssignedNodes().SafeElementAt(counter); + return next; } } @@ -1263,11 +1260,8 @@ nsresult nsDocumentEncoder::RangeSerializer::SerializeChildrenOfContent( uint32_t aCurrentIndex) -> nsIContent* { if (mAllowCrossShadowBoundary == AllowRangeCrossShadowBoundary::Yes) { if (const auto* slot = HTMLSlotElement::FromNode(&aContent)) { - auto assigned = slot->AssignedNodes(); - if (++aCurrentIndex < assigned.Length()) { - return nsIContent::FromNode(assigned[aCurrentIndex]); - } - return nullptr; + auto* next = slot->AssignedNodes().SafeElementAt(++aCurrentIndex); + return nsIContent::FromNodeOrNull(next); } } diff --git a/dom/xul/XULPersist.cpp b/dom/xul/XULPersist.cpp @@ -171,13 +171,13 @@ nsresult XULPersist::ApplyPersistentAttributes() { // We want to hold strong refs to the elements while applying // persistent attributes, just in case. - const Span allElements = mDocument->GetAllElementsForId(id); - if (allElements.IsEmpty()) { + const nsTArray<Element*>* allElements = mDocument->GetAllElementsForId(id); + if (!allElements) { continue; } elements.Clear(); - elements.SetCapacity(allElements.Length()); - for (Element* element : allElements) { + elements.SetCapacity(allElements->Length()); + for (Element* element : *allElements) { elements.AppendObject(element); } diff --git a/layout/style/GeckoBindings.cpp b/layout/style/GeckoBindings.cpp @@ -160,12 +160,10 @@ void Gecko_DestroyAnonymousContentList(nsTArray<nsIContent*>* aAnonContent) { delete aAnonContent; } -RustSpan<const nsINode* const> Gecko_GetAssignedNodes(const Element* aElement) { +const nsTArray<RefPtr<nsINode>>* Gecko_GetAssignedNodes( + const Element* aElement) { MOZ_ASSERT(HTMLSlotElement::FromNode(aElement)); - Span<const RefPtr<nsINode>> span = - static_cast<const HTMLSlotElement*>(aElement)->AssignedNodes(); - return {reinterpret_cast<const nsINode* const*>(span.Elements()), - span.Length()}; + return &static_cast<const HTMLSlotElement*>(aElement)->AssignedNodes(); } void Gecko_GetQueryContainerSize(const Element* aElement, nscoord* aOutWidth, @@ -1607,20 +1605,18 @@ void Gecko_ContentList_AppendAll(nsSimpleContentList* aList, } } -RustSpan<const Element* const> Gecko_Document_GetElementsWithId( - const Document* aDoc, nsAtom* aId) { +const nsTArray<Element*>* Gecko_Document_GetElementsWithId(const Document* aDoc, + nsAtom* aId) { MOZ_ASSERT(aDoc); MOZ_ASSERT(aId); - auto span = aDoc->GetAllElementsForId(aId); - return {span.Elements(), span.Length()}; + return aDoc->GetAllElementsForId(aId); } -RustSpan<const Element* const> Gecko_ShadowRoot_GetElementsWithId( +const nsTArray<Element*>* Gecko_ShadowRoot_GetElementsWithId( const ShadowRoot* aShadowRoot, nsAtom* aId) { MOZ_ASSERT(aShadowRoot); MOZ_ASSERT(aId); - auto span = aShadowRoot->GetAllElementsForId(aId); - return {span.Elements(), span.Length()}; + return aShadowRoot->GetAllElementsForId(aId); } static StyleComputedMozPrefFeatureValue GetPrefValue(const nsCString& aPref) { diff --git a/layout/style/GeckoBindings.h b/layout/style/GeckoBindings.h @@ -55,15 +55,6 @@ const bool GECKO_IS_NIGHTLY = true; const bool GECKO_IS_NIGHTLY = false; #endif -template <typename T> -struct RustSpan { - T* begin; - size_t length; -}; - -template struct RustSpan<const mozilla::dom::Element* const>; -template struct RustSpan<const nsINode* const>; - #define NS_DECL_THREADSAFE_FFI_REFCOUNTING(class_, name_) \ void Gecko_AddRef##name_##ArbitraryThread(class_* aPtr); \ void Gecko_Release##name_##ArbitraryThread(class_* aPtr); @@ -92,7 +83,7 @@ const nsINode* Gecko_GetFlattenedTreeParentNode(const nsINode*); void Gecko_GetAnonymousContentForElement(const mozilla::dom::Element*, nsTArray<nsIContent*>*); -RustSpan<const nsINode* const> Gecko_GetAssignedNodes( +const nsTArray<RefPtr<nsINode>>* Gecko_GetAssignedNodes( const mozilla::dom::Element*); void Gecko_GetQueryContainerSize(const mozilla::dom::Element*, @@ -556,10 +547,10 @@ void Gecko_ContentList_AppendAll(nsSimpleContentList* aContentList, // FIXME(emilio): These two below should be a single function that takes a // `const DocumentOrShadowRoot*`, but that doesn't make MSVC builds happy for a // reason I haven't really dug into. -RustSpan<const mozilla::dom::Element* const> Gecko_Document_GetElementsWithId( +const nsTArray<mozilla::dom::Element*>* Gecko_Document_GetElementsWithId( const mozilla::dom::Document*, nsAtom* aId); -RustSpan<const mozilla::dom::Element* const> Gecko_ShadowRoot_GetElementsWithId( +const nsTArray<mozilla::dom::Element*>* Gecko_ShadowRoot_GetElementsWithId( const mozilla::dom::ShadowRoot*, nsAtom* aId); bool Gecko_EvalMozPrefFeature(nsAtom*, diff --git a/mfbt/Span.h b/mfbt/Span.h @@ -382,7 +382,6 @@ class MOZ_GSL_POINTER Span { using const_reverse_iterator = std::reverse_iterator<const_iterator>; constexpr static const index_type extent = Extent; - constexpr static const index_type npos = index_type(-1); // [Span.cons], Span constructors, copy, assignment, and destructor // "Dependent" is needed to make "std::enable_if_t<(Dependent || @@ -832,18 +831,6 @@ class MOZ_GSL_POINTER Span { return {Elements(), Length()}; } - // Returns the index of the given element in the span, or `npos` otherwise. - template <typename Item> - index_type IndexOf(const Item& aItem) const { - auto begin = this->begin(); - auto end = this->end(); - auto it = std::find(begin, end, aItem); - if (it == end) { - return npos; - } - return index_type(it - begin); - } - private: // this implementation detail class lets us take advantage of the // empty base class optimization to pay for only storage of a single diff --git a/servo/components/style/gecko/wrapper.rs b/servo/components/style/gecko/wrapper.rs @@ -93,14 +93,17 @@ use std::sync::atomic::{AtomicU32, Ordering}; #[inline] fn elements_with_id<'a, 'le>( - array: structs::RustSpan<*const RawGeckoElement>, + array: *const structs::nsTArray<*mut RawGeckoElement>, ) -> &'a [GeckoElement<'le>] { unsafe { - let elements: &[*const RawGeckoElement] = - std::slice::from_raw_parts(array.begin, array.length); + if array.is_null() { + return &[]; + } + + let elements: &[*mut RawGeckoElement] = &**array; // NOTE(emilio): We rely on the in-memory representation of - // GeckoElement<'ld> and *const RawGeckoElement being the same. + // GeckoElement<'ld> and *mut RawGeckoElement being the same. #[allow(dead_code)] unsafe fn static_assert() { mem::transmute::<*mut RawGeckoElement, GeckoElement<'static>>(0xbadc0de as *mut _); @@ -286,14 +289,14 @@ impl<'ln> GeckoNode<'ln> { } fn flags_atomic_for(flags: &Cell<u32>) -> &AtomicU32 { - const_assert!(mem::size_of::<Cell<u32>>() == mem::size_of::<AtomicU32>()); - const_assert!(mem::align_of::<Cell<u32>>() == mem::align_of::<AtomicU32>()); + const_assert!(std::mem::size_of::<Cell<u32>>() == std::mem::size_of::<AtomicU32>()); + const_assert!(std::mem::align_of::<Cell<u32>>() == std::mem::align_of::<AtomicU32>()); // Rust doesn't provide standalone atomic functions like GCC/clang do // (via the atomic intrinsics) or via std::atomic_ref, but it guarantees // that the memory representation of u32 and AtomicU32 matches: // https://doc.rust-lang.org/std/sync/atomic/struct.AtomicU32.html - unsafe { mem::transmute::<&Cell<u32>, &AtomicU32>(flags) } + unsafe { std::mem::transmute::<&Cell<u32>, &AtomicU32>(flags) } } #[inline] @@ -589,7 +592,7 @@ pub enum GeckoChildrenIterator<'a> { /// replaces it with the next sibling when requested. Current(Option<GeckoNode<'a>>), /// A Gecko-implemented iterator we need to drop appropriately. - GeckoIterator(mem::ManuallyDrop<structs::StyleChildrenIterator>), + GeckoIterator(std::mem::ManuallyDrop<structs::StyleChildrenIterator>), } impl<'a> Drop for GeckoChildrenIterator<'a> { @@ -1068,10 +1071,10 @@ impl<'le> TElement for GeckoElement<'le> { || self.may_have_anonymous_children() { unsafe { - let mut iter = mem::MaybeUninit::<structs::StyleChildrenIterator>::uninit(); + let mut iter = std::mem::MaybeUninit::<structs::StyleChildrenIterator>::uninit(); bindings::Gecko_ConstructStyleChildrenIterator(self.0, iter.as_mut_ptr()); return LayoutIterator(GeckoChildrenIterator::GeckoIterator( - mem::ManuallyDrop::new(iter.assume_init()), + std::mem::ManuallyDrop::new(iter.assume_init()), )); } } @@ -1140,23 +1143,38 @@ impl<'le> TElement for GeckoElement<'le> { if !self.is_html_slot_element() || !self.as_node().is_in_shadow_tree() { return &[]; } + + let slot: &structs::HTMLSlotElement = unsafe { mem::transmute(self.0) }; + if cfg!(debug_assertions) { - let slot: &structs::HTMLSlotElement = unsafe { mem::transmute(self.0) }; let base: &RawGeckoElement = &slot._base._base._base; assert_eq!(base as *const _, self.0 as *const _, "Bad cast"); } - unsafe { - let nodes = bindings::Gecko_GetAssignedNodes(self.0); - let nodes: &[*const RawGeckoNode] = - std::slice::from_raw_parts(nodes.begin, nodes.length); - // NOTE(emilio): We rely on the in-memory representation of - // GeckoNode<'ld> and *const RawGeckoNode being the same. - #[allow(dead_code)] - unsafe fn static_assert() { - mem::transmute::<*mut RawGeckoNode, GeckoNode<'static>>(0xbadc0de as *mut _); - } - mem::transmute(nodes) - } + + // FIXME(emilio): Workaround a bindgen bug on Android that causes + // mAssignedNodes to be at the wrong offset. See bug 1466406. + // + // Bug 1466580 tracks running the Android layout tests on automation. + // + // The actual bindgen bug still needs reduction. + let assigned_nodes: &[structs::RefPtr<structs::nsINode>] = if !cfg!(target_os = "android") { + debug_assert_eq!( + unsafe { bindings::Gecko_GetAssignedNodes(self.0) }, + &slot.mAssignedNodes as *const _, + ); + + &*slot.mAssignedNodes + } else { + unsafe { &**bindings::Gecko_GetAssignedNodes(self.0) } + }; + + debug_assert_eq!( + mem::size_of::<structs::RefPtr<structs::nsINode>>(), + mem::size_of::<Self::ConcreteNode>(), + "Bad cast!" + ); + + unsafe { mem::transmute(assigned_nodes) } } #[inline]