Title: [282555] releases/WebKitGTK/webkit-2.32
Revision
282555
Author
[email protected]
Date
2021-09-16 04:42:49 -0700 (Thu, 16 Sep 2021)

Log Message

Merge r280174 - nullptr crash in ApplyStyleCommand::applyRelativeFontStyleChange
https://bugs.webkit.org/show_bug.cgi?id=223974

Patch by Frédéric Wang <[email protected]> on 2021-07-22
Reviewed by Darin Adler.

Source/WebCore:

WebCore::documentOrder does not handle well elements like <summary> that contains a
shadow substree. This is causing assertion failures in debug build when setting start/end
selection and nullptr crashes in release build when trying to browse selection between these
start and end nodes. This patch fixes that issue by switching to shadow including tree order
for these particular cases. It introduces a generic treeOrder<TreeType>(a, b) function that
can be used for TreeType = ShadowIncludingTree as well as by WebCore::documentOrder.

* dom/Node.cpp: Explicitly instantiate commonInclusiveAncestor<ShadowIncludingTree> so that it can be used
by WebCore::treeOrder.
* dom/Position.cpp: Explicitly instantiate templates.
(WebCore::treeOrder): Convert documentOrder to a template parametrized by TreeType.
(WebCore::documentOrder): Implement it with treeOrder<ComposedTree>.
* dom/Position.h: Delcare new template.
* editing/ApplyStyleCommand.cpp:
(WebCore::ApplyStyleCommand::updateStartEnd): Use treeOrder<ShadowIncludingTree>.
(WebCore::ApplyStyleCommand::removeInlineStyle): Ditto.
* editing/VisiblePosition.cpp:
(WebCore::documentOrder): Use treeOrder<ShadowIncludingTree>.
* editing/VisibleSelection.cpp:
(WebCore::VisibleSelection::setBaseAndExtentToDeepEquivalents): Use treeOrder<ShadowIncludingTree>.
(WebCore::VisibleSelection::setWithoutValidation): Ditto.

Tools:

* TestWebKitAPI/Tests/WebCore/DocumentOrder.cpp: Update FIXME.

Modified Paths

Diff

Modified: releases/WebKitGTK/webkit-2.32/Source/WebCore/ChangeLog (282554 => 282555)


--- releases/WebKitGTK/webkit-2.32/Source/WebCore/ChangeLog	2021-09-16 11:42:39 UTC (rev 282554)
+++ releases/WebKitGTK/webkit-2.32/Source/WebCore/ChangeLog	2021-09-16 11:42:49 UTC (rev 282555)
@@ -1,3 +1,32 @@
+2021-07-22  Frédéric Wang  <[email protected]>
+
+        nullptr crash in ApplyStyleCommand::applyRelativeFontStyleChange
+        https://bugs.webkit.org/show_bug.cgi?id=223974
+
+        Reviewed by Darin Adler.
+
+        WebCore::documentOrder does not handle well elements like <summary> that contains a
+        shadow substree. This is causing assertion failures in debug build when setting start/end
+        selection and nullptr crashes in release build when trying to browse selection between these
+        start and end nodes. This patch fixes that issue by switching to shadow including tree order
+        for these particular cases. It introduces a generic treeOrder<TreeType>(a, b) function that
+        can be used for TreeType = ShadowIncludingTree as well as by WebCore::documentOrder.
+
+        * dom/Node.cpp: Explicitly instantiate commonInclusiveAncestor<ShadowIncludingTree> so that it can be used
+        by WebCore::treeOrder.
+        * dom/Position.cpp: Explicitly instantiate templates.
+        (WebCore::treeOrder): Convert documentOrder to a template parametrized by TreeType.
+        (WebCore::documentOrder): Implement it with treeOrder<ComposedTree>.
+        * dom/Position.h: Delcare new template.
+        * editing/ApplyStyleCommand.cpp:
+        (WebCore::ApplyStyleCommand::updateStartEnd): Use treeOrder<ShadowIncludingTree>.
+        (WebCore::ApplyStyleCommand::removeInlineStyle): Ditto.
+        * editing/VisiblePosition.cpp:
+        (WebCore::documentOrder): Use treeOrder<ShadowIncludingTree>.
+        * editing/VisibleSelection.cpp:
+        (WebCore::VisibleSelection::setBaseAndExtentToDeepEquivalents): Use treeOrder<ShadowIncludingTree>.
+        (WebCore::VisibleSelection::setWithoutValidation): Ditto.
+
 2021-07-01  Sergio Villar Senin  <[email protected]>
 
         Missing layouts when using simplified layout with OOF positioned elements

Modified: releases/WebKitGTK/webkit-2.32/Source/WebCore/dom/Node.cpp (282554 => 282555)


--- releases/WebKitGTK/webkit-2.32/Source/WebCore/dom/Node.cpp	2021-09-16 11:42:39 UTC (rev 282554)
+++ releases/WebKitGTK/webkit-2.32/Source/WebCore/dom/Node.cpp	2021-09-16 11:42:49 UTC (rev 282555)
@@ -2676,6 +2676,7 @@
 
 template Node* commonInclusiveAncestor<Tree>(const Node&, const Node&);
 template Node* commonInclusiveAncestor<ComposedTree>(const Node&, const Node&);
+template Node* commonInclusiveAncestor<ShadowIncludingTree>(const Node&, const Node&);
 
 static bool isSiblingSubsequent(const Node& siblingA, const Node& siblingB)
 {

Modified: releases/WebKitGTK/webkit-2.32/Source/WebCore/dom/Position.cpp (282554 => 282555)


--- releases/WebKitGTK/webkit-2.32/Source/WebCore/dom/Position.cpp	2021-09-16 11:42:39 UTC (rev 282554)
+++ releases/WebKitGTK/webkit-2.32/Source/WebCore/dom/Position.cpp	2021-09-16 11:42:49 UTC (rev 282555)
@@ -1600,7 +1600,7 @@
     return BoundaryPoint { container.releaseNonNull(), static_cast<unsigned>(position.computeOffsetInContainerNode()) };
 }
 
-PartialOrdering documentOrder(const Position& a, const Position& b)
+template<TreeType treeType> PartialOrdering treeOrder(const Position& a, const Position& b)
 {
     if (a.isNull() || b.isNull())
         return a.isNull() && b.isNull() ? PartialOrdering::equivalent : PartialOrdering::unordered;
@@ -1609,7 +1609,7 @@
     auto bContainer = b.containerNode();
 
     if (!aContainer || !bContainer) {
-        if (!commonInclusiveAncestor<ComposedTree>(*a.anchorNode(), *b.anchorNode()))
+        if (!commonInclusiveAncestor<treeType>(*a.anchorNode(), *b.anchorNode()))
             return PartialOrdering::unordered;
         if (!aContainer && !bContainer && a.anchorType() == b.anchorType())
             return PartialOrdering::equivalent;
@@ -1620,9 +1620,17 @@
 
     // FIXME: Avoid computing node offset for cases where we don't need to.
 
-    return treeOrder<ComposedTree>(*makeBoundaryPoint(a), *makeBoundaryPoint(b));
+    return treeOrder<treeType>(*makeBoundaryPoint(a), *makeBoundaryPoint(b));
 }
 
+PartialOrdering documentOrder(const Position& a, const Position& b)
+{
+    return treeOrder<ComposedTree>(a, b);
+}
+
+template PartialOrdering treeOrder<ComposedTree>(const Position&, const Position&);
+template PartialOrdering treeOrder<ShadowIncludingTree>(const Position&, const Position&);
+
 } // namespace WebCore
 
 #if ENABLE(TREE_DEBUGGING)

Modified: releases/WebKitGTK/webkit-2.32/Source/WebCore/dom/Position.h (282554 => 282555)


--- releases/WebKitGTK/webkit-2.32/Source/WebCore/dom/Position.h	2021-09-16 11:42:39 UTC (rev 282554)
+++ releases/WebKitGTK/webkit-2.32/Source/WebCore/dom/Position.h	2021-09-16 11:42:49 UTC (rev 282555)
@@ -219,6 +219,7 @@
 bool operator==(const Position&, const Position&);
 bool operator!=(const Position&, const Position&);
 
+template<TreeType treeType> PartialOrdering treeOrder(const Position&, const Position&);
 WEBCORE_EXPORT PartialOrdering documentOrder(const Position&, const Position&);
 bool operator<(const Position&, const Position&);
 bool operator>(const Position&, const Position&);

Modified: releases/WebKitGTK/webkit-2.32/Source/WebCore/editing/ApplyStyleCommand.cpp (282554 => 282555)


--- releases/WebKitGTK/webkit-2.32/Source/WebCore/editing/ApplyStyleCommand.cpp	2021-09-16 11:42:39 UTC (rev 282554)
+++ releases/WebKitGTK/webkit-2.32/Source/WebCore/editing/ApplyStyleCommand.cpp	2021-09-16 11:42:49 UTC (rev 282555)
@@ -172,7 +172,7 @@
 
 void ApplyStyleCommand::updateStartEnd(const Position& newStart, const Position& newEnd)
 {
-    ASSERT(!(newStart > newEnd));
+    ASSERT(!is_gt(treeOrder<ShadowIncludingTree>(newStart, newEnd)));
 
     if (!m_useEndingSelection && (newStart != m_start || newEnd != m_end))
         m_useEndingSelection = true;
@@ -1068,7 +1068,7 @@
     ASSERT(end.isNotNull());
     ASSERT(start.anchorNode()->isConnected());
     ASSERT(end.anchorNode()->isConnected());
-    ASSERT(start <= end);
+    ASSERT(is_lteq(treeOrder<ShadowIncludingTree>(start, end)));
     // FIXME: We should assert that start/end are not in the middle of a text node.
 
     Position pushDownStart = start.downstream();

Modified: releases/WebKitGTK/webkit-2.32/Source/WebCore/editing/VisiblePosition.cpp (282554 => 282555)


--- releases/WebKitGTK/webkit-2.32/Source/WebCore/editing/VisiblePosition.cpp	2021-09-16 11:42:39 UTC (rev 282554)
+++ releases/WebKitGTK/webkit-2.32/Source/WebCore/editing/VisiblePosition.cpp	2021-09-16 11:42:49 UTC (rev 282555)
@@ -800,7 +800,7 @@
 PartialOrdering documentOrder(const VisiblePosition& a, const VisiblePosition& b)
 {
     // FIXME: Should two positions with different affinity be considered equivalent or not?
-    return documentOrder(a.deepEquivalent(), b.deepEquivalent());
+    return treeOrder<ComposedTree>(a.deepEquivalent(), b.deepEquivalent());
 }
 
 bool intersects(const VisiblePositionRange& a, const VisiblePositionRange& b)

Modified: releases/WebKitGTK/webkit-2.32/Source/WebCore/editing/VisibleSelection.cpp (282554 => 282555)


--- releases/WebKitGTK/webkit-2.32/Source/WebCore/editing/VisibleSelection.cpp	2021-09-16 11:42:39 UTC (rev 282554)
+++ releases/WebKitGTK/webkit-2.32/Source/WebCore/editing/VisibleSelection.cpp	2021-09-16 11:42:49 UTC (rev 282555)
@@ -251,7 +251,7 @@
     if (m_focus.isNull())
         m_focus = m_anchor;
 
-    m_anchorIsFirst = m_anchor <= m_focus;
+    m_anchorIsFirst = is_lteq(treeOrder<ShadowIncludingTree>(m_anchor, m_focus));
 
     m_base = VisiblePosition(m_anchor, m_affinity).deepEquivalent();
     if (m_anchor == m_focus)
@@ -461,7 +461,7 @@
     ASSERT(m_affinity == Affinity::Downstream);
     m_anchor = anchor;
     m_focus = focus;
-    m_anchorIsFirst = m_anchor <= m_focus;
+    m_anchorIsFirst = is_lteq(treeOrder<ShadowIncludingTree>(m_anchor, m_focus));
     m_base = anchor;
     m_extent = focus;
     m_start = m_anchorIsFirst ? anchor : focus;

Modified: releases/WebKitGTK/webkit-2.32/Tools/ChangeLog (282554 => 282555)


--- releases/WebKitGTK/webkit-2.32/Tools/ChangeLog	2021-09-16 11:42:39 UTC (rev 282554)
+++ releases/WebKitGTK/webkit-2.32/Tools/ChangeLog	2021-09-16 11:42:49 UTC (rev 282555)
@@ -1,3 +1,12 @@
+2021-07-22  Frédéric Wang  <[email protected]>
+
+        nullptr crash in ApplyStyleCommand::applyRelativeFontStyleChange
+        https://bugs.webkit.org/show_bug.cgi?id=223974
+
+        Reviewed by Darin Adler.
+
+        * TestWebKitAPI/Tests/WebCore/DocumentOrder.cpp: Update FIXME.
+
 2021-05-28  Alex Christensen  <[email protected]>
 
         Punycode encode U+0BE6 when not in context of other Tamil characters

Modified: releases/WebKitGTK/webkit-2.32/Tools/TestWebKitAPI/Tests/WebCore/DocumentOrder.cpp (282554 => 282555)


--- releases/WebKitGTK/webkit-2.32/Tools/TestWebKitAPI/Tests/WebCore/DocumentOrder.cpp	2021-09-16 11:42:39 UTC (rev 282554)
+++ releases/WebKitGTK/webkit-2.32/Tools/TestWebKitAPI/Tests/WebCore/DocumentOrder.cpp	2021-09-16 11:42:49 UTC (rev 282555)
@@ -36,7 +36,7 @@
 #include <WebCore/SimpleRange.h>
 #include <WebCore/TextControlInnerElements.h>
 
-// FIXME: Expose the functions tested here in WebKit internals object, then replace this test with one written in _javascript_.
+// FIXME(https://webkit.org/b/228175): Expose the functions tested here in WebKit internals object, then replace this test with one written in _javascript_.
 // FIXME: When doing the above, don't forget to remove the many WEBCORE_EXPORT that were added so we could compile and link this test.
 
 #define EXPECT_BOTH(a, b, forward, reversed) do { EXPECT_STREQ(string(documentOrder(a, b)), forward); EXPECT_STREQ(string(documentOrder(b, a)), reversed); } while (0)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to