Bug 149365 - Inserting or removing slot elements can cause a crash
Summary: Inserting or removing slot elements can cause a crash
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords:
Depends on:
Blocks: 149330
  Show dependency treegraph
 
Reported: 2015-09-18 19:24 PDT by Ryosuke Niwa
Modified: 2015-09-18 21:18 PDT (History)
6 users (show)

See Also:


Attachments
Fixes the bug (8.78 KB, patch)
2015-09-18 19:46 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Patch for landing (11.05 KB, patch)
2015-09-18 20:30 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Patch for landing (11.27 KB, patch)
2015-09-18 20:33 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.