Bug 1473314 - Make nsRange::GetUsedFontFaces accumulate font faces in the order they are encountered in the document. r=xidorn
authorJonathan Kew <jkew@mozilla.com>
Thu, 27 Sep 2018 11:33:25 +0100
changeset 486826 a05984b5e160b61cc9e7ab8f3a6d28675c019c45
parent 486825 541b5d1ac630bc6043223cc95b44b7fbdbc2e86e
child 486827 4e19fc30b644e1bab2613e4e8db50335362035ad
push id246
push userfmarier@mozilla.com
push dateSat, 13 Oct 2018 00:15:40 +0000
reviewersxidorn
bugs1473314
milestone64.0a1
Bug 1473314 - Make nsRange::GetUsedFontFaces accumulate font faces in the order they are encountered in the document. r=xidorn
dom/base/nsRange.cpp
layout/base/nsLayoutUtils.cpp
layout/base/nsLayoutUtils.h
--- a/dom/base/nsRange.cpp
+++ b/dom/base/nsRange.cpp
@@ -3167,36 +3167,35 @@ nsRange::GetClientRectsAndTexts(
 
   nsLayoutUtils::RectListBuilder builder(aResult.mRectList);
 
   CollectClientRectsAndText(&builder, &aResult.mTextList, this,
     mStart.Container(), mStart.Offset(), mEnd.Container(), mEnd.Offset(), true, true);
 }
 
 nsresult
-nsRange::GetUsedFontFaces(nsTArray<nsAutoPtr<InspectorFontFace>>& aResult,
+nsRange::GetUsedFontFaces(nsLayoutUtils::UsedFontFaceList& aResult,
                           uint32_t aMaxRanges, bool aSkipCollapsedWhitespace)
 {
   NS_ENSURE_TRUE(mStart.Container(), NS_ERROR_UNEXPECTED);
 
   nsCOMPtr<nsINode> startContainer = mStart.Container();
   nsCOMPtr<nsINode> endContainer = mEnd.Container();
 
   // Flush out layout so our frames are up to date.
   nsIDocument* doc = mStart.Container()->OwnerDoc();
   NS_ENSURE_TRUE(doc, NS_ERROR_UNEXPECTED);
   doc->FlushPendingNotifications(FlushType::Frames);
 
   // Recheck whether we're still in the document
   NS_ENSURE_TRUE(mStart.Container()->IsInComposedDoc(), NS_ERROR_UNEXPECTED);
 
   // A table to map gfxFontEntry objects to InspectorFontFace objects.
-  // (We hold on to the InspectorFontFaces strongly due to the nsAutoPtrs
-  // in the nsClassHashtable, until we move them out into aResult at the end
-  // of the function.)
+  // This table does NOT own the InspectorFontFace objects, it only holds
+  // raw pointers to them. They are owned by the aResult array.
   nsLayoutUtils::UsedFontFaceTable fontFaces;
 
   RangeSubtreeIterator iter;
   nsresult rv = iter.Init(this);
   NS_ENSURE_SUCCESS(rv, rv);
 
   while (!iter.IsDone()) {
     // only collect anything if the range is not collapsed
@@ -3212,38 +3211,35 @@ nsRange::GetUsedFontFaces(nsTArray<nsAut
       continue;
     }
 
     if (content->IsText()) {
        if (node == startContainer) {
          int32_t offset = startContainer == endContainer ?
            mEnd.Offset() : content->GetText()->GetLength();
          nsLayoutUtils::GetFontFacesForText(frame, mStart.Offset(), offset,
-                                            true, fontFaces, aMaxRanges,
+                                            true, aResult, fontFaces,
+                                            aMaxRanges,
                                             aSkipCollapsedWhitespace);
          continue;
        }
        if (node == endContainer) {
          nsLayoutUtils::GetFontFacesForText(frame, 0, mEnd.Offset(),
-                                            true, fontFaces, aMaxRanges,
+                                            true, aResult, fontFaces,
+                                            aMaxRanges,
                                             aSkipCollapsedWhitespace);
          continue;
        }
     }
 
-    nsLayoutUtils::GetFontFacesForFrames(frame, fontFaces, aMaxRanges,
+    nsLayoutUtils::GetFontFacesForFrames(frame, aResult, fontFaces,
+                                         aMaxRanges,
                                          aSkipCollapsedWhitespace);
   }
 
-  // Take ownership of the InspectorFontFaces in the table and move them into
-  // the aResult outparam.
-  for (auto iter = fontFaces.Iter(); !iter.Done(); iter.Next()) {
-    aResult.AppendElement(std::move(iter.Data()));
-  }
-
   return NS_OK;
 }
 
 nsINode*
 nsRange::GetRegisteredCommonAncestor()
 {
   MOZ_ASSERT(IsInSelection(),
              "GetRegisteredCommonAncestor only valid for range in selection");
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@@ -7852,66 +7852,69 @@ nsLayoutUtils::AssertTreeOnlyEmptyNextIn
       nsLayoutUtils::AssertTreeOnlyEmptyNextInFlows(childFrames.get());
     }
   }
 }
 #endif
 
 static void
 GetFontFacesForFramesInner(nsIFrame* aFrame,
+                           nsLayoutUtils::UsedFontFaceList& aResult,
                            nsLayoutUtils::UsedFontFaceTable& aFontFaces,
                            uint32_t aMaxRanges,
                            bool aSkipCollapsedWhitespace)
 {
   MOZ_ASSERT(aFrame, "NULL frame pointer");
 
   if (aFrame->IsTextFrame()) {
     if (!aFrame->GetPrevContinuation()) {
       nsLayoutUtils::GetFontFacesForText(aFrame, 0, INT32_MAX, true,
-                                         aFontFaces, aMaxRanges,
+                                         aResult, aFontFaces, aMaxRanges,
                                          aSkipCollapsedWhitespace);
     }
     return;
   }
 
   nsIFrame::ChildListID childLists[] = { nsIFrame::kPrincipalList,
                                          nsIFrame::kPopupList };
   for (size_t i = 0; i < ArrayLength(childLists); ++i) {
     nsFrameList children(aFrame->GetChildList(childLists[i]));
     for (nsFrameList::Enumerator e(children); !e.AtEnd(); e.Next()) {
       nsIFrame* child = e.get();
       child = nsPlaceholderFrame::GetRealFrameFor(child);
-      GetFontFacesForFramesInner(child, aFontFaces, aMaxRanges,
+      GetFontFacesForFramesInner(child, aResult, aFontFaces, aMaxRanges,
                                  aSkipCollapsedWhitespace);
     }
   }
 }
 
 /* static */ nsresult
 nsLayoutUtils::GetFontFacesForFrames(nsIFrame* aFrame,
+                                     UsedFontFaceList& aResult,
                                      UsedFontFaceTable& aFontFaces,
                                      uint32_t aMaxRanges,
                                      bool aSkipCollapsedWhitespace)
 {
   MOZ_ASSERT(aFrame, "NULL frame pointer");
 
   while (aFrame) {
-    GetFontFacesForFramesInner(aFrame, aFontFaces, aMaxRanges,
+    GetFontFacesForFramesInner(aFrame, aResult, aFontFaces, aMaxRanges,
                                aSkipCollapsedWhitespace);
     aFrame = GetNextContinuationOrIBSplitSibling(aFrame);
   }
 
   return NS_OK;
 }
 
 static void
 AddFontsFromTextRun(gfxTextRun* aTextRun,
                     nsTextFrame* aFrame,
                     gfxSkipCharsIterator& aSkipIter,
                     const gfxTextRun::Range& aRange,
+                    nsLayoutUtils::UsedFontFaceList& aResult,
                     nsLayoutUtils::UsedFontFaceTable& aFontFaces,
                     uint32_t aMaxRanges)
 {
   gfxTextRun::GlyphRunIterator glyphRuns(aTextRun, aRange);
   nsIContent* content = aFrame->GetContent();
   int32_t contentLimit = aFrame->GetContentOffset() +
                          aFrame->GetInFlowContentLength();
   while (glyphRuns.NextRun()) {
@@ -7921,16 +7924,17 @@ AddFontsFromTextRun(gfxTextRun* aTextRun
     InspectorFontFace* fontFace = aFontFaces.Get(fe);
     if (fontFace) {
       fontFace->AddMatchType(glyphRuns.GetGlyphRun()->mMatchType);
     } else {
       // A new font entry we haven't seen before
       fontFace = new InspectorFontFace(fe, aTextRun->GetFontGroup(),
                                        glyphRuns.GetGlyphRun()->mMatchType);
       aFontFaces.Put(fe, fontFace);
+      aResult.AppendElement(fontFace);
     }
 
     // Add this glyph run to the fontFace's list of ranges, unless we have
     // already collected as many as wanted.
     if (fontFace->RangeCount() < aMaxRanges) {
       int32_t start =
         aSkipIter.ConvertSkippedToOriginal(glyphRuns.GetStringStart());
       int32_t end =
@@ -7960,16 +7964,17 @@ AddFontsFromTextRun(gfxTextRun* aTextRun
   }
 }
 
 /* static */ void
 nsLayoutUtils::GetFontFacesForText(nsIFrame* aFrame,
                                    int32_t aStartOffset,
                                    int32_t aEndOffset,
                                    bool aFollowContinuations,
+                                   UsedFontFaceList& aResult,
                                    UsedFontFaceTable& aFontFaces,
                                    uint32_t aMaxRanges,
                                    bool aSkipCollapsedWhitespace)
 {
   MOZ_ASSERT(aFrame, "NULL frame pointer");
 
   if (!aFrame->IsTextFrame()) {
     return;
@@ -8007,17 +8012,18 @@ nsLayoutUtils::GetFontFacesForText(nsIFr
       }
     }
 
     if (!aSkipCollapsedWhitespace ||
         (curr->HasAnyNoncollapsedCharacters() &&
          curr->HasNonSuppressedText())) {
       gfxTextRun::Range range(iter.ConvertOriginalToSkipped(fstart),
                               iter.ConvertOriginalToSkipped(fend));
-      AddFontsFromTextRun(textRun, curr, iter, range, aFontFaces, aMaxRanges);
+      AddFontsFromTextRun(textRun, curr, iter, range, aResult, aFontFaces,
+                          aMaxRanges);
     }
 
     curr = next;
   } while (aFollowContinuations && curr);
 }
 
 /* static */
 size_t
--- a/layout/base/nsLayoutUtils.h
+++ b/layout/base/nsLayoutUtils.h
@@ -2284,41 +2284,51 @@ public:
                                       nscolor aBackstop);
 
   /**
    * Returns true if the passed in prescontext needs the dark grey background
    * that goes behind the page of a print preview presentation.
    */
   static bool NeedsPrintPreviewBackground(nsPresContext* aPresContext);
 
-  typedef nsClassHashtable<nsPtrHashKey<gfxFontEntry>,
-                           mozilla::dom::InspectorFontFace> UsedFontFaceTable;
+  /**
+   * Types used by the helpers for InspectorUtils.getUsedFontFaces.
+   * The API returns an array (UsedFontFaceList) that owns the
+   * InspectorFontFace instances, but during range traversal we also
+   * want to maintain a mapping from gfxFontEntry to InspectorFontFace
+   * records, so use a temporary hashtable for that.
+   */
+  typedef nsTArray<nsAutoPtr<mozilla::dom::InspectorFontFace>> UsedFontFaceList;
+  typedef nsDataHashtable<nsPtrHashKey<gfxFontEntry>,
+                          mozilla::dom::InspectorFontFace*> UsedFontFaceTable;
 
   /**
    * Adds all font faces used in the frame tree starting from aFrame
    * to the list aFontFaceList.
    * aMaxRanges: maximum number of text ranges to record for each face.
    */
   static nsresult GetFontFacesForFrames(nsIFrame* aFrame,
-                                        UsedFontFaceTable& aResult,
+                                        UsedFontFaceList& aResult,
+                                        UsedFontFaceTable& aFontFaces,
                                         uint32_t aMaxRanges,
                                         bool aSkipCollapsedWhitespace);
 
   /**
    * Adds all font faces used within the specified range of text in aFrame,
    * and optionally its continuations, to the list in aFontFaceList.
    * Pass 0 and INT32_MAX for aStartOffset and aEndOffset to specify the
    * entire text is to be considered.
    * aMaxRanges: maximum number of text ranges to record for each face.
    */
   static void GetFontFacesForText(nsIFrame* aFrame,
                                   int32_t aStartOffset,
                                   int32_t aEndOffset,
                                   bool aFollowContinuations,
-                                  UsedFontFaceTable& aResult,
+                                  UsedFontFaceList& aResult,
+                                  UsedFontFaceTable& aFontFaces,
                                   uint32_t aMaxRanges,
                                   bool aSkipCollapsedWhitespace);
 
   /**
    * Walks the frame tree starting at aFrame looking for textRuns.
    * If |clear| is true, just clears the TEXT_RUN_MEMORY_ACCOUNTED flag
    * on each textRun found (and |aMallocSizeOf| is not used).
    * If |clear| is false, adds the storage used for each textRun to the