Changeset 228724 in webkit

Timestamp:

Feb 19, 2018 7:21:53 PM (6 years ago)

Author:

commit-queue@webkit.org

Message:

null m_lastNodeInserted dereference at ReplaceSelectionCommand::InsertedNodes::lastLeafInserted
​https://bugs.webkit.org/show_bug.cgi?id=161947

Patch by Fujii Hironori <Fujii Hironori> on 2018-02-19
Reviewed by Ryosuke Niwa.

Source/WebCore:

InsertedNodes happened to be empty if the inserted nodes were
removed. Add more checks if InsertedNodes is empty.

No new tests (Covered by existing tests).

• editing/ReplaceSelectionCommand.cpp:

(WebCore::ReplaceSelectionCommand::doApply): Return early if InsertedNodes becomes empty.

• editing/ReplaceSelectionCommand.h:

(WebCore::ReplaceSelectionCommand::InsertedNodes::isEmpty): New method.
(WebCore::ReplaceSelectionCommand::InsertedNodes::lastLeafInserted const):
Assert m_lastNodeInserted is not null.
(WebCore::ReplaceSelectionCommand::InsertedNodes::pastLastLeaf const): Ditto.

LayoutTests:

• platform/gtk/TestExpectations:

Unmarked editing/execCommand/crash-replacing-list-by-list.html and editing/inserting/insert-table-in-paragraph-crash.html.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/gtk/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/editing/ReplaceSelectionCommand.cpp (modified) (3 diffs)
• Source/WebCore/editing/ReplaceSelectionCommand.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2018-02-19  Fujii Hironori  <Hironori.Fujii@sony.com>
+
+        null m_lastNodeInserted dereference at ReplaceSelectionCommand::InsertedNodes::lastLeafInserted
+        https://bugs.webkit.org/show_bug.cgi?id=161947
+
+        Reviewed by Ryosuke Niwa.
+
+        * platform/gtk/TestExpectations:
+        Unmarked editing/execCommand/crash-replacing-list-by-list.html and editing/inserting/insert-table-in-paragraph-crash.html.
+
 2018-02-19  Antoine Quint  <graouts@apple.com>
 

trunk/LayoutTests/platform/gtk/TestExpectations

@@ -1301 +1301 @@
 webkit.org/b/172281 accessibility/insert-children-assert.html [ Crash ]
 
-webkit.org/b/172951 editing/execCommand/crash-replacing-list-by-list.html [ Crash ]
-webkit.org/b/172951 editing/inserting/insert-table-in-paragraph-crash.html [ Crash ]
-
 webkit.org/b/172955 media/video-preload.html [ Crash Pass ]
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2018-02-19  Fujii Hironori  <Hironori.Fujii@sony.com>
+
+        null m_lastNodeInserted dereference at ReplaceSelectionCommand::InsertedNodes::lastLeafInserted
+        https://bugs.webkit.org/show_bug.cgi?id=161947
+
+        Reviewed by Ryosuke Niwa.
+
+        InsertedNodes happened to be empty if the inserted nodes were
+        removed. Add more checks if InsertedNodes is empty.
+
+        No new tests (Covered by existing tests).
+
+        * editing/ReplaceSelectionCommand.cpp:
+        (WebCore::ReplaceSelectionCommand::doApply): Return early if InsertedNodes becomes empty.
+        * editing/ReplaceSelectionCommand.h:
+        (WebCore::ReplaceSelectionCommand::InsertedNodes::isEmpty): New method.
+        (WebCore::ReplaceSelectionCommand::InsertedNodes::lastLeafInserted const):
+        Assert m_lastNodeInserted is not null.
+        (WebCore::ReplaceSelectionCommand::InsertedNodes::pastLastLeaf const): Ditto.
+
 2018-02-19  Said Abou-Hallawa  <sabouhallawa@apple.com>
 

trunk/Source/WebCore/editing/ReplaceSelectionCommand.cpp

@@ -1143 +1143 @@
     }
 
+    if (insertedNodes.isEmpty())
+        return;
     removeUnrenderedTextNodesAtEnds(insertedNodes);
 
@@ -1149 +1151 @@
 
     // Mutation events (bug 20161) may have already removed the inserted content
-    if (!insertedNodes.firstNodeInserted() || !insertedNodes.firstNodeInserted()->isConnected())
+    if (insertedNodes.isEmpty())
+        return;
+    if (!insertedNodes.firstNodeInserted()->isConnected())
         return;
 
@@ -1170 +1174 @@
     
     makeInsertedContentRoundTrippableWithHTMLTreeBuilder(insertedNodes);
+    if (insertedNodes.isEmpty())
+        return;
 
     removeRedundantStylesAndKeepStyleSpanInline(insertedNodes);
+    if (insertedNodes.isEmpty())
+        return;
 
     if (m_sanitizeFragment)

trunk/Source/WebCore/editing/ReplaceSelectionCommand.h

@@ -70 +70 @@
         void didReplaceNode(Node*, Node* newNode);
 
+        bool isEmpty() { return !m_firstNodeInserted; }
         Node* firstNodeInserted() const { return m_firstNodeInserted.get(); }
-        Node* lastLeafInserted() const { return m_lastNodeInserted->lastDescendant(); }
+        Node* lastLeafInserted() const
+        {
+            ASSERT(m_lastNodeInserted);
+            return m_lastNodeInserted->lastDescendant();
+        }
         Node* pastLastLeaf() const
         {
-            if (m_lastNodeInserted) {
-                ASSERT(lastLeafInserted());
-                return NodeTraversal::next(*lastLeafInserted());
-            }
-            return nullptr;
+            ASSERT(m_lastNodeInserted);
+            return NodeTraversal::next(*lastLeafInserted());
         }