Bug 1771609 - patch 3 - Remove nsAutoLineIterator, as it no longer serves any useful purpose since the iterator is owned by the target frame. r=emilio

Differential Revision: https://phabricator.services.mozilla.com/D147579

accessible/base/TextLeafRange.cpp

@@ -206,7 +206,8 @@ static bool IsLocalAccAtLineStart(LocalAccessible* aAcc) {
     // same line, so we're at the start.
     return !found || prevIt.GetLine() != thisIt.GetLine();
   }
-  nsAutoLineIterator it = prevBlock->GetLineIterator();
+  AutoAssertNoDomMutations guard;
+  nsILineIterator* it = prevBlock->GetLineIterator();
   MOZ_ASSERT(it, "GetLineIterator impl in line-container blocks is infallible");
   int32_t prevLineNum = it->FindLineContaining(prevLineFrame);
   if (prevLineNum < 0) {

accessible/generic/HyperTextAccessible.cpp

@@ -1718,8 +1718,10 @@ int32_t HyperTextAccessible::CaretLineNumber() {
       caretContent, caretOffset, hint, &returnOffsetUnused);
   NS_ENSURE_TRUE(caretFrame, -1);
 
+  AutoAssertNoDomMutations guard;  // The nsILineIterators below will break if
+                                   // the DOM is modified while they're in use!
   int32_t lineNumber = 1;
-  nsAutoLineIterator lineIterForCaret;
+  nsILineIterator* lineIterForCaret = nullptr;
   nsIContent* hyperTextContent = IsContent() ? mContent.get() : nullptr;
   while (caretFrame) {
     if (hyperTextContent == caretFrame->GetContent()) {
@@ -1732,7 +1734,7 @@ int32_t HyperTextAccessible::CaretLineNumber() {
     // Add lines for the sibling frames before the caret
     nsIFrame* sibling = parentFrame->PrincipalChildList().FirstChild();
     while (sibling && sibling != caretFrame) {
-      nsAutoLineIterator lineIterForSibling = sibling->GetLineIterator();
+      nsILineIterator* lineIterForSibling = sibling->GetLineIterator();
       if (lineIterForSibling) {
         // For the frames before that grab all the lines
         int32_t addLines = lineIterForSibling->GetNumLines();

layout/base/PresShell.cpp

@@ -3381,8 +3381,7 @@ static void AccumulateFrameBounds(nsIFrame* aContainerFrame, nsIFrame* aFrame,
                                   bool aUseWholeLineHeightForInlines,
                                   nsRect& aRect, bool& aHaveRect,
                                   nsIFrame*& aPrevBlock,
-                                  nsAutoLineIterator& aLines,
+                                  nsILineIterator*& aLines, int32_t& aCurLine) {
-                                  int32_t& aCurLine) {
   nsIFrame* frame = aFrame;
   nsRect frameBounds = nsRect(nsPoint(0, 0), aFrame->GetSize());
 
@@ -3712,10 +3711,11 @@ void PresShell::DoScrollContentIntoView() {
   bool useWholeLineHeightForInlines = data->mContentScrollVAxis.mWhenToScroll !=
                                       WhenToScroll::IfNotFullyVisible;
   {
+    AutoAssertNoDomMutations guard;  // Ensure use of nsILineIterators is safe.
     nsIFrame* prevBlock = nullptr;
     // Reuse the same line iterator across calls to AccumulateFrameBounds.
     // We set it every time we detect a new block (stored in prevBlock).
-    nsAutoLineIterator lines;
+    nsILineIterator* lines = nullptr;
     // The last line we found a continuation on in |lines|.  We assume that
     // later continuations cannot come on earlier lines.
     int32_t curLine = 0;

layout/generic/nsFrameList.cpp

@@ -309,7 +309,8 @@ nsIFrame* nsFrameList::GetPrevVisualFor(nsIFrame* aFrame) const {
   mozilla::intl::BidiDirection paraDir =
       nsBidiPresUtils::ParagraphDirection(mFirstChild);
 
-  nsAutoLineIterator iter = parent->GetLineIterator();
+  AutoAssertNoDomMutations guard;
+  nsILineIterator* iter = parent->GetLineIterator();
   if (!iter) {
     // Parent is not a block Frame
     if (parent->IsLineFrame()) {
@@ -380,7 +381,8 @@ nsIFrame* nsFrameList::GetNextVisualFor(nsIFrame* aFrame) const {
   mozilla::intl::BidiDirection paraDir =
       nsBidiPresUtils::ParagraphDirection(mFirstChild);
 
-  nsAutoLineIterator iter = parent->GetLineIterator();
+  AutoAssertNoDomMutations guard;
+  nsILineIterator* iter = parent->GetLineIterator();
   if (!iter) {
     // Parent is not a block Frame
     if (parent->IsLineFrame()) {

layout/generic/nsIFrame.cpp

@@ -8322,7 +8322,8 @@ nsresult nsIFrame::GetNextPrevLineFromeBlockFrame(nsPresContext* aPresContext,
   aPos->mAttach = aPos->mDirection == eDirNext ? CARET_ASSOCIATE_AFTER
                                                : CARET_ASSOCIATE_BEFORE;
 
-  nsAutoLineIterator it = aBlockFrame->GetLineIterator();
+  AutoAssertNoDomMutations guard;
+  nsILineIterator* it = aBlockFrame->GetLineIterator();
   if (!it) {
     return NS_ERROR_FAILURE;
   }
@@ -8929,6 +8930,7 @@ nsresult nsIFrame::PeekOffsetForLine(nsPeekOffsetStruct* aPos) {
 
   // outer loop
   // moving to a next block when no more blocks are available in a subtree
+  AutoAssertNoDomMutations guard;
   while (NS_FAILED(result)) {
     auto [newBlock, lineFrame] =
         blockFrame->GetContainingBlockForLine(aPos->mScrollViewStop);
@@ -8936,7 +8938,7 @@ nsresult nsIFrame::PeekOffsetForLine(nsPeekOffsetStruct* aPos) {
       return NS_ERROR_FAILURE;
     }
     blockFrame = newBlock;
-    nsAutoLineIterator iter = blockFrame->GetLineIterator();
+    nsILineIterator* iter = blockFrame->GetLineIterator();
     int32_t thisLine = iter->FindLineContaining(lineFrame);
     if (NS_WARN_IF(thisLine < 0)) {
       return NS_ERROR_FAILURE;
@@ -9034,7 +9036,8 @@ nsresult nsIFrame::PeekOffsetForLineEdge(nsPeekOffsetStruct* aPos) {
   if (!blockFrame) {
     return NS_ERROR_FAILURE;
   }
-  nsAutoLineIterator it = blockFrame->GetLineIterator();
+  AutoAssertNoDomMutations guard;
+  nsILineIterator* it = blockFrame->GetLineIterator();
   int32_t thisLine = it->FindLineContaining(lineFrame);
   if (thisLine < 0) {
     return NS_ERROR_FAILURE;
@@ -9338,6 +9341,7 @@ nsIFrame::SelectablePeekReport nsIFrame::GetFrameFromDirection(
   // Find the prev/next selectable frame
   bool selectable = false;
   nsIFrame* traversedFrame = this;
+  AutoAssertNoDomMutations guard;
   while (!selectable) {
     auto [blockFrame, lineFrame] =
         traversedFrame->GetContainingBlockForLine(aScrollViewStop);
@@ -9345,7 +9349,7 @@ nsIFrame::SelectablePeekReport nsIFrame::GetFrameFromDirection(
       return result;
     }
 
-    nsAutoLineIterator it = blockFrame->GetLineIterator();
+    nsILineIterator* it = blockFrame->GetLineIterator();
     int32_t thisLine = it->FindLineContaining(lineFrame);
     if (thisLine < 0) {
       return result;
@@ -10543,7 +10547,8 @@ nsIFrame::RefreshSizeCache(nsBoxLayoutState& aState) {
     metrics->mBlockMinSize.height = 0;
     // ok we need the max ascent of the items on the line. So to do this
     // ask the block for its line iterator. Get the max ascent.
-    nsAutoLineIterator lines = GetLineIterator();
+    AutoAssertNoDomMutations guard;
+    nsILineIterator* lines = GetLineIterator();
     if (lines) {
       metrics->mBlockMinSize.height = 0;
       int32_t lineCount = lines->GetNumLines();

layout/generic/nsILineIterator.h

@@ -7,6 +7,7 @@
 #define nsILineIterator_h___
 
 #include "nscore.h"
+#include "nsINode.h"
 #include "nsRect.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/Result.h"
@@ -20,16 +21,16 @@ class nsIFrame;
  * the bottom line.
  *
  * Obtain this interface from frames via nsIFrame::GetLineIterator.
- * When you are finished using the iterator, call DisposeLineIterator()
+ * This iterator belongs to the frame from which it was obtained, and should
- * to destroy the iterator if appropriate.
+ * not be deleted by the caller.
+ * Note that any modification of the frame will invalidate the iterator!
+ * Users must get a new iterator any time the target may have been touched.
  */
 class nsILineIterator {
  protected:
   ~nsILineIterator() = default;
 
  public:
-  virtual void DisposeLineIterator() = 0;
-
   /**
    * The number of lines in the block
    */
@@ -89,31 +90,17 @@ class nsILineIterator {
                             nsIFrame** aLastVisual) = 0;
 };
 
-class nsAutoLineIterator {
+/**
+ * Helper intended to be used in a scope where we're using an nsILineIterator
+ * and want to verify that no DOM mutations (which would invalidate the
+ * iterator) occur while we're using it.
+ */
+class MOZ_STACK_CLASS AutoAssertNoDomMutations final {
+#ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
+  nsMutationGuard mGuard;
+#endif
  public:
-  nsAutoLineIterator() : mRawPtr(nullptr) {}
+  ~AutoAssertNoDomMutations() { MOZ_DIAGNOSTIC_ASSERT(!mGuard.Mutated(0)); }
-  MOZ_IMPLICIT nsAutoLineIterator(nsILineIterator* i) : mRawPtr(i) {}
-
-  ~nsAutoLineIterator() {
-    if (mRawPtr) mRawPtr->DisposeLineIterator();
-  }
-
-  operator const nsILineIterator*() const { return mRawPtr; }
-  operator nsILineIterator*() { return mRawPtr; }
-  const nsILineIterator* operator->() const { return mRawPtr; }
-  nsILineIterator* operator->() { return mRawPtr; }
-
-  nsILineIterator* operator=(nsILineIterator* i) {
-    if (i == mRawPtr) return i;
-
-    if (mRawPtr) mRawPtr->DisposeLineIterator();
-
-    mRawPtr = i;
-    return i;
-  }
-
- private:
-  nsILineIterator* mRawPtr;
 };
 
 #endif /* nsILineIterator_h___ */

layout/generic/nsLineBox.h

@@ -1642,8 +1642,6 @@ class nsLineIterator final : public nsILineIterator {
     }
   }
 
-  void DisposeLineIterator() final {}
-
   int32_t GetNumLines() const final {
     if (mNumLines < 0) {
       mNumLines = int32_t(mLines.size());  // This is O(N) in number of lines!

layout/tables/nsTableRowGroupFrame.h

@@ -183,8 +183,6 @@ class nsTableRowGroupFrame final : public nsContainerFrame,
 
   // nsILineIterator methods
  public:
-  virtual void DisposeLineIterator() override {}
-
   // The table row is the equivalent to a line in block layout.
   // The nsILineIterator assumes that a line resides in a block, this role is
   // fullfilled by the row group. Rows in table are counted relative to the
@@ -195,15 +193,15 @@ class nsTableRowGroupFrame final : public nsContainerFrame,
   /** Get the number of rows in a row group
    * @return the number of lines in a row group
    */
-  virtual int32_t GetNumLines() const override;
+  int32_t GetNumLines() const final;
 
   /** @see nsILineIterator.h GetDirection
    * @return true if the table is rtl
    */
-  virtual bool GetDirection() override;
+  bool GetDirection() final;
 
   /** Return structural information about a line. */
-  Result<LineInfo, nsresult> GetLine(int32_t aLineNumber) override;
+  Result<LineInfo, nsresult> GetLine(int32_t aLineNumber) final;
 
   /** Given a frame that's a child of the rowgroup, find which line its on.
    * @param aFrame       - frame, should be a row
@@ -212,8 +210,7 @@ class nsTableRowGroupFrame final : public nsContainerFrame,
    *                       frame and the index is at least aStartLine.
    *                       -1 if the frame cannot be found.
    */
-  virtual int32_t FindLineContaining(nsIFrame* aFrame,
+  int32_t FindLineContaining(nsIFrame* aFrame, int32_t aStartLine = 0) final;
-                                     int32_t aStartLine = 0) override;
 
   /** Find the orginating cell frame on a row that is the nearest to the
    * inline-dir coordinate of aPos.
@@ -228,7 +225,7 @@ class nsTableRowGroupFrame final : public nsContainerFrame,
    */
   NS_IMETHOD FindFrameAt(int32_t aLineNumber, nsPoint aPos,
                          nsIFrame** aFrameFound, bool* aPosIsBeforeFirstFrame,
-                         bool* aPosIsAfterLastFrame) override;
+                         bool* aPosIsAfterLastFrame) final;
 
   /** Check whether visual and logical order of cell frames within a line are
    * identical. As the layout will reorder them this is always the case
@@ -240,7 +237,7 @@ class nsTableRowGroupFrame final : public nsContainerFrame,
 
   NS_IMETHOD CheckLineOrder(int32_t aLine, bool* aIsReordered,
                             nsIFrame** aFirstVisual,
-                            nsIFrame** aLastVisual) override;
+                            nsIFrame** aLastVisual) final;
 
   // row cursor methods to speed up searching for the row(s)
   // containing a point. The basic idea is that we set the cursor