Changeset 164133 in webkit

Timestamp:

Feb 14, 2014 2:27:04 PM (10 years ago)

Author:

rniwa@webkit.org

Message:

setSelection should not synchronously trigger layout
​https://bugs.webkit.org/show_bug.cgi?id=128797

Reviewed by Antti Koivisto.

Only update the appearance and reveal selection when the style and the layout is already up to date.
Otherwise, do so in performPostLayoutTasks.

• editing/FrameSelection.cpp:

(WebCore::FrameSelection::FrameSelection):
(WebCore::FrameSelection::setSelection): Set m_pendingSelectionUpdate and synchronously update caret rect
if we don't need to update style or layout.
(WebCore::updateSelectionByUpdatingLayoutOrStyle): Added. Used by FrameSelection member functions to
trigger layout or style recalc whichever is needed.
(WebCore::FrameSelection::updateAndRevealSelection): Extracted from setSelection.
(WebCore::FrameSelection::absoluteCaretBounds): Call updateSelectionByUpdatingLayoutOrStyle since caret rect
is no longer updated synchronously in setSelection.
(WebCore::FrameSelection::recomputeCaretRect): Don't assert that visibleStart().absoluteCaretBounds() is
equal to m_absCaretBounds since selection may no longer be caret at this point.
(WebCore::FrameSelection::setCaretVisibility): Call updateSelectionByUpdatingLayoutOrStyle since we're
synchronously calling into updateAppearance here. In the future, we should make this asynchronous as well.
(WebCore::FrameSelection::selectionBounds): Call updateSelectionByUpdatingLayoutOrStyle since selection bounds
could be outdated. This code only triggering style recalc was presumably a bug.

• editing/FrameSelection.h:

• page/FrameView.cpp:

(WebCore::FrameView::performPostLayoutTasks): Update selection's appearance and scroll to reveal selection
as needed.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• editing/FrameSelection.cpp (modified) (7 diffs)
• editing/FrameSelection.h (modified) (2 diffs)
• page/FrameView.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2014-02-13  Ryosuke Niwa  <rniwa@webkit.org>
+
+        setSelection should not synchronously trigger layout
+        https://bugs.webkit.org/show_bug.cgi?id=128797
+
+        Reviewed by Antti Koivisto.
+
+        Only update the appearance and reveal selection when the style and the layout is already up to date.
+        Otherwise, do so in performPostLayoutTasks.
+
+        * editing/FrameSelection.cpp:
+        (WebCore::FrameSelection::FrameSelection):
+        (WebCore::FrameSelection::setSelection): Set m_pendingSelectionUpdate and synchronously update caret rect
+        if we don't need to update style or layout.
+        (WebCore::updateSelectionByUpdatingLayoutOrStyle): Added. Used by FrameSelection member functions to
+        trigger layout or style recalc whichever is needed.
+        (WebCore::FrameSelection::updateAndRevealSelection): Extracted from setSelection.
+        (WebCore::FrameSelection::absoluteCaretBounds): Call updateSelectionByUpdatingLayoutOrStyle since caret rect
+        is no longer updated synchronously in setSelection.
+        (WebCore::FrameSelection::recomputeCaretRect): Don't assert that visibleStart().absoluteCaretBounds() is
+        equal to m_absCaretBounds since selection may no longer be caret at this point.
+        (WebCore::FrameSelection::setCaretVisibility): Call updateSelectionByUpdatingLayoutOrStyle since we're
+        synchronously calling into updateAppearance here. In the future, we should make this asynchronous as well.
+        (WebCore::FrameSelection::selectionBounds): Call updateSelectionByUpdatingLayoutOrStyle since selection bounds
+        could be outdated. This code only triggering style recalc was presumably a bug.
+        * editing/FrameSelection.h:
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::performPostLayoutTasks): Update selection's appearance and scroll to reveal selection
+        as needed.
+
 2014-02-14  Andreas Kling  <akling@apple.com>
 

trunk/Source/WebCore/editing/FrameSelection.cpp

@@ -117 +117 @@
     , m_focused(frame && frame->page() && frame->page()->focusController().focusedFrame() == frame)
     , m_shouldShowBlockCursor(false)
+    , m_pendingSelectionUpdate(false)
+    , m_shouldRevealSelection(false)
+    , m_alwaysAlignCursorOnScrollWhenRevealingSelection(false)
 #if PLATFORM(IOS)
     , m_updateAppearanceEnabled(false)
@@ -314 +317 @@
         return;
 
+    Document* document = m_frame->document();
+    if (!document)
+        return;
+
+    m_shouldRevealSelection = options & RevealSelection;
+    m_alwaysAlignCursorOnScrollWhenRevealingSelection = align == AlignCursorOnScrollAlways;
+
+    m_pendingSelectionUpdate = true;
+
+    if (document->hasPendingStyleRecalc())
+        return;
+
+    FrameView* frameView = document->view();
+    if (frameView && frameView->layoutPending())
+        return;
+
+    updateAndRevealSelection();
+}
+
+static void updateSelectionByUpdatingLayoutOrStyle(Frame& frame)
+{
 #if ENABLE(TEXT_CARET)
-    m_frame->document()->updateLayoutIgnorePendingStylesheets();
+    frame.document()->updateLayoutIgnorePendingStylesheets();
 #else
-    m_frame->document()->updateStyleIfNeeded();
-#endif
+    frame.document()->updateStyleIfNeeded();
+#endif
+}
+
+void FrameSelection::updateAndRevealSelection()
+{
+    if (!m_pendingSelectionUpdate)
+        return;
+
+    m_pendingSelectionUpdate = false;
+
     updateAppearance();
 
-    if (options & RevealSelection) {
+    if (m_shouldRevealSelection) {
         ScrollAlignment alignment;
 
         if (m_frame->editor().behavior().shouldCenterAlignWhenSelectionIsRevealed())
-            alignment = (align == AlignCursorOnScrollAlways) ? ScrollAlignment::alignCenterAlways : ScrollAlignment::alignCenterIfNeeded;
+            alignment = m_alwaysAlignCursorOnScrollWhenRevealingSelection ? ScrollAlignment::alignCenterAlways : ScrollAlignment::alignCenterIfNeeded;
         else
-            alignment = (align == AlignCursorOnScrollAlways) ? ScrollAlignment::alignTopAlways : ScrollAlignment::alignToEdgeIfNeeded;
+            alignment = m_alwaysAlignCursorOnScrollWhenRevealingSelection ? ScrollAlignment::alignTopAlways : ScrollAlignment::alignToEdgeIfNeeded;
 
         revealSelection(alignment, RevealExtent);
@@ -1309 +1342 @@
 IntRect FrameSelection::absoluteCaretBounds()
 {
+    if (!m_frame)
+        return IntRect();
+    updateSelectionByUpdatingLayoutOrStyle(*m_frame);
     recomputeCaretRect();
     return m_absCaretBounds;
@@ -1356 +1392 @@
     m_absCaretBounds = absoluteBoundsForLocalCaretRect(rendererForCaretPainting(caretNode.get()), newRect);
 
-    if (m_absCaretBoundsDirty) // We should be able to always assert this condition.
+    if (m_absCaretBoundsDirty && m_selection.isCaret()) // We should be able to always assert this condition.
         ASSERT(m_absCaretBounds == m_selection.visibleStart().absoluteCaretBounds());
 
@@ -1815 +1851 @@
         return;
 
+    // FIXME: We shouldn't trigger a synchrnously layout here.
+    if (m_frame)
+        updateSelectionByUpdatingLayoutOrStyle(*m_frame);
+
 #if ENABLE(TEXT_CARET)
-    m_frame->document()->updateLayoutIgnorePendingStylesheets();
     if (m_caretPaint) {
         m_caretPaint = false;
@@ -1822 +1861 @@
     }
     CaretBase::setCaretVisibility(visibility);
-#else
-    m_frame->document()->updateStyleIfNeeded();
 #endif
 
@@ -1921 +1958 @@
         return LayoutRect();
 
-    m_frame->document()->updateStyleIfNeeded();
+    updateSelectionByUpdatingLayoutOrStyle(*m_frame);
     RenderView* root = m_frame->contentRenderer();
     FrameView* view = m_frame->view();

trunk/Source/WebCore/editing/FrameSelection.h

@@ -144 +144 @@
     const VisibleSelection& selection() const { return m_selection; }
     void setSelection(const VisibleSelection&, SetSelectionOptions = defaultSetSelectionOptions(), CursorAlignOnScroll = AlignCursorOnScrollIfNeeded, TextGranularity = CharacterGranularity);
+    void updateAndRevealSelection();
     bool setSelectedRange(Range*, EAffinity, bool closeTyping);
     void selectAll();
@@ -332 +333 @@
     bool m_focused : 1;
     bool m_shouldShowBlockCursor : 1;
+    bool m_pendingSelectionUpdate : 1;
+    bool m_shouldRevealSelection : 1;
+    bool m_alwaysAlignCursorOnScrollWhenRevealingSelection : 1;
 
 #if PLATFORM(IOS)

trunk/Source/WebCore/page/FrameView.cpp

@@ -2628 +2628 @@
 
     frame().selection().setCaretRectNeedsUpdate();
-    frame().selection().updateAppearance();
+    frame().selection().updateAndRevealSelection();
 
     LayoutMilestones requestedMilestones = 0;