Changeset 239971 in webkit

Timestamp:

Jan 14, 2019 7:31:41 PM (5 years ago)

Author:

Simon Fraser

Message:

Only run the node comparison code in FrameSelection::respondToNodeModification() for range selections
​https://bugs.webkit.org/show_bug.cgi?id=193416

Reviewed by Wenson Hsieh.

The code inside the m_selection.firstRange() clause needs to only run for non-collapsed selections, and
it shows up on Speedometer profiles so optimize to only run this code if we have a selection range.

• editing/FrameSelection.cpp:

(WebCore::FrameSelection::respondToNodeModification):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• editing/FrameSelection.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-01-14  Simon Fraser  <simon.fraser@apple.com>
+
+        Only run the node comparison code in FrameSelection::respondToNodeModification() for range selections
+        https://bugs.webkit.org/show_bug.cgi?id=193416
+
+        Reviewed by Wenson Hsieh.
+
+        The code inside the m_selection.firstRange() clause needs to only run for non-collapsed selections, and
+        it shows up on Speedometer profiles so optimize to only run this code if we have a selection range.
+
+        * editing/FrameSelection.cpp:
+        (WebCore::FrameSelection::respondToNodeModification):
+
 2019-01-14  Simon Fraser  <simon.fraser@apple.com>
 

trunk/Source/WebCore/editing/FrameSelection.cpp

@@ -538 +538 @@
         else
             m_selection.setWithoutValidation(m_selection.end(), m_selection.start());
-    } else if (RefPtr<Range> range = m_selection.firstRange()) {
-        auto compareNodeResult = range->compareNode(node);
-        if (!compareNodeResult.hasException()) {
-            auto compareResult = compareNodeResult.releaseReturnValue();
-            if (compareResult == Range::NODE_BEFORE_AND_AFTER || compareResult == Range::NODE_INSIDE) {
-                // If we did nothing here, when this node's renderer was destroyed, the rect that it 
-                // occupied would be invalidated, but, selection gaps that change as a result of 
-                // the removal wouldn't be invalidated.
-                // FIXME: Don't do so much unnecessary invalidation.
-                clearRenderTreeSelection = true;
+    } else if (isRange()) {
+        if (RefPtr<Range> range = m_selection.firstRange()) {
+            auto compareNodeResult = range->compareNode(node);
+            if (!compareNodeResult.hasException()) {
+                auto compareResult = compareNodeResult.releaseReturnValue();
+                if (compareResult == Range::NODE_BEFORE_AND_AFTER || compareResult == Range::NODE_INSIDE) {
+                    // If we did nothing here, when this node's renderer was destroyed, the rect that it
+                    // occupied would be invalidated, but, selection gaps that change as a result of
+                    // the removal wouldn't be invalidated.
+                    // FIXME: Don't do so much unnecessary invalidation.
+                    clearRenderTreeSelection = true;
+                }
             }
         }