Title: [228724] trunk
Revision
228724
Author
commit-qu...@webkit.org
Date
2018-02-19 19:21:53 -0800 (Mon, 19 Feb 2018)

Log Message

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

Patch by Fujii Hironori <hironori.fu...@sony.com> 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.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (228723 => 228724)


--- trunk/LayoutTests/ChangeLog	2018-02-20 02:12:04 UTC (rev 228723)
+++ trunk/LayoutTests/ChangeLog	2018-02-20 03:21:53 UTC (rev 228724)
@@ -1,3 +1,13 @@
+2018-02-19  Fujii Hironori  <hironori.fu...@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  <grao...@apple.com>
 
         [Modern Media Controls] Implement a compact set of media controls

Modified: trunk/LayoutTests/platform/gtk/TestExpectations (228723 => 228724)


--- trunk/LayoutTests/platform/gtk/TestExpectations	2018-02-20 02:12:04 UTC (rev 228723)
+++ trunk/LayoutTests/platform/gtk/TestExpectations	2018-02-20 03:21:53 UTC (rev 228724)
@@ -1300,9 +1300,6 @@
 
 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 ]
 
 webkit.org/b/175575 imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/ready-states/autoplay-with-slow-text-tracks.html [ Crash Pass ]

Modified: trunk/Source/WebCore/ChangeLog (228723 => 228724)


--- trunk/Source/WebCore/ChangeLog	2018-02-20 02:12:04 UTC (rev 228723)
+++ trunk/Source/WebCore/ChangeLog	2018-02-20 03:21:53 UTC (rev 228724)
@@ -1,3 +1,23 @@
+2018-02-19  Fujii Hironori  <hironori.fu...@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  <sabouhall...@apple.com>
 
         Add SVGPropertyTraits::fromString() to all the SVG animated types

Modified: trunk/Source/WebCore/editing/ReplaceSelectionCommand.cpp (228723 => 228724)


--- trunk/Source/WebCore/editing/ReplaceSelectionCommand.cpp	2018-02-20 02:12:04 UTC (rev 228723)
+++ trunk/Source/WebCore/editing/ReplaceSelectionCommand.cpp	2018-02-20 03:21:53 UTC (rev 228724)
@@ -1142,6 +1142,8 @@
         node = next;
     }
 
+    if (insertedNodes.isEmpty())
+        return;
     removeUnrenderedTextNodesAtEnds(insertedNodes);
 
     if (!handledStyleSpans)
@@ -1148,8 +1150,10 @@
         handleStyleSpans(insertedNodes);
 
     // 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;
 
     VisiblePosition startOfInsertedContent = firstPositionInOrBeforeNode(insertedNodes.firstNodeInserted());
 
@@ -1169,8 +1173,12 @@
     }
     
     makeInsertedContentRoundTrippableWithHTMLTreeBuilder(insertedNodes);
+    if (insertedNodes.isEmpty())
+        return;
 
     removeRedundantStylesAndKeepStyleSpanInline(insertedNodes);
+    if (insertedNodes.isEmpty())
+        return;
 
     if (m_sanitizeFragment)
         applyCommandToComposite(SimplifyMarkupCommand::create(document(), insertedNodes.firstNodeInserted(), insertedNodes.pastLastLeaf()));

Modified: trunk/Source/WebCore/editing/ReplaceSelectionCommand.h (228723 => 228724)


--- trunk/Source/WebCore/editing/ReplaceSelectionCommand.h	2018-02-20 02:12:04 UTC (rev 228723)
+++ trunk/Source/WebCore/editing/ReplaceSelectionCommand.h	2018-02-20 03:21:53 UTC (rev 228724)
@@ -69,15 +69,17 @@
         void willRemoveNode(Node*);
         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());
         }
 
     private:
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to