RenderSubtree: Fix a DCHECK in fragment painting.

This fixes a bug that happens in an interplay of find in page and mutating locked subtrees. We don't paint the subtree, and the DCHECK happens during the self painting phase of the element. This change ensures that the DCHECK is aware of display locking. R=chrishtr@chromium.org Change-Id: Ib58adaffab9e11d2d813fe54a56ea3aa72ddfd32 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1956038 Commit-Queue: Chris Harrelson <chrishtr@chromium.org> Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Cr-Commit-Position: refs/heads/master@{#722666}

RenderSubtree: Fix a DCHECK in fragment painting.
This fixes a bug that happens in an interplay of find in page and mutating locked subtrees. We don't paint the subtree, and the DCHECK happens during the self painting phase of the element. This change ensures that the DCHECK is aware of display locking. R=chrishtr@chromium.org Change-Id: Ib58adaffab9e11d2d813fe54a56ea3aa72ddfd32 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1956038 Commit-Queue: Chris Harrelson <chrishtr@chromium.org> Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Cr-Commit-Position: refs/heads/master@{#722666}
ff1cc060 · Vladimir Levin · Commit Bot · 8a063346 · ff1cc060 · ff1cc060
Commit ff1cc060 authored Dec 06, 2019 by Vladimir Levin Committed by Commit Bot Dec 06, 2019
2 changed files
--- a/third_party/blink/renderer/core/display_lock/display_lock_context_test.cc
+++ b/third_party/blink/renderer/core/display_lock/display_lock_context_test.cc
@@ -339,6 +339,40 @@ TEST_F(DisplayLockContextTest,
  EXPECT_FALSE(container->GetDisplayLockContext()->IsLocked());
 }

+TEST_F(DisplayLockContextTest,
+       FindInPageWhileLockedContentChangesDoesNotCrash) {
+  ResizeAndFocus();
+  SetHtmlInnerHTML(R"HTML(
+    <style>
+    #container {
+      width: 100px;
+      height: 100px;
+      contain: style layout paint;
+    }
+    </style>
+    <body>testing<div id="container">testing</div></body>
+  )HTML");
+
+  const String search_text = "testing";
+  DisplayLockTestFindInPageClient client;
+  client.SetFrame(LocalMainFrame());
+
+  // Lock the container.
+  auto* container = GetDocument().getElementById("container");
+  LockElement(*container, true /* activatable */);
+  EXPECT_TRUE(container->GetDisplayLockContext()->IsLocked());
+
+  // Find the first "testing", container still locked since the match is outside
+  // the container.
+  Find(search_text, client);
+  EXPECT_EQ(2, client.Count());
+  EXPECT_TRUE(container->GetDisplayLockContext()->IsLocked());
+
+  // Change the inner text, this should not DCHECK.
+  container->SetInnerHTMLFromString("please don't DCHECK");
+  UpdateAllLifecyclePhasesForTest();
+}
+
 TEST_F(DisplayLockContextTest, FindInPageWithChangedContent) {
  if (!RuntimeEnabledFeatures::LayoutNGEnabled())
    return;

--- a/third_party/blink/renderer/core/paint/ng/ng_box_fragment_painter.h
+++ b/third_party/blink/renderer/core/paint/ng/ng_box_fragment_painter.h
@@ -265,7 +265,10 @@ inline NGBoxFragmentPainter::NGBoxFragmentPainter(
    // If no children, there maybe or may not be NGPaintFragment.
    // TODO(kojii): To be investigated if this correct or should be fixed.
    if (!box.Children().empty()) {
-      DCHECK(paint_fragment || box.HasItems());
+      if (!box.GetLayoutObject()->PaintBlockedByDisplayLock(
+              DisplayLockLifecycleTarget::kChildren)) {
+        DCHECK(paint_fragment || box.HasItems());
+      }
      if (paint_fragment)
        DCHECK_EQ(&paint_fragment->PhysicalFragment(), &box);
    }