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

Issue 637003 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

Crash: A Shadow host with a pseudo element (the shadow tree has a slot)

Project Member Reported by egarciad@chromium.org, Aug 11 2016

Issue description

Chrome Version: Version 54.0.2826.0 canary (64-bit)
OS: OSX El Capitan V10.11.6 (15G31)
Crash ID: 4030e6f9-b1b4-4863-a370-46eadd36c2f9 (Server ID: b96b1c4100000000)

Can you reproduce this crash?
Yes. Repro: http://jsbin.com/kanukug/edit?html,output

What steps will reproduce this crash (or if it's not reproducible,
what were you doing just before the crash)?
The following code snippet:

<script>
  document.registerElement('x-foo', class extends HTMLElement {
    createdCallback() {
      this.attachShadow({mode: 'open'}).innerHTML = `
      <style>
        :host {
          display: block;
        }
        :host::after {
          content: '';
        }
      </style>
      <div>
        <slot></slot>
      </div>
      `;
    }         
  });
</script>

<x-foo>Foo</x-foo>

Key findings:
1. :host's display must be != inline
2. :host::after rule + content: '';
3. <slot> must be inside a container. (a div in this example)


 
Labels: -Restrict-View-EditIssue
Summary: Using CE and Shadow DOM v1 causes Canary to crash (was: Using CE v0 and Shadow DOM v1 causes Canary to crash)
I can also repro the issue using CE V1 in Canary with `--enable-blink-features=CustomElementsV1`

Repro: http://output.jsbin.com/qulosaq
Source:
<script>
  customElements.define('x-foo', class extends HTMLElement {
    connectedCallback() {
       this.attachShadow({mode: 'open'}).innerHTML = `
        <style>
          :host {
            display: block;
          }
          :host::after {
            content: '';
          }
        </style>
        <div>
          <slot></slot>
        </div>
      `;
    }       
  });
</script>

<x-foo>Foo</x-foo>
Components: -Blink Blink>Layout
crash/c4c6e14100000000


	0x00007f958ca37932	(chrome -LayoutBlock.cpp:257 )	blink::LayoutBlock::addChildBeforeDescendant
0x00007f958a8e9ae7	(chrome -LayoutTreeBuilder.h:76 )	blink::Element::attach
0x00007f958c598bd8	(chrome -PseudoElement.cpp:121 )	blink::PseudoElement::attach
0x00007f958a8e9c8f	(chrome -Element.cpp:2938 )	blink::Element::attach
0x00007f958a8e3bff	(chrome -ContainerNode.cpp:755 )	blink::ContainerNode::attach
0x00007f958a8e983b	(chrome -Element.cpp:1560 )	blink::Element::attach
0x00007f958a8e3bff	(chrome -ContainerNode.cpp:755 )	blink::ContainerNode::attach
0x00007f958a8e983b	(chrome -Element.cpp:1560 )	blink::Element::attach
0x00007f958c58b173	(chrome -Node.cpp:920 )	blink::Node::reattach
0x00007f958a8eab38	(chrome -Element.cpp:1775 )	blink::Element::recalcStyle
0x00007f958c556e5d	(chrome -Document.cpp:1790 )	blink::Document::updateStyle
0x00007f958a8e5997	(chrome -Document.cpp:1725 )	blink::Document::updateStyleAndLayoutTree
0x00007f958c5649af	(chrome -Document.cpp:4805 )	blink::Document::finishedParsing
0x00007f958c68d5d5	(chrome -HTMLDocumentParser.cpp:819 )	blink::HTMLDocumentParser::end
...

Comment 4 by e...@chromium.org, Aug 12 2016

Components: -Blink>Layout Blink>DOM
Over to DOM team as a proxy for ShadowDOM.

Comment 5 by hayato@chromium.org, Aug 15 2016

Components: Blink>WebComponents
Cc: -hayato@chromium.org dominicc@chromium.org
Owner: hayato@chromium.org
Status: Assigned (was: Untriaged)
Thanks for filing this. Would be interesting to see if this can be reproed with a script tag or if there's something special about custom elements.

Comment 7 by hayato@chromium.org, Aug 17 2016

Cc: kochi@chromium.org r...@opera.com
I am not sure how we are supporting a selector like ":host::pseudo-element" well.

@rune, do you have any thought?

Comment 8 by r...@opera.com, Aug 17 2016

I think it's fine with :host::pseudo-element, but regardless, you have the same crash here with the pseudo element defined in the document style as well. Somewhat minimized case:

<!DOCTYPE html>
<style>
    #foo::after {
        content: '';
    }
</style>
<div id="foo">Foo</div>
<script>
    foo.attachShadow({mode: 'open'}).innerHTML = '<span><slot></slot> ';
</script>

Comment 9 by r...@opera.com, Aug 17 2016

The bug is in the layout/flat traversal somewhere in this code:

https://codesearch.chromium.org/chromium/src/third_party/WebKit/Source/core/dom/shadow/FlatTreeTraversal.cpp?l=99-104

The layout tree traversal for the ::after pseudo element returns the whitespace text node after the slot element as the next sibling for the after element. That is wrong and causes the layout code to assert because we are inserting the layout object for the ::after pseudo before the layout object for the whitespace text-node which we presumably already created and inserted as part of generating the layout boxes for nodes distributed to the slot.

Thanks. Oh, pseudo elements are always a headache to me. :(
Let me fix that anyway.
Status: Started (was: Assigned)
Labels: -Pri-3 Pri-2
Project Member

Comment 13 by bugdroid1@chromium.org, Aug 19 2016

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

commit 66b2cb7e218c15404eced5cab488718b3a0e537d
Author: hayato <hayato@chromium.org>
Date: Fri Aug 19 06:58:38 2016

Fix a crash when pseudo elements are used for a shadow host

LayoutTreeBuilderTraversal should not pass a before/after pseudo element to
FlatTreeTraversal because FlatTreeTraversal can not handle it correctly.

Because there are still other clients which pass a pseudo element to
FlatTreTraversal, we can not update Node::canParticipateInFlatTree()'s condition,
which is used in FlatTreeTraversal::assertPreCondition(). That would be done as
another task.

BUG= 637003 

Review-Url: https://codereview.chromium.org/2257053003
Cr-Commit-Position: refs/heads/master@{#413072}

[add] https://crrev.com/66b2cb7e218c15404eced5cab488718b3a0e537d/third_party/WebKit/LayoutTests/shadow-dom/host-pseudo-elements-expected.html
[add] https://crrev.com/66b2cb7e218c15404eced5cab488718b3a0e537d/third_party/WebKit/LayoutTests/shadow-dom/host-pseudo-elements.html
[modify] https://crrev.com/66b2cb7e218c15404eced5cab488718b3a0e537d/third_party/WebKit/Source/core/dom/LayoutTreeBuilderTraversal.cpp
[modify] https://crrev.com/66b2cb7e218c15404eced5cab488718b3a0e537d/third_party/WebKit/Source/core/dom/Node.cpp

Summary: Crash: A Shadow host with a pseudo element (the shadow tree has a slot) (was: Using CE and Shadow DOM v1 causes Canary to crash)
Status: Fixed (was: Started)

Sign in to add a comment