[css-shapes] Remove invalid shelves in NGExclusionSpace::DerivedGeometry

It was possible to have some zero sized shelves in the NGExclusionSpace.
This typically didn't cause any issues, but it was possible for zero
sized elements, and things with shape exclusions to be placed in
invalid places due to this.

The updated unittest actually shows an invalid layout opportunity.
The updated png-test was also something which appears we got wrong when
we rolled out LayoutNG initially. (we are now changing it "back" to the
original ref).

Bug: 1156154
Change-Id: Ia275255683764376d4d9e343bacb063d628fbaee
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2581885
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#835785}

third_party/blink/renderer/core/layout/ng/exclusions/ng_exclusion_space.cc

@@ -448,8 +448,22 @@ void NGExclusionSpaceInternal::DerivedGeometry::Add(
         if (exclusion.shape_data)
           shelf.has_shape_exclusions = true;
 
-        // Just in case the shelf has a negative inline-size.
-        shelf.line_right = std::max(shelf.line_left, shelf.line_right);
+        // A shelf can be completely closed off and not needed anymore. For
+        // example:
+        //
+        //    0 1 2 3 4 5 6 7 8
+        // 0  +---+X----------X
+        //    |xxx|
+        // 10 |xxx|
+        //    +---+
+        // 20
+        //       +-----------+
+        // 30    |NEW (right)|
+        //       +-----------+
+        //
+        // In the above example "NEW (right)" will have shrunk the shelf such
+        // that line_right will now be smaller than line_left.
+        bool is_closed_off = shelf.line_left > shelf.line_right;
 
         // We can end up in a situation where a shelf is the same as the
         // previous one. For example:
@@ -469,7 +483,7 @@ void NGExclusionSpaceInternal::DerivedGeometry::Add(
         bool is_same_as_previous =
             (i > 0) && shelf.line_left == shelves_[i - 1].line_left &&
             shelf.line_right == shelves_[i - 1].line_right;
-        if (is_same_as_previous) {
+        if (is_closed_off || is_same_as_previous) {
           shelves_.EraseAt(i);
           --i;
         }

third_party/blink/renderer/core/layout/ng/exclusions/ng_exclusion_space_test.cc

@@ -126,14 +126,12 @@ TEST(NGExclusionSpaceTest, TwoExclusions) {
       /* offset */ {LayoutUnit(), LayoutUnit()},
       /* available_size */ LayoutUnit(400));
 
-  EXPECT_EQ(4u, opportunites.size());
+  EXPECT_EQ(3u, opportunites.size());
   TEST_OPPORTUNITY(opportunites[0], NGBfcOffset(LayoutUnit(150), LayoutUnit()),
                    NGBfcOffset(LayoutUnit(400), LayoutUnit(75)));
-  TEST_OPPORTUNITY(opportunites[1], NGBfcOffset(LayoutUnit(150), LayoutUnit()),
-                   NGBfcOffset(LayoutUnit(150), LayoutUnit::Max()));
-  TEST_OPPORTUNITY(opportunites[2], NGBfcOffset(LayoutUnit(), LayoutUnit(75)),
+  TEST_OPPORTUNITY(opportunites[1], NGBfcOffset(LayoutUnit(), LayoutUnit(75)),
                    NGBfcOffset(LayoutUnit(100), LayoutUnit::Max()));
-  TEST_OPPORTUNITY(opportunites[3], NGBfcOffset(LayoutUnit(), LayoutUnit(150)),
+  TEST_OPPORTUNITY(opportunites[2], NGBfcOffset(LayoutUnit(), LayoutUnit(150)),
                    NGBfcOffset(LayoutUnit(400), LayoutUnit::Max()));
 }