Bug 1425954 Part 1: Strip out duplicate names when reporting line names via grid chrome API. r=mats
authorBrad Werth <bwerth@mozilla.com>
Mon, 18 Dec 2017 15:42:21 -0800
changeset 397028 7c248344b08bf7bd067fe5761a750374f9fcd236
parent 396993 62b281c39548aa349fd1141caed5d4340700bbb6
child 397029 3b489b994f6d1f88c09a68c3032b19cf20b8d162
push id33123
push userncsoregi@mozilla.com
push dateThu, 21 Dec 2017 10:00:47 +0000
treeherdermozilla-central@06a19fbe2581 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmats
bugs1425954
milestone59.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1425954 Part 1: Strip out duplicate names when reporting line names via grid chrome API. r=mats 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