Backed out 3 changesets (bug 1506687) for devtools failures on browser_flexbox_sizing_info_for_text_nodes. CLOSED TREE
authorCosmin Sabou <csabou@mozilla.com>
Thu, 15 Nov 2018 20:01:52 +0200
changeset 503081 9bff4fddd787a959c07e63032e762dc501f789ea
parent 503080 ebf1f69c10caedbd63923bdf8a99c8d6048ea1c5
child 503082 8a421a0addad5760abd3988f39e388d575e487ad
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1506687
milestone65.0a1
backs out2788a93e179aff117492e29f911c4449cd101a40
1c3baa04d4ce66c9a8b3428b3d475c73bc07f76b
c50af93cfc84373d529aefc2a3ccd423e48b0d1f
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
Backed out 3 changesets (bug 1506687) for devtools failures on browser_flexbox_sizing_info_for_text_nodes. CLOSED TREE Backed out changeset 2788a93e179a (bug 1506687) Backed out changeset 1c3baa04d4ce (bug 1506687) Backed out changeset c50af93cfc84 (bug 1506687)
dom/chrome-webidl/Flex.webidl
dom/flex/FlexItemValues.cpp
dom/flex/FlexItemValues.h
dom/flex/test/chrome/test_flex_items.html
layout/generic/nsFlexContainerFrame.cpp
layout/generic/nsFlexContainerFrame.h
--- a/dom/chrome-webidl/Flex.webidl
+++ b/dom/chrome-webidl/Flex.webidl
@@ -78,20 +78,16 @@ interface FlexLineValues
 enum FlexItemClampState {
   "unclamped", "clamped_to_min", "clamped_to_max"
 };
 
 [ChromeOnly]
 interface FlexItemValues
 {
   readonly attribute Node? node;
-  readonly attribute double mainPosition;
-  readonly attribute double mainSize;
   readonly attribute double mainBaseSize;
   readonly attribute double mainDeltaSize;
   readonly attribute double mainMinSize;
   readonly attribute double mainMaxSize;
-  readonly attribute double crossPosition;
-  readonly attribute double crossSize;
   readonly attribute double crossMinSize;
   readonly attribute double crossMaxSize;
   readonly attribute FlexItemClampState clampState;
 };
--- a/dom/flex/FlexItemValues.cpp
+++ b/dom/flex/FlexItemValues.cpp
@@ -45,31 +45,23 @@ FlexItemValues::FlexItemValues(FlexLineV
   MOZ_ASSERT(aItem,
     "Should never be instantiated with a null ComputedFlexLineInfo.");
 
   // Eagerly copy values from aItem, because we're not
   // going to keep it around.
   mNode = aItem->mNode;
 
   // Convert app unit sizes to css pixel sizes.
-  mMainPosition = nsPresContext::AppUnitsToDoubleCSSPixels(
-    aItem->mMainPosition);
-  mMainSize = nsPresContext::AppUnitsToDoubleCSSPixels(
-    aItem->mMainSize);
   mMainBaseSize = nsPresContext::AppUnitsToDoubleCSSPixels(
     aItem->mMainBaseSize);
   mMainDeltaSize = nsPresContext::AppUnitsToDoubleCSSPixels(
     aItem->mMainDeltaSize);
   mMainMinSize = nsPresContext::AppUnitsToDoubleCSSPixels(
     aItem->mMainMinSize);
   mMainMaxSize = ToPossiblyUnconstrainedPixels(aItem->mMainMaxSize);
-  mCrossPosition = nsPresContext::AppUnitsToDoubleCSSPixels(
-    aItem->mCrossPosition);
-  mCrossSize = nsPresContext::AppUnitsToDoubleCSSPixels(
-    aItem->mCrossSize);
   mCrossMinSize = nsPresContext::AppUnitsToDoubleCSSPixels(
     aItem->mCrossMinSize);
   mCrossMaxSize = ToPossiblyUnconstrainedPixels(aItem->mCrossMaxSize);
 
   mClampState = aItem->mClampState;
 }
 
 JSObject*
@@ -80,28 +72,16 @@ FlexItemValues::WrapObject(JSContext* aC
 
 nsINode*
 FlexItemValues::GetNode() const
 {
   return mNode;
 }
 
 double
-FlexItemValues::MainPosition() const
-{
-  return mMainPosition;
-}
-
-double
-FlexItemValues::MainSize() const
-{
-  return mMainSize;
-}
-
-double
 FlexItemValues::MainBaseSize() const
 {
   return mMainBaseSize;
 }
 
 double
 FlexItemValues::MainDeltaSize() const
 {
@@ -116,28 +96,16 @@ FlexItemValues::MainMinSize() const
 
 double
 FlexItemValues::MainMaxSize() const
 {
   return mMainMaxSize;
 }
 
 double
-FlexItemValues::CrossPosition() const
-{
-  return mCrossPosition;
-}
-
-double
-FlexItemValues::CrossSize() const
-{
-  return mCrossSize;
-}
-
-double
 FlexItemValues::CrossMinSize() const
 {
   return mCrossMinSize;
 }
 
 double
 FlexItemValues::CrossMaxSize() const
 {
--- a/dom/flex/FlexItemValues.h
+++ b/dom/flex/FlexItemValues.h
@@ -34,41 +34,33 @@ public:
 
   virtual JSObject* WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) override;
   FlexLineValues* GetParentObject()
   {
     return mParent;
   }
 
   nsINode* GetNode() const;
-  double MainPosition() const;
-  double MainSize() const;
   double MainBaseSize() const;
   double MainDeltaSize() const;
   double MainMinSize() const;
   double MainMaxSize() const;
-  double CrossPosition() const;
-  double CrossSize() const;
   double CrossMinSize() const;
   double CrossMaxSize() const;
   FlexItemClampState ClampState() const;
 
 protected:
   RefPtr<FlexLineValues> mParent;
   RefPtr<nsINode> mNode;
 
-  // These measurements are all CSS pixel units.
-  double mMainPosition;
-  double mMainSize;
+  // These sizes are all CSS pixel units.
   double mMainBaseSize;
   double mMainDeltaSize;
   double mMainMinSize;
   double mMainMaxSize;
-  double mCrossPosition;
-  double mCrossSize;
   double mCrossMinSize;
   double mCrossMaxSize;
   FlexItemClampState mClampState;
 };
 
 } // namespace dom
 } // namespace mozilla
 
--- a/dom/flex/test/chrome/test_flex_items.html
+++ b/dom/flex/test/chrome/test_flex_items.html
@@ -37,41 +37,36 @@
   .white       { background: white;  }
 
   .crossMinMax { min-height: 40px;
                  max-height: 120px; }
 
   .mainMinMax  { min-width: 120px;
                  max-width: 500px; }
 
+  .flexGrow    { flex-grow: 1; }
   .spacer150   { width: 150px;
                  box-sizing: border-box;
                  height: 10px;
                  border: 1px solid teal; }
 
 </style>
 
 <script>
 "use strict";
 
 SimpleTest.waitForExplicitFinish();
 
+const TEXT_NODE = Node.TEXT_NODE;
+
 function testItemMatchesExpectedValues(item, values, index) {
   if (typeof(values.node) != "undefined") {
     is(item.node, values.node, "Item index " + index + " has expected node.");
   }
 
-  if (typeof(values.mainPosition) != "undefined") {
-    is(item.mainPosition, values.mainPosition, "Item index " + index + " has expected mainPosition.");
-  }
-
-  if (typeof(values.mainSize) != "undefined") {
-    is(item.mainSize, values.mainSize, "Item index " + index + " has expected mainSize.");
-  }
-
   if (typeof(values.mainBaseSize) != "undefined") {
     is(item.mainBaseSize, values.mainBaseSize, "Item index " + index + " has expected mainBaseSize.");
   }
 
   if (typeof(values.mainDeltaSize) != "undefined") {
     is(item.mainDeltaSize, values.mainDeltaSize, "Item index " + index + " has expected mainDeltaSize.");
   }
 
@@ -84,24 +79,16 @@ function testItemMatchesExpectedValues(i
   } else {
     // If we didn't specify an expected mainMaxSize, then it's implied
     // that the max main-size property (max-width/max-height) is at its
     // default "none" value, which our FlexItem API represents as +infinity.
     is(item.mainMaxSize, Number.POSITIVE_INFINITY,
        "Item index " + index + " has expected (default) mainMaxSize.");
   }
 
-  if (typeof(values.crossPosition) != "undefined") {
-    is(item.crossPosition, values.crossPosition, "Item index " + index + " has expected crossPosition.");
-  }
-
-  if (typeof(values.crossSize) != "undefined") {
-    is(item.crossSize, values.crossSize, "Item index " + index + " has expected crossSize.");
-  }
-
   if (typeof(values.crossMinSize) != "undefined") {
     is(item.crossMinSize, values.crossMinSize, "Item index " + index + " has expected crossMinSize.");
   }
 
   if (typeof(values.crossMaxSize) != "undefined") {
     is(item.crossMaxSize, values.crossMaxSize, "Item index " + index + " has expected crossMaxSize.");
   } else {
     // As above for mainMaxSize, no-expected-value implies we expect +infinity.
@@ -138,51 +125,41 @@ function testFlexSanity() {
   let lbElem = document.querySelector(".lastbase");
   let lbElemBoundingRect = lbElem.getBoundingClientRect();
   ok(line.lastBaselineOffset > containerHeight - lbElemBoundingRect.bottom &&
      line.lastBaselineOffset < containerHeight - lbElemBoundingRect.top,
      "Line lastBaselineOffset should land somewhere within the element" +
      "that determines it.");
 
   let expectedValues = [
-    { mainPosition:    0,
-      crossMinSize:    0 },
-    { mainPosition:  300,
-      mainBaseSize:  100,
-      mainDeltaSize:   0 },
-    { crossMinSize:   40,
-      crossMaxSize:  120,
-      mainDeltaSize:   0 },
-    { mainMinSize:   120,
-      mainMaxSize:   500,
-      mainDeltaSize:   0 },
-    { mainMinSize:   120,
-      mainMaxSize:   500,
-      mainBaseSize:  150,
-      mainDeltaSize:   0 },
-    { mainSize:        5,
-      mainBaseSize:   10,
-      mainMaxSize:     5,
-      mainDeltaSize:   0 },
-    { mainSize:       15,
-      mainBaseSize:   10,
-      mainMinSize:    15,
-      mainDeltaSize:   0 },
-    { mainSize:       10,
-      mainBaseSize:   50,
-      mainMaxSize:    10 },
-    { crossSize:      20 },
-    { mainSize:       10,
-      mainBaseSize: 1650,
-      mainMaxSize:    10,
-      mainDeltaSize:   0 },
-    { mainDeltaSize:   0 },
-    { crossPosition:  10,
-      crossSize:      40 },
-    { mainSize:       20 },
+    { crossMinSize: 0 },
+    { mainBaseSize: 100,
+      mainDeltaSize: 0 },
+    { crossMinSize: 40,
+      crossMaxSize: 120,
+      mainDeltaSize: 0 },
+    { mainMinSize: 120,
+      mainMaxSize: 500,
+      mainDeltaSize: 0 },
+    { mainMinSize: 120,
+      mainMaxSize: 500,
+      mainBaseSize: 150,
+      mainDeltaSize: 0 },
+    { mainBaseSize:  10,
+      mainMaxSize:   5,
+      mainDeltaSize: 0 },
+    { mainBaseSize: 10,
+      mainMinSize:  15,
+      mainDeltaSize: 0 },
+    { mainBaseSize: 50,
+      mainMaxSize: 10 },
+    { mainBaseSize: 1650,
+      mainMaxSize: 10,
+      mainDeltaSize: 0 },
+    { mainDeltaSize: 0 },
     { /* final item is anonymous flex item */ },
   ];
 
   let items = line.getItems();
   is(items.length, expectedValues.length,
      "Line should have expected number of items.");
   is(items.length, container.children.length + 1,
      "Line should have as many items as the flex container has child elems, " +
@@ -196,21 +173,27 @@ function testFlexSanity() {
     // - the display:contents element (whose item is its first child)
     // - the final item (which is an anonymous flex item around text)
     if (i < container.children.length) {
       let curElem = container.children[i];
       values.node = (curElem.style.display == "contents"
                      ? curElem.firstElementChild
                      : curElem);
     } else {
-      // The final item should be an anonymous flex item with no node reported.
-      values.node = null;
+      is(container.lastChild.nodeType, TEXT_NODE,
+         "container's last child should be a text node");
+      values.node = container.lastChild;
     }
     testItemMatchesExpectedValues(item, values, i);
   }
+
+  // Check that the delta size of the first item is (roughly) equal to the
+  // actual size minus the base size.
+  isfuzzy(items[0].mainDeltaSize, firstRect.width - items[0].mainBaseSize, 1e-4,
+          "flex-grow item should have expected mainDeltaSize.");
 }
 
 // Test for items in "flex-growing" flex container:
 function testFlexGrowing() {
   let expectedValues = [
     { mainBaseSize:  10,
       mainDeltaSize: 10,
       mainMinSize:   35 },
@@ -247,40 +230,36 @@ function runTests() {
 <body onLoad="runTests();">
   <!-- First flex container to be tested: "flex-sanity".
        We test a few general things, e.g.:
        - accuracy of reported baselines.
        - accuracy of reported flex base size, min/max sizes, & trivial deltas.
        - flex items formation for display:contents subtrees & text nodes.
    -->
   <div id="flex-sanity" class="container">
-    <div class="lime base" style="min-width: 300px">one line (first)</div>
+    <div class="lime base flexGrow">one line (first)</div>
     <div class="yellow lastbase" style="width: 100px">one line (last)</div>
     <div class="orange offset lastbase crossMinMax">two<br/>lines and offset (last)</div>
     <div class="pink offset base mainMinMax">offset (first)</div>
     <!-- Inflexible item w/ content-derived flex base size, which has min/max
          but doesn't violate them: -->
     <div class="tan mainMinMax">
       <div class="spacer150"></div>
     </div>
     <!-- Inflexible item that is trivially clamped to smaller max-width: -->
     <div style="flex: 0 0 10px; max-width: 5px"></div>
     <!-- Inflexible item that is trivially clamped to larger min-width: -->
     <div style="flex: 0 0 10px; min-width: 15px"></div>
     <!-- Item that wants to grow but is trivially clamped to max-width
          below base size: -->
     <div style="flex: 1 1 50px; max-width: 10px"></div>
-    <!-- Item with a fixed cross size: -->
-    <div style="height:20px"></div>
     <div class="clamped-huge-item"></div>
     <div style="display:contents">
       <div class="white">replaced</div>
     </div>
-    <table style="margin-top: 10px"></table>
-    <div style="display:table-cell; width: 20px"></div>
     anon item for text
   </div>
 
   <!-- Second flex container to be tested, with items that grow by specific
        predictable amounts. This ends up triggering 4 passes of the main
        flexbox layout algorithm loop - and note that for each item, we only
        report (via 'mainDeltaSize') the delta that it wanted in the *last pass
        of the loop before that item was frozen*.
--- a/layout/generic/nsFlexContainerFrame.cpp
+++ b/layout/generic/nsFlexContainerFrame.cpp
@@ -156,16 +156,59 @@ ConvertLegacyStyleToJustifyContent(const
       return NS_STYLE_ALIGN_SPACE_BETWEEN;
   }
 
   MOZ_ASSERT_UNREACHABLE("Unrecognized mBoxPack enum value");
   // Fall back to default value of "justify-content" property:
   return NS_STYLE_ALIGN_FLEX_START;
 }
 
+// Helper-function to find the first non-anonymous-box descendent of aFrame.
+static nsIFrame*
+GetFirstNonAnonBoxDescendant(nsIFrame* aFrame)
+{
+  while (aFrame) {
+    nsAtom* pseudoTag = aFrame->Style()->GetPseudo();
+
+    // If aFrame isn't an anonymous container, then it'll do.
+    if (!pseudoTag ||                                 // No pseudotag.
+        !nsCSSAnonBoxes::IsAnonBox(pseudoTag) ||      // Pseudotag isn't anon.
+        nsCSSAnonBoxes::IsNonElement(pseudoTag)) {    // Text, not a container.
+      break;
+    }
+
+    // Otherwise, descend to its first child and repeat.
+
+    // SPECIAL CASE: if we're dealing with an anonymous table, then it might
+    // be wrapping something non-anonymous in its caption or col-group lists
+    // (instead of its principal child list), so we have to look there.
+    // (Note: For anonymous tables that have a non-anon cell *and* a non-anon
+    // column, we'll always return the column. This is fine; we're really just
+    // looking for a handle to *anything* with a meaningful content node inside
+    // the table, for use in DOM comparisons to things outside of the table.)
+    if (MOZ_UNLIKELY(aFrame->IsTableWrapperFrame())) {
+      nsIFrame* captionDescendant =
+        GetFirstNonAnonBoxDescendant(aFrame->GetChildList(kCaptionList).FirstChild());
+      if (captionDescendant) {
+        return captionDescendant;
+      }
+    } else if (MOZ_UNLIKELY(aFrame->IsTableFrame())) {
+      nsIFrame* colgroupDescendant =
+        GetFirstNonAnonBoxDescendant(aFrame->GetChildList(kColGroupList).FirstChild());
+      if (colgroupDescendant) {
+        return colgroupDescendant;
+      }
+    }
+
+    // USUAL CASE: Descend to the first child in principal list.
+    aFrame = aFrame->PrincipalChildList().FirstChild();
+  }
+  return aFrame;
+}
+
 // Indicates whether advancing along the given axis is equivalent to
 // increasing our X or Y position (as opposed to decreasing it).
 static inline bool
 AxisGrowsInPositiveDirection(AxisOrientationType aAxis)
 {
   return eAxis_LR == aAxis || eAxis_TB == aAxis;
 }
 
@@ -4778,34 +4821,47 @@ nsFlexContainerFrame::DoFlexLayout(nsPre
       // when we have real values. But we still add all the items here, so
       // we can capture computed data for each item as we proceed.
       for (const FlexItem* item = line->GetFirstItem(); item;
            item = item->getNext()) {
         nsIFrame* frame = item->Frame();
 
         // The frame may be for an element, or it may be for an
         // anonymous flex item, e.g. wrapping one or more text nodes.
-        // If it's for an element, we want to return the DOM node that
-        // corresponds to it. If it's an anonymous flex item, we want
-        // to return a null DOM node.
-        nsINode* node = nullptr;
-
-        // If this frame is not an anonymous flex item, we will return its
-        // content as the DOM node for this flex item.
-        nsAtom* pseudoTag = frame->Style()->GetPseudo();
-        if (pseudoTag != nsCSSAnonBoxes::anonymousFlexItem()) {
-          node = frame->GetContent();
+        // DevTools wants the content node for the actual child in
+        // the DOM tree, so we descend through anonymous boxes.
+        nsIFrame* targetFrame = GetFirstNonAnonBoxDescendant(frame);
+        nsIContent* content = targetFrame->GetContent();
+
+        // Skip over content that is only whitespace, which might
+        // have been broken off from a text node which is our real
+        // target.
+        while (content && content->TextIsOnlyWhitespace()) {
+          // If content is only whitespace, try the frame sibling.
+          targetFrame = targetFrame->GetNextSibling();
+          if (targetFrame) {
+            content = targetFrame->GetContent();
+          } else {
+            content = nullptr;
+          }
         }
 
-        ComputedFlexItemInfo* itemInfo = lineInfo->mItems.AppendElement();
-        itemInfo->mNode = node;
-
-        // itemInfo->mMainBaseSize and mMainDeltaSize will be filled out
-        // in ResolveFlexibleLengths(). Other measurements will be captured
-        // at the end of this function.
+        ComputedFlexItemInfo* itemInfo =
+          lineInfo->mItems.AppendElement();
+
+        itemInfo->mNode = content;
+
+        // mMainBaseSize and itemInfo->mMainDeltaSize will
+        // be filled out in ResolveFlexibleLengths().
+
+        // Other FlexItem properties can be captured now.
+        itemInfo->mMainMinSize = item->GetMainMinSize();
+        itemInfo->mMainMaxSize = item->GetMainMaxSize();
+        itemInfo->mCrossMinSize = item->GetCrossMinSize();
+        itemInfo->mCrossMaxSize = item->GetCrossMaxSize();
       }
     }
   }
 
   aContentBoxMainSize =
     ResolveFlexContainerMainSize(aReflowInput, aAxisTracker,
                                  aContentBoxMainSize, aAvailableBSizeForContent,
                                  lines.getFirst(), aStatus);
@@ -5200,40 +5256,26 @@ nsFlexContainerFrame::DoFlexLayout(nsPre
     ConsiderChildOverflow(aDesiredSize.mOverflowAreas, childFrame);
   }
 
   FinishReflowWithAbsoluteFrames(aPresContext, aDesiredSize,
                                  aReflowInput, aStatus);
 
   NS_FRAME_SET_TRUNCATION(aStatus, aReflowInput, aDesiredSize)
 
-  // Finally update our line and item measurements in our containerInfo.
+  // Finally update our line sizing values in our containerInfo.
   if (MOZ_UNLIKELY(containerInfo)) {
     lineIndex = 0;
     for (const FlexLine* line = lines.getFirst(); line;
          line = line->getNext(), ++lineIndex) {
       ComputedFlexLineInfo& lineInfo = containerInfo->mLines[lineIndex];
 
       lineInfo.mCrossSize = line->GetLineCrossSize();
       lineInfo.mFirstBaselineOffset = line->GetFirstBaselineOffset();
       lineInfo.mLastBaselineOffset = line->GetLastBaselineOffset();
-
-      uint32_t itemIndex = 0;
-      for (const FlexItem* item = line->GetFirstItem(); item;
-           item = item->getNext(), ++itemIndex) {
-        ComputedFlexItemInfo& itemInfo = lineInfo.mItems[itemIndex];
-        itemInfo.mMainPosition = item->GetMainPosition();
-        itemInfo.mMainSize = item->GetMainSize();
-        itemInfo.mMainMinSize = item->GetMainMinSize();
-        itemInfo.mMainMaxSize = item->GetMainMaxSize();
-        itemInfo.mCrossPosition = item->GetCrossPosition();
-        itemInfo.mCrossSize = item->GetCrossSize();
-        itemInfo.mCrossMinSize = item->GetCrossMinSize();
-        itemInfo.mCrossMaxSize = item->GetCrossMaxSize();
-      }
     }
   }
 }
 
 void
 nsFlexContainerFrame::MoveFlexItemToFinalPosition(
   const ReflowInput& aReflowInput,
   const FlexItem& aItem,
--- a/layout/generic/nsFlexContainerFrame.h
+++ b/layout/generic/nsFlexContainerFrame.h
@@ -28,18 +28,16 @@ nsContainerFrame* NS_NewFlexContainerFra
  * extracted by devtools via Chrome APIs. The structures are only
  * created when requested in GetFlexFrameWithComputedInfo(), and
  * the structures are attached to the nsFlexContainerFrame via the
  * FlexContainerInfo property.
  */
 struct ComputedFlexItemInfo
 {
   nsCOMPtr<nsINode> mNode;
-  nscoord mMainPosition;
-  nscoord mMainSize;
   /**
    * mMainBaseSize is a measure of the size of the item in the main
    * axis before the flex sizing algorithm is applied. In the spec,
    * this is called "flex base size", but we use this name to connect
    * the value to the other main axis sizes.
    */
   nscoord mMainBaseSize;
   /**
@@ -49,18 +47,16 @@ struct ComputedFlexItemInfo
    * flex layout algorithm "wants" to shrink or grow the item, and
    * would do, if it was unconstrained. Since the flex sizing
    * algorithm proceeds linearly, the mMainDeltaSize for an item only
    * respects the resolved size of items already frozen.
    */
   nscoord mMainDeltaSize;
   nscoord mMainMinSize;
   nscoord mMainMaxSize;
-  nscoord mCrossPosition;
-  nscoord mCrossSize;
   nscoord mCrossMinSize;
   nscoord mCrossMaxSize;
   mozilla::dom::FlexItemClampState mClampState;
 };
 
 struct ComputedFlexLineInfo
 {
   nsTArray<ComputedFlexItemInfo> mItems;