Bug 1378201 - Improve the performance of TableRowsCollection, r=ehsan

MozReview-Commit-ID: 4joB73SXNGA

dom/html/HTMLTableElement.cpp

@@ -24,8 +24,9 @@ namespace dom {
  * This class provides a late-bound collection of rows in a table.
  * mParent is NOT ref-counted to avoid circular references
  */
-class TableRowsCollection final : public nsIHTMLCollection,
+class TableRowsCollection final : public nsIHTMLCollection
-                                  public nsWrapperCache
+                                , public nsStubMutationObserver
+                                , public nsWrapperCache
 {
 public:
   explicit TableRowsCollection(HTMLTableElement* aParent);
@@ -33,6 +34,11 @@ public:
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_NSIDOMHTMLCOLLECTION
 
+  NS_DECL_NSIMUTATIONOBSERVER_CONTENTAPPENDED
+  NS_DECL_NSIMUTATIONOBSERVER_CONTENTINSERTED
+  NS_DECL_NSIMUTATIONOBSERVER_CONTENTREMOVED
+  NS_DECL_NSIMUTATIONOBSERVER_NODEWILLBEDESTROYED
+
   virtual Element* GetElementAt(uint32_t aIndex) override;
   virtual nsINode* GetParentObject() override
   {
@@ -45,14 +51,27 @@ public:
 
   NS_IMETHOD    ParentDestroyed();
 
-  NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(TableRowsCollection)
+  NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS_AMBIGUOUS(TableRowsCollection, nsIHTMLCollection)
 
   // nsWrapperCache
   using nsWrapperCache::GetWrapperPreserveColor;
   using nsWrapperCache::PreserveWrapper;
   virtual JSObject* WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) override;
 protected:
-  virtual ~TableRowsCollection();
+  // Unregister ourselves as a mutation observer, and clear our internal state.
+  void CleanUp();
+  void LastRelease()
+  {
+    CleanUp();
+  }
+  virtual ~TableRowsCollection()
+  {
+    // we do NOT have a ref-counted reference to mParent, so do NOT
+    // release it!  this is to avoid circular references.  The
+    // instantiator who provided mParent is responsible for managing our
+    // reference for us.
+    CleanUp();
+  }
 
   virtual JSObject* GetWrapperPreserveColorInternal() override
   {
@@ -63,22 +82,126 @@ protected:
     nsWrapperCache::PreserveWrapper(aScriptObjectHolder);
   }
 
-  // Those rows that are not in table sections
+  // Ensure that HTMLTableElement is in a valid state. This must be called
+  // before inspecting the mRows object.
+  void EnsureInitialized();
+
+  // Checks if the passed-in container is interesting for the purposes of
+  // invalidation due to a mutation observer.
+  bool InterestingContainer(nsIContent* aContainer);
+
+  // Check if the passed-in nsIContent is a <tr> within the section defined by
+  // `aSection`. The root of the table is considered to be part of the `<tbody>`
+  // section.
+  bool IsAppropriateRow(nsIAtom* aSection, nsIContent* aContent);
+
+  // Scan backwards starting from `aCurrent` in the table, looking for the
+  // previous row in the table which is within the section `aSection`.
+  nsIContent* PreviousRow(nsIAtom* aSection, nsIContent* aCurrent);
+
+  // Handle the insertion of the child `aChild` into the container `aContainer`
+  // within the tree. The container must be an `InterestingContainer`. This
+  // method updates the mRows, mBodyStart, and mFootStart member variables.
+  //
+  // HandleInsert returns an integer which can be passed to the next call of the
+  // method in a loop inserting children into the same container. This will
+  // optimize subsequent insertions to require less work. This can either be -1,
+  // in which case we don't know where to insert the next row, and When passed
+  // to HandleInsert, it will use `PreviousRow` to locate the index to insert.
+  // Or, it can be an index to insert the next <tr> in the same container at.
+  int32_t HandleInsert(nsIContent* aContainer,
+                       nsIContent* aChild,
+                       int32_t aIndexGuess = -1);
+
+  // The HTMLTableElement which this TableRowsCollection tracks the rows for.
   HTMLTableElement* mParent;
+
+  // The current state of the TableRowsCollection. mBodyStart and mFootStart are
+  // indices into mRows which represent the location of the first row in the
+  // body or foot section. If there are no rows in a section, the index points
+  // at the location where the first element in that section would be inserted.
+  nsTArray<nsCOMPtr<nsIContent>> mRows;
+  uint32_t mBodyStart;
+  uint32_t mFootStart;
+  bool mInitialized;
 };
 
 
 TableRowsCollection::TableRowsCollection(HTMLTableElement *aParent)
   : mParent(aParent)
+  , mBodyStart(0)
+  , mFootStart(0)
+  , mInitialized(false)
 {
+  MOZ_ASSERT(mParent);
 }
 
-TableRowsCollection::~TableRowsCollection()
+void
+TableRowsCollection::EnsureInitialized()
 {
-  // we do NOT have a ref-counted reference to mParent, so do NOT
+  if (mInitialized) {
-  // release it!  this is to avoid circular references.  The
+    return;
-  // instantiator who provided mParent is responsible for managing our
+  }
-  // reference for us.
+  mInitialized = true;
+
+  // Initialize mRows as the TableRowsCollection is created. The mutation
+  // observer should keep it up to date.
+  //
+  // It should be extremely unlikely that anyone creates a TableRowsCollection
+  // without calling a method on it, so lazily performing this initialization
+  // seems unnecessary.
+  AutoTArray<nsCOMPtr<nsIContent>, 32> body;
+  AutoTArray<nsCOMPtr<nsIContent>, 32> foot;
+  mRows.Clear();
+
+  auto addRowChildren = [&] (nsTArray<nsCOMPtr<nsIContent>>& aArray, nsIContent* aNode) {
+    for (nsIContent* inner = aNode->nsINode::GetFirstChild();
+         inner; inner = inner->GetNextSibling()) {
+      if (inner->IsHTMLElement(nsGkAtoms::tr)) {
+        aArray.AppendElement(inner);
+      }
+    }
+  };
+
+  for (nsIContent* node = mParent->nsINode::GetFirstChild();
+       node; node = node->GetNextSibling()) {
+    if (node->IsHTMLElement(nsGkAtoms::thead)) {
+      addRowChildren(mRows, node);
+    } else if (node->IsHTMLElement(nsGkAtoms::tbody)) {
+      addRowChildren(body, node);
+    } else if (node->IsHTMLElement(nsGkAtoms::tfoot)) {
+      addRowChildren(foot, node);
+    } else if (node->IsHTMLElement(nsGkAtoms::tr)) {
+      body.AppendElement(node);
+    }
+  }
+
+  mBodyStart = mRows.Length();
+  mRows.AppendElements(Move(body));
+  mFootStart = mRows.Length();
+  mRows.AppendElements(Move(foot));
+
+  mParent->AddMutationObserver(this);
+}
+
+void
+TableRowsCollection::CleanUp()
+{
+  // Unregister ourselves as a mutation observer.
+  if (mInitialized && mParent)  {
+    mParent->RemoveMutationObserver(this);
+  }
+
+  // Clean up all of our internal state and make it empty in case someone looks
+  // at us.
+  mRows.Clear();
+  mBodyStart = 0;
+  mFootStart = 0;
+
+  // We set mInitialized to true in case someone still has a reference to us, as
+  // we don't need to try to initialize first.
+  mInitialized = true;
+  mParent = nullptr;
 }
 
 JSObject*
@@ -87,136 +210,33 @@ TableRowsCollection::WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProt
   return HTMLCollectionBinding::Wrap(aCx, this, aGivenProto);
 }
 
-NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE_0(TableRowsCollection)
+NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE(TableRowsCollection, mRows)
 NS_IMPL_CYCLE_COLLECTING_ADDREF(TableRowsCollection)
-NS_IMPL_CYCLE_COLLECTING_RELEASE(TableRowsCollection)
+NS_IMPL_CYCLE_COLLECTING_RELEASE_WITH_LAST_RELEASE(TableRowsCollection,
+                                                   LastRelease())
 
 NS_INTERFACE_TABLE_HEAD(TableRowsCollection)
   NS_WRAPPERCACHE_INTERFACE_TABLE_ENTRY
   NS_INTERFACE_TABLE(TableRowsCollection, nsIHTMLCollection,
-                     nsIDOMHTMLCollection)
+                     nsIDOMHTMLCollection, nsIMutationObserver)
   NS_INTERFACE_TABLE_TO_MAP_SEGUE_CYCLE_COLLECTION(TableRowsCollection)
 NS_INTERFACE_MAP_END
 
-// Macro that can be used to avoid copy/pasting code to iterate over the
-// rowgroups.  _code should be the code to execute for each rowgroup.  The
-// rowgroup's rows will be in the nsIDOMHTMLCollection* named "rows".
-// _trCode should be the code to execute for each tr row.  Note that
-// this may be null at any time.  This macro assumes an nsresult named
-// |rv| is in scope.
-  #define DO_FOR_EACH_BY_ORDER(_code, _trCode)                       \
-    do {                                                             \
-      if (mParent) {                                                 \
-        HTMLTableSectionElement* rowGroup;                           \
-        nsIHTMLCollection* rows;                                     \
-        /* THead */                                                  \
-        for (nsIContent* _node = mParent->nsINode::GetFirstChild();  \
-             _node; _node = _node->GetNextSibling()) {               \
-           if (_node->IsHTMLElement(nsGkAtoms::thead)) {             \
-             rowGroup = static_cast<HTMLTableSectionElement*>(_node);\
-             rows = rowGroup->Rows();                                \
-             do { /* gives scoping */                                \
-               _code                                                 \
-             } while (0);                                            \
-           }                                                         \
-        }                                                            \
-        /* TBodies */                                                \
-        for (nsIContent* _node = mParent->nsINode::GetFirstChild();  \
-             _node; _node = _node->GetNextSibling()) {               \
-          if (_node->IsHTMLElement(nsGkAtoms::tr)) {                 \
-            do {                                                     \
-              _trCode                                                \
-            } while (0);                                             \
-          } else if (_node->IsHTMLElement(nsGkAtoms::tbody)) {       \
-            rowGroup = static_cast<HTMLTableSectionElement*>(_node); \
-            rows = rowGroup->Rows();                                 \
-            do { /* gives scoping */                                 \
-              _code                                                  \
-            } while (0);                                             \
-          }                                                          \
-        }                                                            \
-        /* TFoot */                                                  \
-        for (nsIContent* _node = mParent->nsINode::GetFirstChild();  \
-             _node; _node = _node->GetNextSibling()) {               \
-           if (_node->IsHTMLElement(nsGkAtoms::tfoot)) {             \
-             rowGroup = static_cast<HTMLTableSectionElement*>(_node);\
-             rows = rowGroup->Rows();                                \
-             do { /* gives scoping */                                \
-               _code                                                 \
-             } while (0);                                            \
-           }                                                         \
-        }                                                            \
-      }                                                              \
-    } while (0)
-
-static uint32_t
-CountRowsInRowGroup(nsIDOMHTMLCollection* rows)
-{
-  uint32_t length = 0;
-
-  if (rows) {
-    rows->GetLength(&length);
-  }
-
-  return length;
-}
-
-// we re-count every call.  A better implementation would be to set
-// ourselves up as an observer of contentAppended, contentInserted,
-// and contentDeleted
 NS_IMETHODIMP
 TableRowsCollection::GetLength(uint32_t* aLength)
 {
-  *aLength=0;
+  EnsureInitialized();
-
+  *aLength = mRows.Length();
-  DO_FOR_EACH_BY_ORDER({
-    *aLength += CountRowsInRowGroup(rows);
-  }, {
-    (*aLength)++;
-  });
-
   return NS_OK;
 }
 
-// Returns the item at index aIndex if available. If null is returned,
-// then aCount will be set to the number of rows in this row collection.
-// Otherwise, the value of aCount is undefined.
-static Element*
-GetItemOrCountInRowGroup(nsIDOMHTMLCollection* rows,
-                         uint32_t aIndex, uint32_t* aCount)
-{
-  *aCount = 0;
-
-  if (rows) {
-    rows->GetLength(aCount);
-    if (aIndex < *aCount) {
-      nsIHTMLCollection* list = static_cast<nsIHTMLCollection*>(rows);
-      return list->GetElementAt(aIndex);
-    }
-  }
-
-  return nullptr;
-}
-
 Element*
 TableRowsCollection::GetElementAt(uint32_t aIndex)
 {
-  DO_FOR_EACH_BY_ORDER({
+  EnsureInitialized();
-    uint32_t count;
+  if (aIndex < mRows.Length()) {
-    Element* node = GetItemOrCountInRowGroup(rows, aIndex, &count);
+    return mRows[aIndex]->AsElement();
-    if (node) {
+  }
-      return node;
-    }
-
-    NS_ASSERTION(count <= aIndex, "GetItemOrCountInRowGroup screwed up");
-    aIndex -= count;
-  },{
-    if (aIndex == 0) {
-      return _node->AsElement();
-    }
-    aIndex--;
-  });
-
   return nullptr;
 }
 
@@ -236,23 +256,20 @@ TableRowsCollection::Item(uint32_t aIndex, nsIDOMNode** aReturn)
 Element*
 TableRowsCollection::GetFirstNamedElement(const nsAString& aName, bool& aFound)
 {
+  EnsureInitialized();
   aFound = false;
   nsCOMPtr<nsIAtom> nameAtom = NS_Atomize(aName);
   NS_ENSURE_TRUE(nameAtom, nullptr);
-  DO_FOR_EACH_BY_ORDER({
+
-    Element* item = rows->NamedGetter(aName, aFound);
+  for (auto& node : mRows) {
-    if (aFound) {
+    if (node->AttrValueIs(kNameSpaceID_None, nsGkAtoms::name,
-      return item;
+                          nameAtom, eCaseMatters) ||
-    }
+        node->AttrValueIs(kNameSpaceID_None, nsGkAtoms::id,
-  }, {
+                          nameAtom, eCaseMatters)) {
-    if (_node->AttrValueIs(kNameSpaceID_None, nsGkAtoms::name,
-                           nameAtom, eCaseMatters) ||
-        _node->AttrValueIs(kNameSpaceID_None, nsGkAtoms::id,
-                           nameAtom, eCaseMatters)) {
       aFound = true;
-      return _node->AsElement();
+      return node->AsElement();
     }
-  });
+  }
 
   return nullptr;
 }
@@ -260,20 +277,10 @@ TableRowsCollection::GetFirstNamedElement(const nsAString& aName, bool& aFound)
 void
 TableRowsCollection::GetSupportedNames(nsTArray<nsString>& aNames)
 {
-  DO_FOR_EACH_BY_ORDER({
+  EnsureInitialized();
-    nsTArray<nsString> names;
+  for (auto& node : mRows) {
-    nsCOMPtr<nsIHTMLCollection> coll = do_QueryInterface(rows);
+    if (node->HasID()) {
-    if (coll) {
+      nsIAtom* idAtom = node->GetID();
-      coll->GetSupportedNames(names);
-      for (uint32_t i = 0; i < names.Length(); ++i) {
-        if (!aNames.Contains(names[i])) {
-          aNames.AppendElement(names[i]);
-        }
-      }
-    }
-  }, {
-    if (_node->HasID()) {
-      nsIAtom* idAtom = _node->GetID();
       MOZ_ASSERT(idAtom != nsGkAtoms::_empty,
                  "Empty ids don't get atomized");
       nsDependentAtomString idStr(idAtom);
@@ -282,7 +289,7 @@ TableRowsCollection::GetSupportedNames(nsTArray<nsString>& aNames)
       }
     }
 
-    nsGenericHTMLElement* el = nsGenericHTMLElement::FromContent(_node);
+    nsGenericHTMLElement* el = nsGenericHTMLElement::FromContent(node);
     if (el) {
       const nsAttrValue* val = el->GetParsedAttr(nsGkAtoms::name);
       if (val && val->Type() == nsAttrValue::eAtom) {
@@ -295,7 +302,7 @@ TableRowsCollection::GetSupportedNames(nsTArray<nsString>& aNames)
         }
       }
     }
-  });
+  }
 }
 
 
@@ -317,12 +324,257 @@ TableRowsCollection::NamedItem(const nsAString& aName,
 NS_IMETHODIMP
 TableRowsCollection::ParentDestroyed()
 {
-  // see comment in destructor, do NOT release mParent!
+  CleanUp();
-  mParent = nullptr;
-
   return NS_OK;
 }
 
+bool
+TableRowsCollection::InterestingContainer(nsIContent* aContainer)
+{
+  return mParent && aContainer &&
+    (aContainer == mParent ||
+     (aContainer->GetParent() == mParent &&
+      aContainer->IsAnyOfHTMLElements(nsGkAtoms::thead,
+                                      nsGkAtoms::tbody,
+                                      nsGkAtoms::tfoot)));
+}
+
+bool
+TableRowsCollection::IsAppropriateRow(nsIAtom* aSection, nsIContent* aContent)
+{
+  if (!aContent->IsHTMLElement(nsGkAtoms::tr)) {
+    return false;
+  }
+  // If it's in the root, then we consider it to be in a tbody.
+  nsIContent* parent = aContent->GetParent();
+  if (aSection == nsGkAtoms::tbody && parent == mParent) {
+    return true;
+  }
+  return parent->IsHTMLElement(aSection);
+}
+
+nsIContent*
+TableRowsCollection::PreviousRow(nsIAtom* aSection, nsIContent* aCurrent)
+{
+  // Keep going backwards until we've found a `tr` element. We want to always
+  // run at least once, as we don't want to find ourselves.
+  //
+  // Each spin of the loop we step backwards one element. If we're at the top of
+  // a section, we step out of it into the root, and if we step onto a section
+  // matching `aSection`, we step into it. We keep spinning the loop until
+  // either we reach the first element in mParent, or find a <tr> in an
+  // appropriate section.
+  nsIContent* prev = aCurrent;
+  do {
+    nsIContent* parent = prev->GetParent();
+    prev = prev->GetPreviousSibling();
+
+    // Ascend out of any sections we're currently in, if we've run out of
+    // elements.
+    if (!prev && parent != mParent) {
+      prev = parent->GetPreviousSibling();
+    }
+
+    // Descend into a section if we stepped onto one.
+    if (prev && prev->GetParent() == mParent && prev->IsHTMLElement(aSection)) {
+      prev = prev->GetLastChild();
+    }
+  } while (prev && !IsAppropriateRow(aSection, prev));
+  return prev;
+}
+
+int32_t
+TableRowsCollection::HandleInsert(nsIContent* aContainer,
+                                  nsIContent* aChild,
+                                  int32_t aIndexGuess)
+{
+  if (!nsContentUtils::IsInSameAnonymousTree(mParent, aChild)) {
+    return aIndexGuess; // Nothing inserted, guess hasn't changed.
+  }
+
+  // If we're adding a section to the root, add each of the rows in that section
+  // individually.
+  if (aContainer == mParent &&
+      aChild->IsAnyOfHTMLElements(nsGkAtoms::thead,
+                                  nsGkAtoms::tbody,
+                                  nsGkAtoms::tfoot)) {
+    // If we're entering a tbody, we can persist the index guess we were passed,
+    // as the newly added items are in the same section as us, however, if we're
+    // entering thead or tfoot we will have to re-scan.
+    bool isTBody = aChild->IsHTMLElement(nsGkAtoms::tbody);
+    int32_t indexGuess = isTBody ? aIndexGuess : -1;
+
+    for (nsIContent* inner = aChild->GetFirstChild();
+         inner; inner = inner->GetNextSibling()) {
+      indexGuess = HandleInsert(aChild, inner, indexGuess);
+    }
+
+    return isTBody ? indexGuess : -1;
+  }
+  if (!aChild->IsHTMLElement(nsGkAtoms::tr)) {
+    return aIndexGuess; // Nothing inserted, guess hasn't changed.
+  }
+
+  // We should have only been passed an insertion from an interesting container,
+  // so we can get the container we're inserting to fairly easily.
+  nsIAtom* section = aContainer == mParent
+    ? nsGkAtoms::tbody
+    : aContainer->NodeInfo()->NameAtom();
+
+  // Determine the default index we would to insert after if we don't find any
+  // previous row, and offset our section boundaries based on the section we're
+  // planning to insert into.
+  size_t index = 0;
+  if (section == nsGkAtoms::thead) {
+    mBodyStart++;
+    mFootStart++;
+  } else if (section == nsGkAtoms::tbody) {
+    index = mBodyStart;
+    mFootStart++;
+  } else if (section == nsGkAtoms::tfoot) {
+    index = mFootStart;
+  } else {
+    MOZ_ASSERT(false, "section should be one of thead, tbody, or tfoot");
+  }
+
+  // If we already have an index guess, we can skip scanning for the previous row.
+  if (aIndexGuess >= 0) {
+    index = aIndexGuess;
+  } else {
+    // Find the previous row in the section we're inserting into. If we find it,
+    // we can use it to override our insertion index. We don't need to modify
+    // mBodyStart or mFootStart anymore, as they have already been correctly
+    // updated based only on section.
+    nsIContent* insertAfter = PreviousRow(section, aChild);
+    if (insertAfter) {
+      // NOTE: We want to ensure that appending elements is quick, so we search
+      // from the end rather than from the beginning.
+      index = mRows.LastIndexOf(insertAfter) + 1;
+      MOZ_ASSERT(index != nsTArray<nsCOMPtr<nsIContent>>::NoIndex);
+    }
+  }
+
+#ifdef DEBUG
+  // Assert that we're inserting into the correct section.
+  if (section == nsGkAtoms::thead) {
+    MOZ_ASSERT(index < mBodyStart);
+  } else if (section == nsGkAtoms::tbody) {
+    MOZ_ASSERT(index >= mBodyStart);
+    MOZ_ASSERT(index < mFootStart);
+  } else if (section == nsGkAtoms::tfoot) {
+    MOZ_ASSERT(index >= mFootStart);
+    MOZ_ASSERT(index <= mRows.Length());
+  }
+
+  MOZ_ASSERT(mBodyStart <= mFootStart);
+  MOZ_ASSERT(mFootStart <= mRows.Length() + 1);
+#endif
+
+  mRows.InsertElementAt(index, aChild);
+  return index + 1;
+}
+
+// nsIMutationObserver
+
+void
+TableRowsCollection::ContentAppended(nsIDocument* aDocument,
+                                     nsIContent* aContainer,
+                                     nsIContent* aFirstNewContent,
+                                     int32_t aNewIndexInContainer)
+{
+  if (!nsContentUtils::IsInSameAnonymousTree(mParent, aFirstNewContent) ||
+      !InterestingContainer(aContainer)) {
+    return;
+  }
+
+  // We usually can't guess where we need to start inserting, unless we're
+  // appending into mParent, in which case we can provide the guess that we
+  // should insert at the end of the body, which can help us avoid potentially
+  // expensive work in the common case.
+  int32_t indexGuess = mParent == aContainer ? mFootStart : -1;
+
+  // Insert each of the newly added content one at a time. The indexGuess should
+  // make insertions of a large number of elements cheaper.
+  for (nsIContent* content = aFirstNewContent;
+       content; content = content->GetNextSibling()) {
+    indexGuess = HandleInsert(aContainer, content, indexGuess);
+  }
+}
+
+void
+TableRowsCollection::ContentInserted(nsIDocument* aDocument,
+                                     nsIContent* aContainer,
+                                     nsIContent* aChild,
+                                     int32_t aIndexInContainer)
+{
+  if (!nsContentUtils::IsInSameAnonymousTree(mParent, aChild) ||
+      !InterestingContainer(aContainer)) {
+    return;
+  }
+
+  HandleInsert(aContainer, aChild);
+}
+
+void
+TableRowsCollection::ContentRemoved(nsIDocument* aDocument,
+                                    nsIContent* aContainer,
+                                    nsIContent* aChild,
+                                    int32_t aIndexInContainer,
+                                    nsIContent* aPreviousSibling)
+{
+  if (!nsContentUtils::IsInSameAnonymousTree(mParent, aChild) ||
+      !InterestingContainer(aContainer)) {
+    return;
+  }
+
+  // If the element being removed is a `tr`, we can just remove it from our
+  // list. It shouldn't change the order of anything.
+  if (aChild->IsHTMLElement(nsGkAtoms::tr)) {
+    size_t index = mRows.IndexOf(aChild);
+    if (index != nsTArray<nsCOMPtr<nsIContent>>::NoIndex) {
+      mRows.RemoveElementAt(index);
+      if (mBodyStart > index) {
+        mBodyStart--;
+      }
+      if (mFootStart > index) {
+        mFootStart--;
+      }
+    }
+    return;
+  }
+
+  // If the element being removed is a `thead`, `tbody`, or `tfoot`, we can
+  // remove any `tr`s in our list which have that element as its parent node. In
+  // any other situation, the removal won't affect us, so we can ignore it.
+  if (!aChild->IsAnyOfHTMLElements(nsGkAtoms::thead, nsGkAtoms::tbody, nsGkAtoms::tfoot)) {
+    return;
+  }
+
+  size_t beforeLength = mRows.Length();
+  mRows.RemoveElementsBy([&] (nsIContent* element) {
+    return element->GetParent() == aChild;
+  });
+  size_t removed = beforeLength - mRows.Length();
+  if (aChild->IsHTMLElement(nsGkAtoms::thead)) {
+    // NOTE: Need to move both tbody and tfoot, as we removed from head.
+    mBodyStart -= removed;
+    mFootStart -= removed;
+  } else if (aChild->IsHTMLElement(nsGkAtoms::tbody)) {
+    // NOTE: Need to move tfoot, as we removed from body.
+    mFootStart -= removed;
+  }
+}
+
+void
+TableRowsCollection::NodeWillBeDestroyed(const nsINode* aNode)
+{
+  // Set mInitialized to false so CleanUp doesn't try to remove our mutation
+  // observer, as we're going away. CleanUp() will reset mInitialized to true as
+  // it returns.
+  mInitialized = false;
+  CleanUp();
+}
+
 /* --------------------------- HTMLTableElement ---------------------------- */
 
 HTMLTableElement::HTMLTableElement(already_AddRefed<mozilla::dom::NodeInfo>& aNodeInfo)