Backed out changeset ce929d9e000a (bug 1732674) for causing nsLineIterator::FindLineContaining crashes (bug 1733047) . a=aryx
authorcriss <ccozmuta@mozilla.com>
Sat, 16 Oct 2021 12:46:38 +0300
changeset 596077 e4581e1d6e4b7bc1e647350c0fb67fd2b57e90c9
parent 596076 3e844ac540bd6a0d8595bfdaefbaa5e291b75eb0
child 596086 409f6fdff6518c8519ca56008637d40d4d6f61fe
push id38883
push userccozmuta@mozilla.com
push dateSat, 16 Oct 2021 09:48:19 +0000
treeherdermozilla-central@e4581e1d6e4b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaryx
bugs1732674, 1733047
milestone95.0a1
backs outce929d9e000aff5448f4e9679ac67c55781021fc
first release with
nightly linux32
e4581e1d6e4b / 95.0a1 / 20211016094819 / files
nightly linux64
e4581e1d6e4b / 95.0a1 / 20211016094819 / files
nightly mac
e4581e1d6e4b / 95.0a1 / 20211016094819 / files
nightly win32
e4581e1d6e4b / 95.0a1 / 20211016094819 / files
nightly win64
e4581e1d6e4b / 95.0a1 / 20211016094819 / files
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out changeset ce929d9e000a (bug 1732674) for causing nsLineIterator::FindLineContaining crashes (bug 1733047) . a=aryx
layout/generic/nsIFrame.cpp
layout/generic/nsILineIterator.h
layout/generic/nsLineBox.cpp
layout/generic/nsLineBox.h
layout/tables/nsTableRowGroupFrame.cpp
layout/tables/nsTableRowGroupFrame.h
--- a/layout/generic/nsIFrame.cpp
+++ b/layout/generic/nsIFrame.cpp
@@ -8162,17 +8162,17 @@ nsresult nsIFrame::GetNextPrevLineFromeB
   // block
   if (!aBlockFrame || !aPos) return NS_ERROR_NULL_POINTER;
 
   aPos->mResultFrame = nullptr;
   aPos->mResultContent = nullptr;
   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;
   }
   int32_t searchingLine = aLineStart;
   int32_t countLines = it->GetNumLines();
   if (aOutSideLimit > 0)  // start at end
     searchingLine = countLines;
   else if (aOutSideLimit < 0)  // start at beginning
--- a/layout/generic/nsILineIterator.h
+++ b/layout/generic/nsILineIterator.h
@@ -56,17 +56,18 @@ class nsILineIterator {
      * positioning then its coordinates may be outside the line bounds)
      */
     nsRect mLineBounds;
     /** Whether the line is wrapped at the end */
     bool mIsWrapped;
   };
 
   // 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
    * and return that line index, as long as it's at least as big as
    * aStartLine.  Returns -1 if the frame cannot be found on lines
    * starting with aStartLine.
    */
   virtual int32_t FindLineContaining(nsIFrame* aFrame,
@@ -74,17 +75,17 @@ class nsILineIterator {
 
   // Given a line number and a coordinate, find the frame on the line
   // that is nearest to aPos along the inline axis. (The block-axis coord
   // of aPos is irrelevant.)
   // The aPosIsBeforeFirstFrame and aPosIsAfterLastFrame flags are updated
   // 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.
   //  If not, return the first and last visual frames
   NS_IMETHOD CheckLineOrder(int32_t aLine, bool* aIsReordered,
                             nsIFrame** aFirstVisual,
                             nsIFrame** aLastVisual) = 0;
 };
--- a/layout/generic/nsLineBox.cpp
+++ b/layout/generic/nsLineBox.cpp
@@ -554,51 +554,85 @@ void nsLineBox::SetOverflowAreas(const O
     // the right value.
     mData->mOverflowAreas = aOverflowAreas;
     MaybeFreeData();
   }
 }
 
 //----------------------------------------------------------------------
 
+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();
   structure.mIsWrapped = line->IsLineWrapped();
   return structure;
 }
 
 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;
 }
 
 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;
     return NS_OK;
   }
 
   nsIFrame* leftmostFrame;
   nsIFrame* rightmostFrame;
@@ -612,25 +646,28 @@ nsLineIterator::CheckLineOrder(int32_t a
 
   return NS_OK;
 }
 
 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;
     *aPosIsAfterLastFrame = false;
     return NS_OK;
   }
 
   if (line->ISize() == 0 && line->BSize() == 0) return NS_ERROR_FAILURE;
--- a/layout/generic/nsLineBox.h
+++ b/layout/generic/nsLineBox.h
@@ -1629,87 +1629,60 @@ nsLineList_const_reverse_iterator::opera
 inline nsLineList_const_reverse_iterator&
 nsLineList_const_reverse_iterator::operator=(
     const nsLineList_const_reverse_iterator& aOther) = default;
 
 //----------------------------------------------------------------------
 
 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() = default;
+  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;
-  }
+  virtual int32_t GetNumLines() const override;
+  virtual bool GetDirection() override;
 
-  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) 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;
+  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___ */
--- a/layout/tables/nsTableRowGroupFrame.cpp
+++ b/layout/tables/nsTableRowGroupFrame.cpp
@@ -1691,17 +1691,17 @@ void nsTableRowGroupFrame::SetContinuous
 int32_t nsTableRowGroupFrame::GetNumLines() const { return GetRowCount(); }
 
 bool nsTableRowGroupFrame::GetDirection() {
   return (StyleDirection::Rtl ==
           GetTableFrame()->StyleVisibility()->mDirection);
 }
 
 Result<nsILineIterator::LineInfo, nsresult> nsTableRowGroupFrame::GetLine(
-    int32_t aLineNumber) {
+    int32_t aLineNumber) const {
   if ((aLineNumber < 0) || (aLineNumber >= GetRowCount())) {
     return Err(NS_ERROR_FAILURE);
   }
   LineInfo structure;
   nsTableFrame* table = GetTableFrame();
   nsTableCellMap* cellMap = table->GetCellMap();
   aLineNumber += GetStartRowIndex();
 
@@ -1745,17 +1745,17 @@ nsTableRowGroupFrame::CheckLineOrder(int
   *aLastVisual = nullptr;
   return NS_OK;
 }
 
 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();
 
   WritingMode wm = table->GetWritingMode();
   nsSize containerSize = table->GetSize();
   LogicalPoint pos(wm, aPos, containerSize);
 
   *aFrameFound = nullptr;
--- a/layout/tables/nsTableRowGroupFrame.h
+++ b/layout/tables/nsTableRowGroupFrame.h
@@ -198,17 +198,17 @@ class nsTableRowGroupFrame final : publi
   virtual int32_t GetNumLines() const override;
 
   /** @see nsILineIterator.h GetDirection
    * @return true if the table is rtl
    */
   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
    * @param aStartLine   - minimal index to return
    * @return               row index relative to the row group if this a row
    *                       frame and the index is at least aStartLine.
    *                       -1 if the frame cannot be found.
    */
@@ -223,17 +223,17 @@ class nsTableRowGroupFrame final : publi
    * @param aFrameFound        - pointer to the cellframe
    * @param aPosIsBeforeFirstFrame - the point is before the first originating
    *                               cellframe
    * @param aPosIsAfterLastFrame   - the point is after the last originating
    *                               cellframe
    */
   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
    * @param aLine        - the index of the row relative to the table
    * @param aIsReordered - returns false
    * @param aFirstVisual - if the table is rtl first originating cell frame
    * @param aLastVisual  - if the table is rtl last originating cell frame
    */