Title: [293197] trunk/Source/WebCore
Revision
293197
Author
cdu...@apple.com
Date
2022-04-21 17:01:22 -0700 (Thu, 21 Apr 2022)

Log Message

Simplify FrameSelection::textWasReplaced() and callers
https://bugs.webkit.org/show_bug.cgi?id=239620

Reviewed by Geoffrey Garen.

* dom/CharacterData.cpp:
(WebCore::CharacterData::setData):
- Cache document().frame()
- Call textWasReplaced() with *this instead of this
- Use modern template deduction for Ref<>.

(WebCore::CharacterData::setDataAndUpdate):
- Use WTFMove() for m_data to avoid ref-counting churn. We're about to overwrite
  m_data anyway.
- Use `else if` for the ProcessingInstruction branch instead of `if` to avoid this
  check in the common case where |this| is a Text node. A Text node cannot be a
  ProcessingInstruction node.
- Cache document().frame()
- Call textWasReplaced() with *this instead of this

* editing/FrameSelection.cpp:
(WebCore::updatePositionAfterAdoptingTextReplacement):
- Pass node by reference now that the caller now has a reference instead of a pointer
- Drop !position.anchorNode() check. It is not necessary as the position.anchorNode() != &node
  check would already be true if the anchorNode were null.

(WebCore::FrameSelection::textWasReplaced):
- Drop outdated comment about a fragment check since there is no such check anymore
- Pass node by reference instead of pointer as all call sites have a non-null pointer
- Drop null check for node that is no longer necessary

* editing/FrameSelection.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (293196 => 293197)


--- trunk/Source/WebCore/ChangeLog	2022-04-21 23:55:03 UTC (rev 293196)
+++ trunk/Source/WebCore/ChangeLog	2022-04-22 00:01:22 UTC (rev 293197)
@@ -1,5 +1,40 @@
 2022-04-21  Chris Dumez  <cdu...@apple.com>
 
+        Simplify FrameSelection::textWasReplaced() and callers
+        https://bugs.webkit.org/show_bug.cgi?id=239620
+
+        Reviewed by Geoffrey Garen.
+
+        * dom/CharacterData.cpp:
+        (WebCore::CharacterData::setData):
+        - Cache document().frame()
+        - Call textWasReplaced() with *this instead of this
+        - Use modern template deduction for Ref<>.
+
+        (WebCore::CharacterData::setDataAndUpdate):
+        - Use WTFMove() for m_data to avoid ref-counting churn. We're about to overwrite
+          m_data anyway.
+        - Use `else if` for the ProcessingInstruction branch instead of `if` to avoid this
+          check in the common case where |this| is a Text node. A Text node cannot be a
+          ProcessingInstruction node.
+        - Cache document().frame()
+        - Call textWasReplaced() with *this instead of this
+
+        * editing/FrameSelection.cpp:
+        (WebCore::updatePositionAfterAdoptingTextReplacement):
+        - Pass node by reference now that the caller now has a reference instead of a pointer
+        - Drop !position.anchorNode() check. It is not necessary as the position.anchorNode() != &node
+          check would already be true if the anchorNode were null.
+
+        (WebCore::FrameSelection::textWasReplaced):
+        - Drop outdated comment about a fragment check since there is no such check anymore
+        - Pass node by reference instead of pointer as all call sites have a non-null pointer
+        - Drop null check for node that is no longer necessary
+
+        * editing/FrameSelection.h:
+
+2022-04-21  Chris Dumez  <cdu...@apple.com>
+
         Adopt RobinHoodHashMap / RobinHoodHashSet more broadly in WebCore
         https://bugs.webkit.org/show_bug.cgi?id=239576
 

Modified: trunk/Source/WebCore/dom/CharacterData.cpp (293196 => 293197)


--- trunk/Source/WebCore/dom/CharacterData.cpp	2022-04-21 23:55:03 UTC (rev 293196)
+++ trunk/Source/WebCore/dom/CharacterData.cpp	2022-04-22 00:01:22 UTC (rev 293197)
@@ -56,13 +56,12 @@
 
     if (m_data == nonNullData && canUseSetDataOptimization(*this)) {
         document().textRemoved(*this, 0, oldLength);
-        if (document().frame())
-            document().frame()->selection().textWasReplaced(this, 0, oldLength, oldLength);
+        if (auto* frame = document().frame())
+            frame->selection().textWasReplaced(*this, 0, oldLength, oldLength);
         return;
     }
 
-    Ref<CharacterData> protectedThis(*this);
-
+    Ref protectedThis { *this };
     setDataAndUpdate(nonNullData, 0, oldLength, nonNullData.length());
 }
 
@@ -186,7 +185,7 @@
 {
     auto childChange = makeChildChange(*this, ContainerNode::ChildChange::Source::API);
 
-    String oldData = m_data;
+    String oldData = WTFMove(m_data);
     {
         std::optional<Style::ChildChangeInvalidation> styleInvalidation;
         if (auto* parent = parentNode())
@@ -203,12 +202,11 @@
     ASSERT(!renderer() || is<Text>(*this));
     if (auto text = dynamicDowncast<Text>(*this))
         text->updateRendererAfterContentChange(offsetOfReplacedData, oldLength);
-
-    if (auto processingIntruction = dynamicDowncast<ProcessingInstruction>(*this))
+    else if (auto processingIntruction = dynamicDowncast<ProcessingInstruction>(*this))
         processingIntruction->checkStyleSheet();
 
-    if (document().frame())
-        document().frame()->selection().textWasReplaced(this, offsetOfReplacedData, oldLength, newLength);
+    if (auto* frame = document().frame())
+        frame->selection().textWasReplaced(*this, offsetOfReplacedData, oldLength, newLength);
 
     notifyParentAfterChange(childChange);
 

Modified: trunk/Source/WebCore/editing/FrameSelection.cpp (293196 => 293197)


--- trunk/Source/WebCore/editing/FrameSelection.cpp	2022-04-21 23:55:03 UTC (rev 293196)
+++ trunk/Source/WebCore/editing/FrameSelection.cpp	2022-04-22 00:01:22 UTC (rev 293197)
@@ -631,9 +631,9 @@
         setSelection(VisibleSelection(), DoNotSetFocus);
 }
 
-static void updatePositionAfterAdoptingTextReplacement(Position& position, CharacterData* node, unsigned offset, unsigned oldLength, unsigned newLength)
+static void updatePositionAfterAdoptingTextReplacement(Position& position, CharacterData& node, unsigned offset, unsigned oldLength, unsigned newLength)
 {
-    if (!position.anchorNode() || position.anchorNode() != node || position.anchorType() != Position::PositionIsOffsetInAnchor)
+    if (position.anchorNode() != &node || position.anchorType() != Position::PositionIsOffsetInAnchor)
         return;
 
     // See: http://www.w3.org/TR/DOM-Level-2-Traversal-Range/ranges.html#Level-2-Range-Mutation
@@ -648,13 +648,12 @@
     if (positionOffset > offset + oldLength)
         position.moveToOffset(positionOffset - oldLength + newLength);
 
-    ASSERT(static_cast<unsigned>(position.offsetInContainerNode()) <= node->length());
+    ASSERT(static_cast<unsigned>(position.offsetInContainerNode()) <= node.length());
 }
 
-void FrameSelection::textWasReplaced(CharacterData* node, unsigned offset, unsigned oldLength, unsigned newLength)
+void FrameSelection::textWasReplaced(CharacterData& node, unsigned offset, unsigned oldLength, unsigned newLength)
 {
-    // The fragment check is a performance optimization. See http://trac.webkit.org/changeset/30062.
-    if (isNone() || !node || !node->isConnected())
+    if (isNone() || !node.isConnected())
         return;
 
     Position base = m_selection.base();

Modified: trunk/Source/WebCore/editing/FrameSelection.h (293196 => 293197)


--- trunk/Source/WebCore/editing/FrameSelection.h	2022-04-21 23:55:03 UTC (rev 293196)
+++ trunk/Source/WebCore/editing/FrameSelection.h	2022-04-22 00:01:22 UTC (rev 293197)
@@ -191,7 +191,7 @@
     void debugRenderer(RenderObject*, bool selected) const;
 
     void nodeWillBeRemoved(Node&);
-    void textWasReplaced(CharacterData*, unsigned offset, unsigned oldLength, unsigned newLength);
+    void textWasReplaced(CharacterData&, unsigned offset, unsigned oldLength, unsigned newLength);
 
     void setCaretVisible(bool caretIsVisible) { setCaretVisibility(caretIsVisible ? Visible : Hidden, ShouldUpdateAppearance::Yes); }
     void paintCaret(GraphicsContext&, const LayoutPoint&, const LayoutRect& clipRect);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to