Bug 1340852 - (wip) avoid passing raw pointers for gfxTextRun::PropertyProvider::Spacing. draft
authorJeremy Chen <jeremychen@mozilla.com>
Sun, 19 Feb 2017 15:24:51 +0800
changeset 486699 7f7c46ee1890c9435391ebe66a9d4c6cfa9b938f
parent 486570 698de2db1b16a5ef3c6a39f0f72885e69aee4022
child 546295 81b6162f9d8783ffe32680b7ae4e812a824beec8
push id46036
push userjichen@mozilla.com
push dateSun, 19 Feb 2017 07:29:29 +0000
bugs1340852
milestone54.0a1
Bug 1340852 - (wip) avoid passing raw pointers for gfxTextRun::PropertyProvider::Spacing. MozReview-Commit-ID: KCWGDOeX7Wf
gfx/src/nsFontMetrics.cpp
gfx/thebes/gfxTextRun.cpp
gfx/thebes/gfxTextRun.h
layout/generic/nsTextFrame.cpp
--- a/gfx/src/nsFontMetrics.cpp
+++ b/gfx/src/nsFontMetrics.cpp
@@ -98,17 +98,17 @@ public:
     virtual already_AddRefed<mozilla::gfx::DrawTarget> GetDrawTarget() {
         NS_ERROR("This shouldn't be called because we never enable hyphens");
         return nullptr;
     }
     virtual uint32_t GetAppUnitsPerDevUnit() {
         NS_ERROR("This shouldn't be called because we never enable hyphens");
         return 60;
     }
-    virtual void GetSpacing(gfxTextRun::Range aRange, Spacing* aSpacing) {
+    virtual void GetSpacing(gfxTextRun::Range aRange, nsTArray<Spacing>& aSpacing) {
         NS_ERROR("This shouldn't be called because we never enable spacing");
     }
 };
 
 } // namespace
 
 nsFontMetrics::nsFontMetrics(const nsFont& aFont, const Params& aParams,
                              nsDeviceContext *aContext)
--- a/gfx/thebes/gfxTextRun.cpp
+++ b/gfx/thebes/gfxTextRun.cpp
@@ -295,26 +295,27 @@ gfxTextRun::ComputeLigatureData(Range aP
         // this part.
         result.mClipBeforePart = partClusterIndex > 0;
         // We need to clip after the part if any cluster is drawn after
         // this part.
         result.mClipAfterPart = partClusterIndex + partClusterCount < totalClusterCount;
     }
 
     if (aProvider && (mFlags & gfxTextRunFactory::TEXT_ENABLE_SPACING)) {
-        gfxFont::Spacing spacing;
+        nsTArray<gfxFont::Spacing> spacing;
+        spacing.AppendElements(1);
         if (aPartRange.start == result.mRange.start) {
             aProvider->GetSpacing(
-                Range(aPartRange.start, aPartRange.start + 1), &spacing);
-            result.mPartWidth += spacing.mBefore;
+                Range(aPartRange.start, aPartRange.start + 1), spacing);
+            result.mPartWidth += spacing[0].mBefore;
         }
         if (aPartRange.end == result.mRange.end) {
             aProvider->GetSpacing(
-                Range(aPartRange.end - 1, aPartRange.end), &spacing);
-            result.mPartWidth += spacing.mAfter;
+                Range(aPartRange.end - 1, aPartRange.end), spacing);
+            result.mPartWidth += spacing[0].mAfter;
         }
     }
 
     return result;
 }
 
 gfxFloat
 gfxTextRun::ComputePartialLigatureWidth(Range aPartRange,
@@ -334,17 +335,17 @@ gfxTextRun::GetAdvanceForGlyphs(Range aR
         advance += GetAdvanceForGlyph(i);
     }
     return advance;
 }
 
 static void
 GetAdjustedSpacing(const gfxTextRun *aTextRun, gfxTextRun::Range aRange,
                    gfxTextRun::PropertyProvider *aProvider,
-                   gfxTextRun::PropertyProvider::Spacing *aSpacing)
+                   nsTArray<gfxTextRun::PropertyProvider::Spacing>& aSpacing)
 {
     if (aRange.start >= aRange.end)
         return;
 
     aProvider->GetSpacing(aRange, aSpacing);
 
 #ifdef DEBUG
     // Check to see if we have spacing inside ligatures
@@ -363,28 +364,36 @@ GetAdjustedSpacing(const gfxTextRun *aTe
         }
     }
 #endif
 }
 
 bool
 gfxTextRun::GetAdjustedSpacingArray(Range aRange, PropertyProvider *aProvider,
                                     Range aSpacingRange,
-                                    nsTArray<PropertyProvider::Spacing>*
+                                    nsTArray<PropertyProvider::Spacing>&
                                         aSpacing) const
 {
-    if (!aProvider || !(mFlags & gfxTextRunFactory::TEXT_ENABLE_SPACING))
+    if (!aProvider || !(mFlags & gfxTextRunFactory::TEXT_ENABLE_SPACING)) {
         return false;
-    if (!aSpacing->AppendElements(aRange.Length()))
+    }
+    if (!aSpacing.AppendElements(aRange.Length())) {
         return false;
+    }
     auto spacingOffset = aSpacingRange.start - aRange.start;
-    memset(aSpacing->Elements(), 0, sizeof(gfxFont::Spacing) * spacingOffset);
-    GetAdjustedSpacing(this, aSpacingRange, aProvider,
-                       aSpacing->Elements() + spacingOffset);
-    memset(aSpacing->Elements() + aSpacingRange.end - aRange.start, 0,
+    memset(aSpacing.Elements(), 0, sizeof(gfxFont::Spacing) * spacingOffset);
+    nsTArray<PropertyProvider::Spacing> spacingArray;
+    if (!spacingArray.AppendElements(aSpacingRange.Length())) {
+        return false;
+    }
+    GetAdjustedSpacing(this, aSpacingRange, aProvider, spacingArray);
+    for (uint32_t i = 0; i < aSpacingRange.Length(); i++) {
+        aSpacing[i + spacingOffset] = spacingArray[i];
+    }
+    memset(aSpacing.Elements() + aSpacingRange.end - aRange.start, 0,
            sizeof(gfxFont::Spacing) * (aRange.end - aSpacingRange.end));
     return true;
 }
 
 void
 gfxTextRun::ShrinkToLigatureBoundaries(Range* aRange) const
 {
     if (aRange->start >= aRange->end)
@@ -406,17 +415,17 @@ gfxTextRun::ShrinkToLigatureBoundaries(R
 
 void
 gfxTextRun::DrawGlyphs(gfxFont *aFont, Range aRange, gfxPoint *aPt,
                        PropertyProvider *aProvider, Range aSpacingRange,
                        TextRunDrawParams& aParams, uint16_t aOrientation) const
 {
     AutoTArray<PropertyProvider::Spacing,200> spacingBuffer;
     bool haveSpacing = GetAdjustedSpacingArray(aRange, aProvider,
-                                               aSpacingRange, &spacingBuffer);
+                                               aSpacingRange, spacingBuffer);
     aParams.spacing = haveSpacing ? spacingBuffer.Elements() : nullptr;
     aFont->Draw(this, aRange.start, aRange.end, aPt, aParams, aOrientation);
 }
 
 static void
 ClipPartialLigature(const gfxTextRun* aTextRun,
                     gfxFloat *aStart, gfxFloat *aEnd,
                     gfxFloat aOrigin,
@@ -720,17 +729,17 @@ gfxTextRun::DrawEmphasisMarks(gfxContext
         Range ligatureRange(start, end);
         ShrinkToLigatureBoundaries(&ligatureRange);
 
         inlineCoord += direction * ComputePartialLigatureWidth(
             Range(start, ligatureRange.start), aProvider);
 
         AutoTArray<PropertyProvider::Spacing, 200> spacingBuffer;
         bool haveSpacing = GetAdjustedSpacingArray(
-            ligatureRange, aProvider, ligatureRange, &spacingBuffer);
+            ligatureRange, aProvider, ligatureRange, spacingBuffer);
         params.spacing = haveSpacing ? spacingBuffer.Elements() : nullptr;
         font->DrawEmphasisMarks(this, &aPt, ligatureRange.start,
                                 ligatureRange.Length(), params);
 
         inlineCoord += direction * ComputePartialLigatureWidth(
             Range(ligatureRange.end, end), aProvider);
     }
 }
@@ -741,17 +750,17 @@ gfxTextRun::AccumulateMetricsForRun(gfxF
                                     DrawTarget* aRefDrawTarget,
                                     PropertyProvider *aProvider,
                                     Range aSpacingRange,
                                     uint16_t aOrientation,
                                     Metrics *aMetrics) const
 {
     AutoTArray<PropertyProvider::Spacing,200> spacingBuffer;
     bool haveSpacing = GetAdjustedSpacingArray(aRange, aProvider,
-                                               aSpacingRange, &spacingBuffer);
+                                               aSpacingRange, spacingBuffer);
     Metrics metrics = aFont->Measure(this, aRange.start, aRange.end,
                                      aBoundingBoxType, aRefDrawTarget,
                                      haveSpacing ? spacingBuffer.Elements() : nullptr,
                                      aOrientation);
     aMetrics->CombineWith(metrics, IsRightToLeft());
 }
 
 void
@@ -851,17 +860,17 @@ gfxTextRun::BreakAndMeasureText(uint32_t
                                 gfxBreakPriority *aBreakPriority)
 {
     aMaxLength = std::min(aMaxLength, GetLength() - aStart);
 
     NS_ASSERTION(aStart + aMaxLength <= GetLength(), "Substring out of range");
 
     Range bufferRange(aStart, aStart +
         std::min<uint32_t>(aMaxLength, MEASUREMENT_BUFFER_SIZE));
-    PropertyProvider::Spacing spacingBuffer[MEASUREMENT_BUFFER_SIZE];
+    AutoTArray<PropertyProvider::Spacing, MEASUREMENT_BUFFER_SIZE> spacingBuffer;
     bool haveSpacing = aProvider && (mFlags & gfxTextRunFactory::TEXT_ENABLE_SPACING) != 0;
     if (haveSpacing) {
         GetAdjustedSpacing(this, bufferRange, aProvider, spacingBuffer);
     }
     bool hyphenBuffer[MEASUREMENT_BUFFER_SIZE];
     bool haveHyphenation = aProvider &&
         (aProvider->GetHyphensOption() == StyleHyphens::Auto ||
          (aProvider->GetHyphensOption() == StyleHyphens::Manual &&
@@ -1048,20 +1057,19 @@ gfxTextRun::GetAdvanceWidth(Range aRange
 
     // Account for all remaining spacing here. This is more efficient than
     // processing it along with the glyphs.
     if (aProvider && (mFlags & gfxTextRunFactory::TEXT_ENABLE_SPACING)) {
         uint32_t i;
         AutoTArray<PropertyProvider::Spacing,200> spacingBuffer;
         if (spacingBuffer.AppendElements(aRange.Length())) {
             GetAdjustedSpacing(this, ligatureRange, aProvider,
-                               spacingBuffer.Elements());
+                               spacingBuffer);
             for (i = 0; i < ligatureRange.Length(); ++i) {
-                PropertyProvider::Spacing *space = &spacingBuffer[i];
-                result += space->mBefore + space->mAfter;
+                result += spacingBuffer[i].mBefore + spacingBuffer[i].mAfter;
             }
             if (aSpacing) {
                 aSpacing->mBefore = spacingBuffer[0].mBefore;
                 aSpacing->mAfter = spacingBuffer.LastElement().mAfter;
             }
         }
     }
 
--- a/gfx/thebes/gfxTextRun.h
+++ b/gfx/thebes/gfxTextRun.h
@@ -203,17 +203,17 @@ public:
         typedef gfxFont::Spacing Spacing;
 
         /**
          * Get the spacing around the indicated characters. Spacing must be zero
          * inside clusters. In other words, if character i is not
          * CLUSTER_START, then character i-1 must have zero after-spacing and
          * character i must have zero before-spacing.
          */
-        virtual void GetSpacing(Range aRange, Spacing *aSpacing) = 0;
+        virtual void GetSpacing(Range aRange, nsTArray<Spacing>& aSpacing) = 0;
 
         // Returns a gfxContext that can be used to measure the hyphen glyph.
         // Only called if the hyphen width is requested.
         virtual already_AddRefed<DrawTarget> GetDrawTarget() = 0;
 
         // Return the appUnitsPerDevUnit value to be used when measuring.
         // Only called if the hyphen width is requested.
         virtual uint32_t GetAppUnitsPerDevUnit() = 0;
@@ -682,17 +682,17 @@ private:
     int32_t GetAdvanceForGlyphs(Range aRange) const;
 
     // Spacing for characters outside the range aSpacingStart/aSpacingEnd
     // is assumed to be zero; such characters are not passed to aProvider.
     // This is useful to protect aProvider from being passed character indices
     // it is not currently able to handle.
     bool GetAdjustedSpacingArray(Range aRange, PropertyProvider *aProvider,
                                  Range aSpacingRange,
-                                 nsTArray<PropertyProvider::Spacing>*
+                                 nsTArray<PropertyProvider::Spacing>&
                                      aSpacing) const;
 
     CompressedGlyph& EnsureComplexGlyph(uint32_t aIndex)
     {
         gfxShapedText::EnsureComplexGlyph(aIndex, mCharacterGlyphs[aIndex]);
         return mCharacterGlyphs[aIndex];
     }
 
--- a/layout/generic/nsTextFrame.cpp
+++ b/layout/generic/nsTextFrame.cpp
@@ -116,17 +116,17 @@ struct TabWidthStore {
   explicit TabWidthStore(int32_t aValidForContentOffset)
     : mLimit(0)
     , mValidForContentOffset(aValidForContentOffset)
   { }
 
   // Apply tab widths to the aSpacing array, which corresponds to characters
   // beginning at aOffset and has length aLength. (Width records outside this
   // range will be ignored.)
-  void ApplySpacing(gfxTextRun::PropertyProvider::Spacing *aSpacing,
+  void ApplySpacing(nsTArray<gfxTextRun::PropertyProvider::Spacing>& aSpacing,
                     uint32_t aOffset, uint32_t aLength);
 
   // Offset up to which tabs have been measured; positions beyond this have not
   // been calculated yet but may be appended if needed later.  It's a DOM
   // offset relative to the current frame's offset.
   uint32_t mLimit;
 
   // Need to recalc tab offsets if frame content offset differs from this.
@@ -146,17 +146,17 @@ struct TabwidthAdaptor
   uint32_t operator[](size_t aIdx) const {
     return mWidths[aIdx].mOffset;
   }
 };
 
 } // namespace
 
 void
-TabWidthStore::ApplySpacing(gfxTextRun::PropertyProvider::Spacing *aSpacing,
+TabWidthStore::ApplySpacing(nsTArray<gfxTextRun::PropertyProvider::Spacing>& aSpacing,
                             uint32_t aOffset, uint32_t aLength)
 {
   size_t i = 0;
   const size_t len = mWidths.Length();
 
   // If aOffset is non-zero, do a binary search to find where to start
   // processing the tab widths, in case the list is really long. (See bug
   // 953247.)
@@ -3134,32 +3134,33 @@ public:
     NS_ASSERTION(mTextRun, "Textrun not initialized!");
   }
 
   // Call this after construction if you're not going to reflow the text
   void InitializeForDisplay(bool aTrimAfter);
 
   void InitializeForMeasure();
 
-  virtual void GetSpacing(Range aRange, Spacing* aSpacing);
+  virtual void GetSpacing(Range aRange, nsTArray<Spacing>& aSpacing);
   virtual gfxFloat GetHyphenWidth();
   virtual void GetHyphenationBreaks(Range aRange, bool* aBreakBefore);
   virtual StyleHyphens GetHyphensOption() {
     return mTextStyle->mHyphens;
   }
 
   virtual already_AddRefed<DrawTarget> GetDrawTarget() {
     return CreateReferenceDrawTarget(GetFrame());
   }
 
   virtual uint32_t GetAppUnitsPerDevUnit() {
     return mTextRun->GetAppUnitsPerDevUnit();
   }
 
-  void GetSpacingInternal(Range aRange, Spacing* aSpacing, bool aIgnoreTabs);
+  void GetSpacingInternal(Range aRange, nsTArray<Spacing>& aSpacing,
+                          bool aIgnoreTabs);
 
   /**
    * Compute the justification information in given DOM range, return
    * justification info and assignments if requested.
    */
   JustificationInfo ComputeJustification(
     Range aRange, nsTArray<JustificationAssignment>* aAssignments = nullptr);
 
@@ -3354,17 +3355,17 @@ PropertyProvider::ComputeJustification(
   if (aAssignments) {
     *aAssignments = Move(assignments);
   }
   return info;
 }
 
 // aStart, aLength in transformed string offsets
 void
-PropertyProvider::GetSpacing(Range aRange, Spacing* aSpacing)
+PropertyProvider::GetSpacing(Range aRange, nsTArray<Spacing>& aSpacing)
 {
   GetSpacingInternal(aRange, aSpacing,
                      (mTextRun->GetFlags() & nsTextFrameUtils::TEXT_HAS_TAB) == 0);
 }
 
 static bool
 CanAddSpacingAfter(const gfxTextRun* aTextRun, uint32_t aOffset)
 {
@@ -3392,17 +3393,17 @@ ComputeTabWidthAppUnits(nsIFrame* aFrame
   gfxFloat spaceWidthAppUnits =
     NS_round(GetFirstFontMetrics(aTextRun->GetFontGroup(),
                                  aTextRun->IsVertical()).spaceWidth *
              aTextRun->GetAppUnitsPerDevUnit());
   return spaces * spaceWidthAppUnits;
 }
 
 void
-PropertyProvider::GetSpacingInternal(Range aRange, Spacing* aSpacing,
+PropertyProvider::GetSpacingInternal(Range aRange, nsTArray<Spacing>& aSpacing,
                                      bool aIgnoreTabs)
 {
   NS_PRECONDITION(IsInBounds(mStart, mLength, aRange), "Range out of bounds");
 
   uint32_t index;
   for (index = 0; index < aRange.Length(); ++index) {
     aSpacing[index].mBefore = 0.0;
     aSpacing[index].mAfter = 0.0;
@@ -3522,19 +3523,20 @@ PropertyProvider::CalcTabWidths(Range aR
   MOZ_ASSERT(aRange.end <= startOffset + mLength, "beyond the end");
   uint32_t tabsEnd =
     (mTabWidths ? mTabWidths->mLimit : mTabWidthsAnalyzedLimit) + startOffset;
   if (tabsEnd < aRange.end) {
     NS_ASSERTION(mReflowing,
                  "We need precomputed tab widths, but don't have enough.");
 
     for (uint32_t i = tabsEnd; i < aRange.end; ++i) {
-      Spacing spacing;
-      GetSpacingInternal(Range(i, i + 1), &spacing, true);
-      mOffsetFromBlockOriginForTabs += spacing.mBefore;
+      nsTArray<Spacing> spacing;
+      spacing.AppendElements(1);
+      GetSpacingInternal(Range(i, i + 1), spacing, true);
+      mOffsetFromBlockOriginForTabs += spacing[0].mBefore;
 
       if (!mTextRun->CharIsTab(i)) {
         if (mTextRun->IsClusterStart(i)) {
           uint32_t clusterEnd = i + 1;
           while (clusterEnd < mTextRun->GetLength() &&
                  !mTextRun->IsClusterStart(clusterEnd)) {
             ++clusterEnd;
           }
@@ -3548,17 +3550,17 @@ PropertyProvider::CalcTabWidths(Range aR
         }
         double nextTab = AdvanceToNextTab(mOffsetFromBlockOriginForTabs,
                 mFrame, mTextRun, aTabWidth);
         mTabWidths->mWidths.AppendElement(TabWidth(i - startOffset,
                 NSToIntRound(nextTab - mOffsetFromBlockOriginForTabs)));
         mOffsetFromBlockOriginForTabs = nextTab;
       }
 
-      mOffsetFromBlockOriginForTabs += spacing.mAfter;
+      mOffsetFromBlockOriginForTabs += spacing[0].mAfter;
     }
 
     if (mTabWidths) {
       mTabWidths->mLimit = aRange.end - startOffset;
     }
   }
 
   if (!mTabWidths) {
@@ -8419,25 +8421,26 @@ nsTextFrame::AddInlineMinISizeForFlow(ns
           aData->mTrailingWhitespace = std::max(0, wsWidth);
         }
       } else {
         aData->mTrailingWhitespace = 0;
       }
     }
 
     if (preformattedTab) {
-      PropertyProvider::Spacing spacing;
-      provider.GetSpacing(Range(i, i + 1), &spacing);
-      aData->mCurrentLine += nscoord(spacing.mBefore);
+      nsTArray<PropertyProvider::Spacing> spacing;
+      spacing.AppendElements(1);
+      provider.GetSpacing(Range(i, i + 1), spacing);
+      aData->mCurrentLine += nscoord(spacing[0].mBefore);
       if (tabWidth < 0) {
         tabWidth = ComputeTabWidthAppUnits(this, textRun);
       }
       gfxFloat afterTab =
         AdvanceToNextTab(aData->mCurrentLine, this, textRun, tabWidth);
-      aData->mCurrentLine = nscoord(afterTab + spacing.mAfter);
+      aData->mCurrentLine = nscoord(afterTab + spacing[0].mAfter);
       wordStart = i + 1;
     } else if (i < flowEndInTextRun ||
         (i == textRun->GetLength() &&
          (textRun->GetFlags() & nsTextFrameUtils::TEXT_HAS_TRAILING_BREAK))) {
       if (preformattedNewline) {
         aData->ForceBreak();
       } else if (i < flowEndInTextRun && hyphBreakBefore &&
                  hyphBreakBefore[i - start]) {
@@ -8582,25 +8585,26 @@ nsTextFrame::AddInlinePrefISizeForFlow(n
           aData->mTrailingWhitespace = std::max(0, wsWidth);
         }
       } else {
         aData->mTrailingWhitespace = 0;
       }
     }
 
     if (preformattedTab) {
-      PropertyProvider::Spacing spacing;
-      provider.GetSpacing(Range(i, i + 1), &spacing);
-      aData->mCurrentLine += nscoord(spacing.mBefore);
+      nsTArray<PropertyProvider::Spacing> spacing;
+      spacing.AppendElements(1);
+      provider.GetSpacing(Range(i, i + 1), spacing);
+      aData->mCurrentLine += nscoord(spacing[0].mBefore);
       if (tabWidth < 0) {
         tabWidth = ComputeTabWidthAppUnits(this, textRun);
       }
       gfxFloat afterTab =
         AdvanceToNextTab(aData->mCurrentLine, this, textRun, tabWidth);
-      aData->mCurrentLine = nscoord(afterTab + spacing.mAfter);
+      aData->mCurrentLine = nscoord(afterTab + spacing[0].mAfter);
       aData->mLineIsEmpty = false;
       lineStart = i + 1;
     } else if (preformattedNewline) {
       aData->ForceBreak();
       lineStart = i;
     }
   }