setSelection should not synchronously trigger layout
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Feb 2014 22:27:04 +0000 (22:27 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 14 Feb 2014 22:27:04 +0000 (22:27 +0000)
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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@164133 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/editing/FrameSelection.cpp
Source/WebCore/editing/FrameSelection.h
Source/WebCore/page/FrameView.cpp

index c8d30633e421c54b772da9618453b3320b993d15..817103c0a6547bcdfb05e19b8f651011e05662e0 100644 (file)
@@ -1,3 +1,34 @@
+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>
 
         Purge remaining ENABLE(SHADOW_DOM) cruft.
index c5e3cd8b75ceb69662bbbc9a369d5d8b58d38066..50331cf41678de9d6b1b05f90b7be483cfa3452b 100644 (file)
@@ -116,6 +116,9 @@ FrameSelection::FrameSelection(Frame* frame)
     , m_isCaretBlinkingSuspended(false)
     , 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)
     , m_caretBlinks(true)
@@ -313,20 +316,50 @@ void FrameSelection::setSelection(const VisibleSelection& selection, SetSelectio
     if (!setSelectionWithoutUpdatingAppearance(selection, options, align, granularity))
         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();
+    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);
     }
@@ -1308,6 +1341,9 @@ static bool isNonOrphanedCaret(const VisibleSelection& selection)
 
 IntRect FrameSelection::absoluteCaretBounds()
 {
+    if (!m_frame)
+        return IntRect();
+    updateSelectionByUpdatingLayoutOrStyle(*m_frame);
     recomputeCaretRect();
     return m_absCaretBounds;
 }
@@ -1355,7 +1391,7 @@ bool FrameSelection::recomputeCaretRect()
     IntRect oldAbsCaretBounds = m_absCaretBounds;
     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());
 
     m_absCaretBoundsDirty = false;
@@ -1814,15 +1850,16 @@ void FrameSelection::setCaretVisibility(CaretVisibility visibility)
     if (caretVisibility() == visibility)
         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;
         invalidateCaretRect();
     }
     CaretBase::setCaretVisibility(visibility);
-#else
-    m_frame->document()->updateStyleIfNeeded();
 #endif
 
     updateAppearance();
@@ -1920,7 +1957,7 @@ FloatRect FrameSelection::selectionBounds(bool clipToVisibleContent) const
     if (!m_frame->document())
         return LayoutRect();
 
-    m_frame->document()->updateStyleIfNeeded();
+    updateSelectionByUpdatingLayoutOrStyle(*m_frame);
     RenderView* root = m_frame->contentRenderer();
     FrameView* view = m_frame->view();
     if (!root || !view)
index 327873b935e8d341f50542bdb4726866c9916808..f30d66e2c123fe495be5abe6474b2b33b65bb9b3 100644 (file)
@@ -143,6 +143,7 @@ public:
 
     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();
     void clear();
@@ -331,6 +332,9 @@ private:
     bool m_isCaretBlinkingSuspended : 1;
     bool m_focused : 1;
     bool m_shouldShowBlockCursor : 1;
+    bool m_pendingSelectionUpdate : 1;
+    bool m_shouldRevealSelection : 1;
+    bool m_alwaysAlignCursorOnScrollWhenRevealingSelection : 1;
 
 #if PLATFORM(IOS)
     bool m_updateAppearanceEnabled : 1;
index b36c0bed8bfd4b6afd80fb95a307ac78938d5b91..0758e272afdac1a85c08da48a3673029ca4fc350 100644 (file)
@@ -2627,7 +2627,7 @@ void FrameView::performPostLayoutTasks()
     m_postLayoutTasksTimer.stop();
 
     frame().selection().setCaretRectNeedsUpdate();
-    frame().selection().updateAppearance();
+    frame().selection().updateAndRevealSelection();
 
     LayoutMilestones requestedMilestones = 0;
     LayoutMilestones milestonesAchieved = 0;