SubtreeVisibility: Allow quirks mode percent descendants to remain dirty.

This patch does two things:
1. It ensures that when we have percent descendants that cross a
   display-lock boundary, they are allowed to remain dirty (since the
   display lock would block the traversal)
2. Ensures that if we force unlock a lock, bookkeeping is properly
   updated.
3. Adds a missing compositing dirty bit propagation to the
   DisplayLockContext::Unlock function

R=cbiesinger@chromium.org, ikilpatrick@chromium.org

Fixed: 1070017
Change-Id: I11e896c59421ad99c7a0a274902053a8787a7aae
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2148823
Commit-Queue: vmpstr <vmpstr@chromium.org>
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#759425}

third_party/blink/renderer/core/display_lock/display_lock_context.cc

@@ -532,6 +532,7 @@ void DisplayLockContext::Unlock() {
   MarkForLayoutIfNeeded();
   MarkAncestorsForPrePaintIfNeeded();
   MarkPaintLayerNeedsRepaint();
+  MarkForCompositingUpdatesIfNeeded();
 }
 
 void DisplayLockContext::AddToWhitespaceReattachSet(Element& element) {
@@ -883,7 +884,8 @@ bool DisplayLockContext::ForceUnlockIfNeeded() {
   // commit() isn't in progress, the web author won't know that the element
   // got unlocked. Figure out how to notify the author.
   if (auto* reason = ShouldForceUnlock()) {
-    is_locked_ = false;
+    if (IsLocked())
+      Unlock();
     return true;
   }
   return false;

third_party/blink/renderer/core/display_lock/display_lock_context_test.cc

@@ -2172,4 +2172,80 @@ TEST_F(DisplayLockContextRenderingTest, NestedLockDoesHideWhenItIsOffscreen) {
   // We're locked.
   EXPECT_TRUE(inner_context->IsLocked());
 }
+
+TEST_F(DisplayLockContextRenderingTest, ForcedUnlockBookkeeping) {
+  SetHtmlInnerHTML(R"HTML(
+    <style>
+      .hidden { subtree-visibility: hidden; }
+      .inline { display: inline; }
+    </style>
+    <div id=target class=hidden></div>
+  )HTML");
+
+  auto* target = GetDocument().getElementById("target");
+  auto* context = target->GetDisplayLockContext();
+
+  ASSERT_TRUE(context);
+  EXPECT_TRUE(context->IsLocked());
+  EXPECT_EQ(
+      GetDocument().GetDisplayLockDocumentState().LockedDisplayLockCount(), 1);
+
+  target->classList().Add("inline");
+  UpdateAllLifecyclePhasesForTest();
+
+  EXPECT_FALSE(context->IsLocked());
+  EXPECT_EQ(
+      GetDocument().GetDisplayLockDocumentState().LockedDisplayLockCount(), 0);
+}
+
+class DisplayLockContextLegacyRenderingTest
+    : public RenderingTest,
+      private ScopedCSSSubtreeVisibilityHiddenMatchableForTest,
+      private ScopedLayoutNGForTest {
+ public:
+  DisplayLockContextLegacyRenderingTest()
+      : RenderingTest(MakeGarbageCollected<SingleChildLocalFrameClient>()),
+        ScopedCSSSubtreeVisibilityHiddenMatchableForTest(true),
+        ScopedLayoutNGForTest(false) {}
+};
+
+TEST_F(DisplayLockContextLegacyRenderingTest,
+       QuirksHiddenParentBlocksChildLayout) {
+  GetDocument().SetCompatibilityMode(Document::kQuirksMode);
+  SetHtmlInnerHTML(R"HTML(
+    <style>
+      .hidden { subtree-visibility: hidden; }
+      #grandparent { height: 100px; }
+      #parent { height: auto; }
+      #item { height: 10%; }
+    </style>
+    <div id=grandparent>
+      <div id=parent>
+        <div>
+          <div id=item></div>
+        </div>
+      </div>
+    </div>
+  )HTML");
+
+  auto* grandparent = GetDocument().getElementById("grandparent");
+  auto* parent = GetDocument().getElementById("parent");
+  auto* item = GetDocument().getElementById("item");
+
+  auto* grandparent_box = ToLayoutBox(grandparent->GetLayoutObject());
+  auto* item_box = ToLayoutBox(item->GetLayoutObject());
+
+  ASSERT_TRUE(grandparent_box);
+  ASSERT_TRUE(parent->GetLayoutObject());
+  ASSERT_TRUE(item_box);
+
+  EXPECT_EQ(item_box->PercentHeightContainer(), grandparent_box);
+  parent->classList().Add("hidden");
+  grandparent->setAttribute(html_names::kStyleAttr, "height: 150px");
+
+  // This shouldn't DCHECK. We are allowed to have dirty percent height
+  // descendants in quirks mode since they can cross a display-lock boundary.
+  UpdateAllLifecyclePhasesForTest();
+}
+
 }  // namespace blink

third_party/blink/renderer/core/layout/subtree_layout_scope.cc

@@ -30,6 +30,7 @@
 
 #include "third_party/blink/renderer/core/layout/subtree_layout_scope.h"
 
+#include "third_party/blink/renderer/core/display_lock/display_lock_utilities.h"
 #include "third_party/blink/renderer/core/frame/local_frame_view.h"
 #include "third_party/blink/renderer/core/layout/layout_object.h"
 
@@ -46,8 +47,29 @@ SubtreeLayoutScope::~SubtreeLayoutScope() {
                                     DisplayLockLifecycleTarget::kChildren));
 
 #if DCHECK_IS_ON()
-  for (auto* layout_object : layout_objects_to_layout_)
-    layout_object->AssertLaidOut();
+  for (auto* layout_object : layout_objects_to_layout_) {
+    // There are situations where the object to layout was never laid out, such
+    // as if there was a display-locked descendant of the root and ancestor of
+    // the object which prevented layout. This can happen in quirks mode, where
+    // an ancestor can mark a descendant as dirty through its
+    // PercentHeightDescendants() list, which will not get cleared because
+    // traversal is blocked by a display lock. This finds such cases and allows
+    // these objects to be dirty.
+    bool object_allowed_to_be_dirty = false;
+    for (auto* ancestor = DisplayLockUtilities::NearestLockedExclusiveAncestor(
+             *layout_object);
+         ancestor;
+         ancestor =
+             DisplayLockUtilities::NearestLockedExclusiveAncestor(*ancestor)) {
+      if (!ancestor->GetDisplayLockContext()->ShouldLayout(
+              DisplayLockLifecycleTarget::kChildren)) {
+        object_allowed_to_be_dirty = true;
+        break;
+      }
+    }
+    if (!object_allowed_to_be_dirty)
+      layout_object->AssertLaidOut();
+  }
 #endif
 }