From 95cb7c72832b4f4d4cc41ba477baf9b9be7216a4 Mon Sep 17 00:00:00 2001 From: "shinyak@chromium.org" Date: Mon, 25 Jun 2012 16:59:25 +0000 Subject: [PATCH] [Shadow] Executing Italic and InsertUnorderedList in Shadow DOM causes a crash https://bugs.webkit.org/show_bug.cgi?id=88495 Reviewed by Ryosuke Niwa. Source/WebCore: InsertionPoint::removedFrom(insertionPoint) tries to find its owner ElementShadow from parentNode or insertionPoint. If the parent node exsits but we cannot reach ElementShadow from the parent node, InsertionPoint::removedFrom does not try to find ElementShadow anymore. It's OK if the ElementShadow is being destructed, but there is a case ElementShadow is not being destructed in editing. In this case, we should try to find ElementShadow from insertionPoint. Otherwise it will bring inconsistency to Shadow DOM, and causes a crash. Actually checking the existence of parentNode() does not make any sense. We should get shadowRoot() directly. Test: editing/shadow/insertorderedlist-crash.html * html/shadow/InsertionPoint.cpp: (WebCore::InsertionPoint::removedFrom): LayoutTests: * editing/shadow/insertorderedlist-crash-expected.txt: Added. * editing/shadow/insertorderedlist-crash.html: Added. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@121162 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- LayoutTests/ChangeLog | 10 +++++ .../shadow/insertorderedlist-crash-expected.txt | 3 ++ .../editing/shadow/insertorderedlist-crash.html | 50 ++++++++++++++++++++++ Source/WebCore/ChangeLog | 23 ++++++++++ Source/WebCore/html/shadow/InsertionPoint.cpp | 15 +++---- 5 files changed, 93 insertions(+), 8 deletions(-) create mode 100644 LayoutTests/editing/shadow/insertorderedlist-crash-expected.txt create mode 100644 LayoutTests/editing/shadow/insertorderedlist-crash.html diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index f114df2..72184b3 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,3 +1,13 @@ +2012-06-25 Shinya Kawanaka + + [Shadow] Executing Italic and InsertUnorderedList in Shadow DOM causes a crash + https://bugs.webkit.org/show_bug.cgi?id=88495 + + Reviewed by Ryosuke Niwa. + + * editing/shadow/insertorderedlist-crash-expected.txt: Added. + * editing/shadow/insertorderedlist-crash.html: Added. + 2012-06-25 Allan Sandfeld Jensen [Qt] Unskip 3D transform tests. diff --git a/LayoutTests/editing/shadow/insertorderedlist-crash-expected.txt b/LayoutTests/editing/shadow/insertorderedlist-crash-expected.txt new file mode 100644 index 0000000..d713963 --- /dev/null +++ b/LayoutTests/editing/shadow/insertorderedlist-crash-expected.txt @@ -0,0 +1,3 @@ +This test confirms some combination of editing command with Shadow DOM does not cause a crash. To test manually, select from (before nested) to (after nested), then press Italic, and InsertUnorderedList. + +PASS diff --git a/LayoutTests/editing/shadow/insertorderedlist-crash.html b/LayoutTests/editing/shadow/insertorderedlist-crash.html new file mode 100644 index 0000000..31afc57 --- /dev/null +++ b/LayoutTests/editing/shadow/insertorderedlist-crash.html @@ -0,0 +1,50 @@ + + + + + +

This test confirms some combination of editing command with Shadow DOM does not cause a crash. +To test manually, select from (before nested) to (after nested), then press Italic, and InsertUnorderedList.

+ +

(before host)

not editable

(after host)

+ + + +

+ + + + + + diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index e7bd03f..af7c740 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,26 @@ +2012-06-25 Shinya Kawanaka + + [Shadow] Executing Italic and InsertUnorderedList in Shadow DOM causes a crash + https://bugs.webkit.org/show_bug.cgi?id=88495 + + Reviewed by Ryosuke Niwa. + + InsertionPoint::removedFrom(insertionPoint) tries to find its owner ElementShadow from + parentNode or insertionPoint. If the parent node exsits but we cannot reach ElementShadow from + the parent node, InsertionPoint::removedFrom does not try to find ElementShadow anymore. + + It's OK if the ElementShadow is being destructed, but there is a case ElementShadow is not being + destructed in editing. In this case, we should try to find ElementShadow from insertionPoint. + Otherwise it will bring inconsistency to Shadow DOM, and causes a crash. + + Actually checking the existence of parentNode() does not make any sense. We should get + shadowRoot() directly. + + Test: editing/shadow/insertorderedlist-crash.html + + * html/shadow/InsertionPoint.cpp: + (WebCore::InsertionPoint::removedFrom): + 2012-06-25 Kinuko Yasuda Remove responseBlob field from XMLHttpResponse.idl diff --git a/Source/WebCore/html/shadow/InsertionPoint.cpp b/Source/WebCore/html/shadow/InsertionPoint.cpp index 5805128..0794c34 100644 --- a/Source/WebCore/html/shadow/InsertionPoint.cpp +++ b/Source/WebCore/html/shadow/InsertionPoint.cpp @@ -128,14 +128,13 @@ Node::InsertionNotificationRequest InsertionPoint::insertedInto(ContainerNode* i void InsertionPoint::removedFrom(ContainerNode* insertionPoint) { if (insertionPoint->inDocument()) { - Node* parent = parentNode(); - if (!parent) - parent = insertionPoint; - if (ShadowRoot* root = parent->shadowRoot()) { - // host can be null when removedFrom() is called from ElementShadow destructor. - if (root->host()) - root->owner()->invalidateDistribution(); - } + ShadowRoot* root = shadowRoot(); + if (!root) + root = insertionPoint->shadowRoot(); + + // host can be null when removedFrom() is called from ElementShadow destructor. + if (root && root->host()) + root->owner()->invalidateDistribution(); // Since this insertion point is no longer visible from the shadow subtree, it need to clean itself up. clearDistribution(); -- 2.7.4