Bug 1732674 - Make nsLineIterator a lightweight wrapper for an nsLineList_iterator instead of building a separate array. r=emilio

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

layout/generic/nsIFrame.cpp

@@ -8170,7 +8170,7 @@ nsresult nsIFrame::GetNextPrevLineFromeBlockFrame(nsPresContext* aPresContext,
   aPos->mAttach = aPos->mDirection == eDirNext ? CARET_ASSOCIATE_AFTER
                                                : CARET_ASSOCIATE_BEFORE;
 
-  const nsAutoLineIterator it = aBlockFrame->GetLineIterator();
+  nsAutoLineIterator it = aBlockFrame->GetLineIterator();
   if (!it) {
     return NS_ERROR_FAILURE;
   }

layout/generic/nsILineIterator.h

@@ -61,8 +61,7 @@ class nsILineIterator {
   };
 
   // Return miscellaneous information about a line.
-  virtual mozilla::Result<LineInfo, nsresult> GetLine(
-      int32_t aLineNumber) const = 0;
+  virtual mozilla::Result<LineInfo, nsresult> GetLine(int32_t aLineNumber) = 0;
 
   /**
    * Given a frame that's a child of the block, find which line its on
@@ -80,7 +79,7 @@ class nsILineIterator {
   // appropriately.
   NS_IMETHOD FindFrameAt(int32_t aLineNumber, nsPoint aPos,
                          nsIFrame** aFrameFound, bool* aPosIsBeforeFirstFrame,
-                         bool* aPosIsAfterLastFrame) const = 0;
+                         bool* aPosIsAfterLastFrame) = 0;
 
   // Check whether visual and logical order of frames within a line are
   // identical.

layout/generic/nsLineBox.cpp

@@ -559,46 +559,13 @@ void nsLineBox::SetOverflowAreas(const OverflowAreas& aOverflowAreas) {
 
 //----------------------------------------------------------------------
 
-static nsLineBox* gDummyLines[1];
-
-nsLineIterator::nsLineIterator(nsLineList& aLines, bool aRightToLeft)
-    : mIndex(0), mNumLines(aLines.size()), mRightToLeft(aRightToLeft) {
-  if (0 == mNumLines) {
-    // Use gDummyLines so that we don't need null pointer checks in
-    // the accessor methods
-    mLines = gDummyLines;
-    return;
-  }
-
-  // Make a linear array of the lines
-  mLines = new nsLineBox*[mNumLines];
-  nsLineBox** lp = mLines;
-  for (nsLineList::iterator line = aLines.begin(), line_end = aLines.end();
-       line != line_end; ++line) {
-    *lp++ = line;
-  }
-}
-
-nsLineIterator::~nsLineIterator() {
-  if (mLines != gDummyLines) {
-    delete[] mLines;
-  }
-}
-
-/* virtual */
-void nsLineIterator::DisposeLineIterator() { delete this; }
-
-int32_t nsLineIterator::GetNumLines() const { return mNumLines; }
-
-bool nsLineIterator::GetDirection() { return mRightToLeft; }
-
 Result<nsILineIterator::LineInfo, nsresult> nsLineIterator::GetLine(
-    int32_t aLineNumber) const {
-  if ((aLineNumber < 0) || (aLineNumber >= mNumLines)) {
+    int32_t aLineNumber) {
+  const nsLineBox* line = GetLineAt(aLineNumber);
+  if (!line) {
     return Err(NS_ERROR_FAILURE);
   }
   LineInfo structure;
-  nsLineBox* line = mLines[aLineNumber];
   structure.mFirstFrameOnLine = line->mFirstChild;
   structure.mNumFramesOnLine = line->GetChildCount();
   structure.mLineBounds = line->GetPhysicalBounds();
@@ -608,14 +575,13 @@ Result<nsILineIterator::LineInfo, nsresult> nsLineIterator::GetLine(
 
 int32_t nsLineIterator::FindLineContaining(nsIFrame* aFrame,
                                            int32_t aStartLine) {
-  MOZ_ASSERT(aStartLine <= mNumLines, "Bogus line numbers");
-  int32_t lineNumber = aStartLine;
-  while (lineNumber != mNumLines) {
-    nsLineBox* line = mLines[lineNumber];
+  const nsLineBox* line = GetLineAt(aStartLine);
+  MOZ_ASSERT(line, "aStartLine out of range");
+  while (line) {
     if (line->Contains(aFrame)) {
-      return lineNumber;
+      return mIndex;
     }
-    ++lineNumber;
+    line = GetNextLine();
   }
   return -1;
 }
@@ -624,10 +590,10 @@ NS_IMETHODIMP
 nsLineIterator::CheckLineOrder(int32_t aLine, bool* aIsReordered,
                                nsIFrame** aFirstVisual,
                                nsIFrame** aLastVisual) {
-  NS_ASSERTION(aLine >= 0 && aLine < mNumLines, "aLine out of range!");
-  nsLineBox* line = mLines[aLine];
+  const nsLineBox* line = GetLineAt(aLine);
+  MOZ_ASSERT(line, "aLine out of range!");
 
-  if (!line->mFirstChild) {  // empty line
+  if (!line || !line->mFirstChild) {  // empty line
     *aIsReordered = false;
     *aFirstVisual = nullptr;
     *aLastVisual = nullptr;
@@ -651,18 +617,15 @@ NS_IMETHODIMP
 nsLineIterator::FindFrameAt(int32_t aLineNumber, nsPoint aPos,
                             nsIFrame** aFrameFound,
                             bool* aPosIsBeforeFirstFrame,
-                            bool* aPosIsAfterLastFrame) const {
+                            bool* aPosIsAfterLastFrame) {
   MOZ_ASSERT(aFrameFound && aPosIsBeforeFirstFrame && aPosIsAfterLastFrame,
              "null OUT ptr");
 
   if (!aFrameFound || !aPosIsBeforeFirstFrame || !aPosIsAfterLastFrame) {
     return NS_ERROR_NULL_POINTER;
   }
-  if ((aLineNumber < 0) || (aLineNumber >= mNumLines)) {
-    return NS_ERROR_INVALID_ARG;
-  }
 
-  nsLineBox* line = mLines[aLineNumber];
+  const nsLineBox* line = GetLineAt(aLineNumber);
   if (!line) {
     *aFrameFound = nullptr;
     *aPosIsBeforeFirstFrame = true;

layout/generic/nsLineBox.h

@@ -1634,55 +1634,79 @@ nsLineList_const_reverse_iterator::operator=(
 
 class nsLineIterator final : public nsILineIterator {
  public:
-  nsLineIterator(nsLineList& aLines, bool aRightToLeft);
-  ~nsLineIterator();
+  nsLineIterator(const nsLineList& aLines, bool aRightToLeft)
+      : mLines(aLines), mRightToLeft(aRightToLeft) {
+    mIter = mLines.begin();
+    if (mIter != mLines.end()) {
+      mIndex = 0;
+    }
+  }
+  ~nsLineIterator() = default;
 
-  virtual void DisposeLineIterator() override;
+  void DisposeLineIterator() final { delete this; }
 
-  virtual int32_t GetNumLines() const override;
-  virtual bool GetDirection() override;
+  int32_t GetNumLines() const final {
+    if (mNumLines < 0) {
+      mNumLines = int32_t(mLines.size());  // This is O(N) in number of lines!
+    }
+    return mNumLines;
+  }
+
+  bool GetDirection() final { return mRightToLeft; }
+
+  // Note that this updates the iterator's current position!
+  mozilla::Result<LineInfo, nsresult> GetLine(int32_t aLineNumber) final;
+
+  int32_t FindLineContaining(nsIFrame* aFrame, int32_t aStartLine = 0) final;
 
-  mozilla::Result<LineInfo, nsresult> GetLine(
-      int32_t aLineNumber) const override;
-  virtual int32_t FindLineContaining(nsIFrame* aFrame,
-                                     int32_t aStartLine = 0) override;
   NS_IMETHOD FindFrameAt(int32_t aLineNumber, nsPoint aPos,
                          nsIFrame** aFrameFound, bool* aPosIsBeforeFirstFrame,
-                         bool* aPosIsAfterLastFrame) const override;
+                         bool* aPosIsAfterLastFrame) final;
 
   NS_IMETHOD CheckLineOrder(int32_t aLine, bool* aIsReordered,
                             nsIFrame** aFirstVisual,
-                            nsIFrame** aLastVisual) override;
+                            nsIFrame** aLastVisual) final;
 
  private:
   nsLineIterator() = delete;
   nsLineIterator(const nsLineIterator& aOther) = delete;
 
-  nsLineBox* PrevLine() {
-    if (0 == mIndex) {
-      return nullptr;
-    }
-    return mLines[--mIndex];
+  const nsLineBox* GetNextLine() {
+    MOZ_ASSERT(mIter != mLines.end(), "Already at end!");
+    ++mIndex;
+    ++mIter;
+    return mIter == mLines.end() ? nullptr : mIter.get();
   }
 
-  nsLineBox* NextLine() {
-    if (mIndex >= mNumLines - 1) {
+  // Note that this may update the iterator's current position!
+  const nsLineBox* GetLineAt(int32_t aIndex) {
+    if (aIndex < 0 || (mNumLines >= 0 && aIndex >= mNumLines)) {
       return nullptr;
     }
-    return mLines[++mIndex];
-  }
-
-  nsLineBox* LineAt(int32_t aIndex) {
-    if ((aIndex < 0) || (aIndex >= mNumLines)) {
-      return nullptr;
+    while (mIndex > aIndex) {
+      // This cannot run past the start of the list, because we checked that
+      // aIndex is non-negative.
+      --mIter;
+      --mIndex;
     }
-    return mLines[aIndex];
+    while (mIndex < aIndex) {
+      // Here we have to check for reaching the end, as aIndex could be out of
+      // range (if mNumLines was not initialized, so we couldn't range-check
+      // aIndex on entry).
+      if (mIter == mLines.end()) {
+        return nullptr;
+      }
+      ++mIter;
+      ++mIndex;
+    }
+    return mIter.get();
   }
 
-  nsLineBox** mLines;
-  int32_t mIndex;
-  int32_t mNumLines;
-  bool mRightToLeft;
+  const nsLineList& mLines;
+  nsLineList_const_iterator mIter;
+  int32_t mIndex = -1;
+  mutable int32_t mNumLines = -1;
+  const bool mRightToLeft;
 };
 
 #endif /* nsLineBox_h___ */

layout/tables/nsTableRowGroupFrame.cpp

@@ -1696,7 +1696,7 @@ bool nsTableRowGroupFrame::GetDirection() {
 }
 
 Result<nsILineIterator::LineInfo, nsresult> nsTableRowGroupFrame::GetLine(
-    int32_t aLineNumber) const {
+    int32_t aLineNumber) {
   if ((aLineNumber < 0) || (aLineNumber >= GetRowCount())) {
     return Err(NS_ERROR_FAILURE);
   }
@@ -1750,7 +1750,7 @@ NS_IMETHODIMP
 nsTableRowGroupFrame::FindFrameAt(int32_t aLineNumber, nsPoint aPos,
                                   nsIFrame** aFrameFound,
                                   bool* aPosIsBeforeFirstFrame,
-                                  bool* aPosIsAfterLastFrame) const {
+                                  bool* aPosIsAfterLastFrame) {
   nsTableFrame* table = GetTableFrame();
   nsTableCellMap* cellMap = table->GetCellMap();
 

layout/tables/nsTableRowGroupFrame.h

@@ -203,7 +203,7 @@ class nsTableRowGroupFrame final : public nsContainerFrame,
   virtual bool GetDirection() override;
 
   /** Return structural information about a line. */
-  Result<LineInfo, nsresult> GetLine(int32_t aLineNumber) const override;
+  Result<LineInfo, nsresult> GetLine(int32_t aLineNumber) override;
 
   /** Given a frame that's a child of the rowgroup, find which line its on.
    * @param aFrame       - frame, should be a row
@@ -228,7 +228,7 @@ class nsTableRowGroupFrame final : public nsContainerFrame,
    */
   NS_IMETHOD FindFrameAt(int32_t aLineNumber, nsPoint aPos,
                          nsIFrame** aFrameFound, bool* aPosIsBeforeFirstFrame,
-                         bool* aPosIsAfterLastFrame) const override;
+                         bool* aPosIsAfterLastFrame) override;
 
   /** 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