Bug 1425954 Part 1: Strip out duplicate names when reporting line names via grid chrome API. draft
authorBrad Werth <bwerth@mozilla.com>
Mon, 18 Dec 2017 15:42:21 -0800
changeset 713597 0d8de59041894c7c558736d861a7b46d13cc490e
parent 713529 2ff08db67b917fba1558986f3f2f796260f970f8
child 713598 87d11bb6b82a886563addb682af87fecfe9bc3b3
push id93687
push userbwerth@mozilla.com
push dateWed, 20 Dec 2017 17:35:54 +0000
bugs1425954
milestone59.0a1
Bug 1425954 Part 1: Strip out duplicate names when reporting line names via grid chrome API. MozReview-Commit-ID: 9DW6EOFEpR6
dom/grid/GridLines.cpp
--- a/dom/grid/GridLines.cpp
+++ b/dom/grid/GridLines.cpp
@@ -57,16 +57,32 @@ GridLines::IndexedGetter(uint32_t aIndex
 {
   aFound = aIndex < mLines.Length();
   if (!aFound) {
     return nullptr;
   }
   return mLines[aIndex];
 }
 
+static void AddLineNameIfNotPresent(nsTArray<nsString>& aLineNames,
+                             const nsString& aName)
+{
+  if (!aLineNames.Contains(aName)) {
+    aLineNames.AppendElement(aName);
+  }
+}
+
+static void AddLineNamesIfNotPresent(nsTArray<nsString>& aLineNames,
+                              const nsTArray<nsString>& aNames)
+{
+  for (const auto& name : aNames) {
+    AddLineNameIfNotPresent(aLineNames, name);
+  }
+}
+
 void
 GridLines::SetLineInfo(const ComputedGridTrackInfo* aTrackInfo,
                        const ComputedGridLineInfo* aLineInfo,
                        const nsTArray<RefPtr<GridArea>>& aAreas,
                        bool aIsRow)
 {
   MOZ_ASSERT(aLineInfo);
   mLines.Clear();
@@ -109,18 +125,32 @@ GridLines::SetLineInfo(const ComputedGri
       // Since line indexes are 1-based, calculate a 1-based value
       // for this track to simplify some calculations.
       const uint32_t line1Index = i + 1;
 
       startOfNextTrack = (i < aTrackInfo->mEndFragmentTrack) ?
                          aTrackInfo->mPositions[i] :
                          lastTrackEdge;
 
+      // Get the line names for the current line. aLineInfo->mNames
+      // may contain duplicate names. This is intentional, since grid
+      // layout works fine with duplicate names, and we don't want to
+      // detect and remove duplicates in layout since it is an O(n^2)
+      // problem. We do the work here since this is only run when
+      // requested by devtools, and slowness here will not affect
+      // normal browsing.
+      nsTArray<nsString> possiblyDuplicateLineNames(
+        aLineInfo->mNames.SafeElementAt(i, nsTArray<nsString>()));
+
+      // Add the possiblyDuplicateLineNames one at a time to filter
+      // out the duplicates.
       nsTArray<nsString> lineNames;
-      lineNames = aLineInfo->mNames.SafeElementAt(i, nsTArray<nsString>());
+      for (const auto& name : possiblyDuplicateLineNames) {
+        AddLineNameIfNotPresent(lineNames, name);
+      }
 
       // Add in names from grid areas where this line is used as a boundary.
       for (auto area : aAreas) {
         bool haveNameToAdd = false;
         nsAutoString nameToAdd;
         area->GetName(nameToAdd);
         if (aIsRow) {
           if (area->RowStart() == line1Index) {
@@ -135,18 +165,18 @@ GridLines::SetLineInfo(const ComputedGri
             haveNameToAdd = true;
             nameToAdd.AppendLiteral("-start");
           } else if (area->ColumnEnd() == line1Index) {
             haveNameToAdd = true;
             nameToAdd.AppendLiteral("-end");
           }
         }
 
-        if (haveNameToAdd && !lineNames.Contains(nameToAdd)) {
-          lineNames.AppendElement(nameToAdd);
+        if (haveNameToAdd) {
+          AddLineNameIfNotPresent(lineNames, nameToAdd);
         }
       }
 
       if (i >= (aTrackInfo->mRepeatFirstTrack +
                 aTrackInfo->mNumLeadingImplicitTracks) &&
           repeatIndex < numRepeatTracks) {
         numAddedLines += AppendRemovedAutoFits(aTrackInfo,
                                                aLineInfo,
@@ -237,17 +267,17 @@ GridLines::AppendRemovedAutoFits(const C
         aLineNames.RemoveElement(extractedName);
       }
       extractedExplicitLineNames = true;
     }
 
     // If this is the second or later time through, or didn't already
     // have before names, add them.
     if (linesAdded > 0 || !alreadyHasBeforeLineNames) {
-      aLineNames.AppendElements(aLineInfo->mNamesBefore);
+      AddLineNamesIfNotPresent(aLineNames, aLineInfo->mNamesBefore);
     }
 
     RefPtr<GridLine> line = new GridLine(this);
     mLines.AppendElement(line);
 
     // Time to calculate the line numbers. For the positive numbers
     // we count with a 1-based index from mRepeatFirstTrack. Although
     // this number is the index of the first repeat track AFTER all
@@ -280,21 +310,21 @@ GridLines::AppendRemovedAutoFits(const C
     aRepeatIndex++;
 
     linesAdded++;
   }
   aRepeatIndex++;
 
   if (extractedExplicitLineNames) {
     // Pass on the explicit names we saved to the next explicit line.
-    aLineNames.AppendElements(explicitLineNames);
+    AddLineNamesIfNotPresent(aLineNames, explicitLineNames);
   }
 
   if (alreadyHasBeforeLineNames && linesAdded > 0) {
     // If we started with before names, pass them on to the next explicit
     // line.
-    aLineNames.AppendElements(aLineInfo->mNamesBefore);
+    AddLineNamesIfNotPresent(aLineNames, aLineInfo->mNamesBefore);
   }
   return linesAdded;
 }
 
 } // namespace dom
 } // namespace mozilla