Backed out 2 changesets (bug 1732674) for line iterator crashes (bug 1733047) a=backout

Backed out changeset 730555699380 (bug 1732674)
Backed out changeset f529288a6dde (bug 1732674)

layout/base/PresShell.cpp

@@ -3615,19 +3615,17 @@ void PresShell::DoScrollContentIntoView() {
   bool haveRect = false;
   bool useWholeLineHeightForInlines = data->mContentScrollVAxis.mWhenToScroll !=
                                       WhenToScroll::IfNotFullyVisible;
-  {
-    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;
-    // The last line we found a continuation on in |lines|.  We assume that later
-    // continuations cannot come on earlier lines.
-    int32_t curLine = 0;
-    do {
-      AccumulateFrameBounds(container, frame, useWholeLineHeightForInlines,
-                            frameBounds, haveRect, prevBlock, lines, curLine);
-    } while ((frame = frame->GetNextContinuation()));
-  }
+  // Reuse the same line iterator across calls to AccumulateFrameBounds.  We set
+  // it every time we detect a new block (stored in prevBlock).
+  nsIFrame* prevBlock = nullptr;
+  nsAutoLineIterator lines;
+  // The last line we found a continuation on in |lines|.  We assume that later
+  // continuations cannot come on earlier lines.
+  int32_t curLine = 0;
+  do {
+    AccumulateFrameBounds(container, frame, useWholeLineHeightForInlines,
+                          frameBounds, haveRect, prevBlock, lines, curLine);
+  } while ((frame = frame->GetNextContinuation()));
 
   ScrollFrameRectIntoView(container, frameBounds, scrollMargin,
                           data->mContentScrollVAxis, data->mContentScrollHAxis,

layout/generic/nsIFrame.cpp

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

layout/generic/nsILineIterator.h

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

layout/generic/nsLineBox.cpp

@@ -559,13 +559,46 @@ 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 nsLineBox* line = GetLineAt(aLineNumber);
-  if (!line) {
+    int32_t aLineNumber) const {
+  if ((aLineNumber < 0) || (aLineNumber >= mNumLines)) {
     return Err(NS_ERROR_FAILURE);
   }
   LineInfo structure;
+  nsLineBox* line = mLines[aLineNumber];
   structure.mFirstFrameOnLine = line->mFirstChild;
   structure.mNumFramesOnLine = line->GetChildCount();
   structure.mLineBounds = line->GetPhysicalBounds();
@@ -575,13 +608,14 @@ Result<nsILineIterator::LineInfo, nsresult> nsLineIterator::GetLine(
 
 int32_t nsLineIterator::FindLineContaining(nsIFrame* aFrame,
                                            int32_t aStartLine) {
-  const nsLineBox* line = GetLineAt(aStartLine);
-  MOZ_ASSERT(line, "aStartLine out of range");
-  while (line) {
+  MOZ_ASSERT(aStartLine <= mNumLines, "Bogus line numbers");
+  int32_t lineNumber = aStartLine;
+  while (lineNumber != mNumLines) {
+    nsLineBox* line = mLines[lineNumber];
     if (line->Contains(aFrame)) {
-      return mIndex;
+      return lineNumber;
     }
-    line = NextLine();
+    ++lineNumber;
   }
   return -1;
 }
@@ -590,10 +624,10 @@ NS_IMETHODIMP
 nsLineIterator::CheckLineOrder(int32_t aLine, bool* aIsReordered,
                                nsIFrame** aFirstVisual,
                                nsIFrame** aLastVisual) {
-  const nsLineBox* line = GetLineAt(aLine);
-  MOZ_ASSERT(line, "aLine out of range!");
+  NS_ASSERTION(aLine >= 0 && aLine < mNumLines, "aLine out of range!");
+  nsLineBox* line = mLines[aLine];
 
-  if (!line || !line->mFirstChild) {  // empty line
+  if (!line->mFirstChild) {  // empty line
     *aIsReordered = false;
     *aFirstVisual = nullptr;
     *aLastVisual = nullptr;
@@ -617,15 +651,18 @@ NS_IMETHODIMP
 nsLineIterator::FindFrameAt(int32_t aLineNumber, nsPoint aPos,
                             nsIFrame** aFrameFound,
                             bool* aPosIsBeforeFirstFrame,
-                            bool* aPosIsAfterLastFrame) {
+                            bool* aPosIsAfterLastFrame) const {
   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;
+  }
 
-  const nsLineBox* line = GetLineAt(aLineNumber);
+  nsLineBox* line = mLines[aLineNumber];
   if (!line) {
     *aFrameFound = nullptr;
     *aPosIsBeforeFirstFrame = true;

layout/generic/nsLineBox.h

@@ -1634,87 +1634,55 @@ nsLineList_const_reverse_iterator::operator=(
 
 class nsLineIterator final : public nsILineIterator {
  public:
-  nsLineIterator(const nsLineList& aLines, bool aRightToLeft)
-      : mLines(aLines), mRightToLeft(aRightToLeft) {
-    mIter = mLines.begin();
-    if (mIter != mLines.end()) {
-      mIndex = 0;
-    }
-  }
-  ~nsLineIterator() {
-    MOZ_DIAGNOSTIC_ASSERT(!mMutationGuard.Mutated(0));
-  };
+  nsLineIterator(nsLineList& aLines, bool aRightToLeft);
+  ~nsLineIterator();
 
-  void DisposeLineIterator() final { delete this; }
+  virtual void DisposeLineIterator() 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;
+  virtual int32_t GetNumLines() const override;
+  virtual bool GetDirection() override;
 
+  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) final;
+                         bool* aPosIsAfterLastFrame) const override;
 
   NS_IMETHOD CheckLineOrder(int32_t aLine, bool* aIsReordered,
                             nsIFrame** aFirstVisual,
-                            nsIFrame** aLastVisual) final;
+                            nsIFrame** aLastVisual) override;
 
  private:
   nsLineIterator() = delete;
   nsLineIterator(const nsLineIterator& aOther) = delete;
 
-  const nsLineBox* NextLine() {
-    if (mIter == mLines.end()) {
+  nsLineBox* PrevLine() {
+    if (0 == mIndex) {
       return nullptr;
     }
-    ++mIter;
-    ++mIndex;
-    return mIter.get();
+    return mLines[--mIndex];
   }
 
-  // Note that this updates the iterator's current position!
-  const nsLineBox* GetLineAt(int32_t aIndex) {
-    if (aIndex < 0 || mIndex < 0) {
+  nsLineBox* NextLine() {
+    if (mIndex >= mNumLines - 1) {
       return nullptr;
     }
-    if (mIndex == aIndex) {
-      return mIter.get();
-    }
-    while (mIndex > aIndex) {
-      if (!mIndex) {
-        return nullptr;
-      }
-      --mIter;
-      --mIndex;
-    }
-    while (mIndex < aIndex) {
-      if (mIter == mLines.end()) {
-        return nullptr;
-      }
-      ++mIter;
-      ++mIndex;
-    }
-    return mIter.get();
+    return mLines[++mIndex];
   }
 
-  const nsLineList& mLines;
-  nsLineList_const_iterator mIter;
-  int32_t mIndex = -1;
-  mutable int32_t mNumLines = -1;
-  const bool mRightToLeft;
-#ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
-  nsMutationGuard mMutationGuard;
-#endif
+  nsLineBox* LineAt(int32_t aIndex) {
+    if ((aIndex < 0) || (aIndex >= mNumLines)) {
+      return nullptr;
+    }
+    return mLines[aIndex];
+  }
+
+  nsLineBox** mLines;
+  int32_t mIndex;
+  int32_t mNumLines;
+  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) {
+    int32_t aLineNumber) const {
   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) {
+                                  bool* aPosIsAfterLastFrame) const {
   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) override;
+  Result<LineInfo, nsresult> GetLine(int32_t aLineNumber) const 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) override;
+                         bool* aPosIsAfterLastFrame) const 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