Bug 149365

Summary: Inserting or removing slot elements can cause a crash
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: DOMAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarcelo, commit-queue, esprehn+autocc, gyuyoung.kim, kangil.han, koivisto
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 149330    
Attachments:
Description Flags
Fixes the bug
none
Patch for landing
none
Patch for landing none

Description Ryosuke Niwa 2015-09-18 19:24:44 PDT
HTMLSlotElement::insertedInto and HTMLSlotElement::removedFrom are completely wrong.
Comment 1 Ryosuke Niwa 2015-09-18 19:46:30 PDT
Created attachment 261561 [details]
Fixes the bug
Comment 2 Antti Koivisto 2015-09-18 20:08:46 PDT
Comment on attachment 261561 [details]
Fixes the bug

View in context: https://bugs.webkit.org/attachment.cgi?id=261561&action=review

> Source/WebCore/html/HTMLSlotElement.cpp:52
> +    if (insertionPoint.isInShadowTree() && isInShadowTree() && &insertionPoint.treeScope() == &treeScope()) {

This could use a comment.

> Source/WebCore/html/HTMLSlotElement.cpp:53
> +        if (auto shadowRoot = containingShadowRoot())

auto*

> Source/WebCore/html/HTMLSlotElement.cpp:63
>      // Can't call containingShadowRoot() here since this node has already been disconnected from the parent.
> -    if (isInShadowTree()) {
> -        auto& oldShadowRoot = downcast<ShadowRoot>(insertionPoint.treeScope().rootNode());
> -        oldShadowRoot.removeSlotElementByName(fastGetAttribute(nameAttr), *this);
> +    if (insertionPoint.isInShadowTree() && isInShadowTree() && &treeScope() == &document()) {

This could use a bit more comprehensive comment.
Comment 3 Ryosuke Niwa 2015-09-18 20:30:16 PDT
Created attachment 261563 [details]
Patch for landing
Comment 4 WebKit Commit Bot 2015-09-18 20:32:04 PDT
Comment on attachment 261563 [details]
Patch for landing

Rejecting attachment 261563 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 261563, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in LayoutTests/ChangeLog contains OOPS!.

Full output: http://webkit-queues.webkit.org/results/185244
Comment 5 Ryosuke Niwa 2015-09-18 20:33:21 PDT
Created attachment 261564 [details]
Patch for landing
Comment 6 WebKit Commit Bot 2015-09-18 21:18:44 PDT
Comment on attachment 261564 [details]
Patch for landing

Clearing flags on attachment: 261564

Committed r190008: <http://trac.webkit.org/changeset/190008>
Comment 7 WebKit Commit Bot 2015-09-18 21:18:49 PDT
All reviewed patches have been landed.  Closing bug.