New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 850664 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug

Blocking:
issue 738611



Sign in to add a comment

DCHECK failure when opening <dialog> in Shadow DOM v1

Project Member Reported by dpa...@chromium.org, Jun 7 2018

Issue description

Issue found by rbpotter@.

Minimal repro example attached (also pasting below). Need to have DCHECKs enabled to reproduce. Note that this is blocking WebUIs migration to Polymer 2 (which uses Shadow DOM v1)

<button>Click to show dialog</button>
<custom-dialog>
  <list-item></list-item>
</custom-dialog>

<script>
  customElements.define('list-item', class extends HTMLElement {
    constructor() {
      super();
      const shadowRoot = this.attachShadow({mode: 'open'});
      shadowRoot.innerHTML = '<div></div>';
    }
  });

  customElements.define('custom-dialog', class extends HTMLElement {
    constructor() {
      super();
      const shadowRoot = this.attachShadow({mode: 'open'});
      shadowRoot.innerHTML = '<dialog><slot></slot></dialog>';
    }

    show() {
      this.shadowRoot.querySelector('dialog').showModal();
    }
  });

  const b = document.body.querySelector('button');
  b.addEventListener('click', () => {
    const el = document.body.querySelector('custom-dialog');
    el.show();
  });
</script>


Pasting stacktrace below.

1:1:0607/123312.503686:FATAL:document.cc(2071)] Check failed: !node.NeedsReattachLayoutTree(). 
#0 0x7f2e6b931f6c base::debug::StackTrace::StackTrace()
#1 0x7f2e6b87b6db logging::LogMessage::~LogMessage()
#2 0x7f2e63346a39 blink::AssertLayoutTreeUpdated()
#3 0x7f2e633469b2 blink::AssertLayoutTreeUpdated()
#4 0x7f2e63341124 blink::Document::UpdateStyleAndLayoutTree()
#5 0x7f2e63347990 blink::Document::UpdateStyleAndLayoutTreeIgnorePendingStylesheets()
#6 0x7f2e63347172 blink::Document::UpdateStyleAndLayoutIgnorePendingStylesheets()
#7 0x7f2e637eed64 blink::HTMLDialogElement::showModal()
#8 0x7f2e640623dd blink::V8HTMLDialogElement::showModalMethodCallback()
#9 0x7f2e648d4813 v8::internal::FunctionCallbackArguments::Call()
#10 0x7f2e648d2b72 v8::internal::(anonymous namespace)::HandleApiCallHelper<>()
#11 0x7f2e648d0f38 v8::internal::Builtin_Impl_HandleApiCall()
#12 0x7f2e648d097d v8::internal::Builtin_HandleApiCall()
#13 0x7f2e654deed5 <unknown>

 
shadow_dom_dialog_bug.html
854 bytes View Download
Blocking: 738611
@hayato: They seem different bugs to me. One is using Shadow DOM V0, one is using v1, and the stack traces look different as well.
Owner: kochi@chromium.org
Status: Assigned (was: Untriaged)
dpaped@, thanks.
This can be a regression.

kochi@, could you work on this?

Labels: -Pri-2 Pri-1

Comment 6 by kochi@chromium.org, Jun 8 2018

Status: Started (was: Assigned)
I'll work on this.

Comment 7 by kochi@chromium.org, Jun 8 2018

Owner: futhark@chromium.org
Status: Assigned (was: Started)
git bisect found the following is the start of this crash:
https://chromium-review.googlesource.com/1070979

futhark@, could you take a look?

Comment 8 Deleted

Smaller test:

<!DOCTYPE html>
<div id="dialog">
  <div id="item"></div>
</div>
<script>
  const itemRoot = item.attachShadow({mode: 'open'});
  const dialogRoot = dialog.attachShadow({mode: 'open'});
  dialogRoot.innerHTML = '<dialog><slot></slot></dialog>';
  dialog.offsetTop;
  dialogRoot.firstChild.showModal();
</script>

Project Member

Comment 10 by bugdroid1@chromium.org, Jun 11 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/864a1050facb4d6cbb159404ebac88d6b18f0bfb

commit 864a1050facb4d6cbb159404ebac88d6b18f0bfb
Author: Rune Lillesveen <futhark@chromium.org>
Date: Mon Jun 11 19:06:16 2018

[Squad] Clear ChildNeedsStyleRecalc() when recalc for reattach.

Otherwise, we ended up recalculating style for an element twice when we
first recalculated style for re-attach of slotted elements in the flat
tree and then traversed the same slotted children in their light-tree
position.

This caused a DCHECK failure for clean style and layout.

Added more DCHECKs to detect if we do extra calls to
StyleForLayoutObject().

Bug:  850664 ,  851384 
Change-Id: I975509d23f91b7ae7ea5b4545377436bb7514f0a
Reviewed-on: https://chromium-review.googlesource.com/1095096
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Anders Ruud <andruud@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566091}
[add] https://crrev.com/864a1050facb4d6cbb159404ebac88d6b18f0bfb/third_party/WebKit/LayoutTests/html/dialog/showmodal-in-shadow-crash.html
[modify] https://crrev.com/864a1050facb4d6cbb159404ebac88d6b18f0bfb/third_party/blink/renderer/core/dom/element.cc
[modify] https://crrev.com/864a1050facb4d6cbb159404ebac88d6b18f0bfb/third_party/blink/renderer/core/dom/node.cc
[modify] https://crrev.com/864a1050facb4d6cbb159404ebac88d6b18f0bfb/third_party/blink/renderer/core/dom/node.h

Status: Fixed (was: Started)

Sign in to add a comment