Title: [163920] trunk/Source
Revision
163920
Author
rn...@webkit.org
Date
2014-02-11 17:53:03 -0800 (Tue, 11 Feb 2014)

Log Message

Frame::rectForSelection shouldn't instantiate FrameSelection
https://bugs.webkit.org/show_bug.cgi?id=128587

Reviewed by Enrica Casucci.

Source/WebCore: 

Made VisiblePosition::absoluteCaretBounds more interoperable with the one in FrameSelection and made
iOS's Frame::rectForScrollToVisible use that function instead.

The above change allows us to remove:
- suppressCloseTyping(), restoreCloseTyping(), and m_closeTypingSuppressions in FrameSelection
- suppressSelectionNotifications() and restoreSelectionNotifications() in EditorClient

See inline comments below for more details.

* Source/WebCore/WebCore.exp.in:

* editing/FrameSelection.cpp:
(WebCore::FrameSelection::FrameSelection):
(WebCore::FrameSelection::setSelectionWithoutUpdatingAppearance):
(WebCore::CaretBase::updateCaretRect):
(WebCore::FrameSelection::caretRendererWithoutUpdatingLayout):
(WebCore::DragCaretController::caretRenderer):
(WebCore::repaintCaretForLocalRect):
(WebCore::FrameSelection::recomputeCaretRect): Merged FrameSelection::localCaretRect(). Modified
the code to update caretNode when and only when caret rect is updated. Also added an assertion to
ensure absoluteCaretBounds() on FrameSelection and VisiblePosition yield the same result.

(WebCore::CaretBase::paintCaret):
* editing/FrameSelection.h:

* editing/VisiblePosition.cpp:
(WebCore::VisiblePosition::absoluteCaretBounds): Fixed the bug where the old code wasn't respecting
the convention to use containing block as the renderer to paint caret.

* editing/htmlediting.cpp:
(WebCore::caretRendersInsideNode): Moved from FrameSelection.cpp.
(WebCore::rendererForCaretPainting): Ditto and renamed from caretRenderer.
(WebCore::localCaretRectInRendererForCaretPainting): Extracted from FrameSelection::updateCaretRect.
(WebCore::absoluteBoundsForLocalCaretRect): Ditto from CaretBase::absoluteBoundsForLocalRect.
* editing/htmlediting.h:

* loader/EmptyClients.h:
* page/EditorClient.h:
* page/Frame.h:

* page/ios/FrameIOS.mm:
(WebCore::Frame::rectForScrollToVisible): Reimplemented in its simplest form using VisiblePosition's
absoluteCaretBounds().

Source/WebKit/mac: 

* WebCoreSupport/WebEditorClient.h:
* WebCoreSupport/WebEditorClient.mm:
(WebEditorClient::WebEditorClient):
(WebEditorClient::respondToChangedSelection):

Source/WebKit2: 

* WebProcess/WebCoreSupport/WebEditorClient.h:
* WebProcess/WebCoreSupport/ios/WebEditorClientIOS.mm:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (163919 => 163920)


--- trunk/Source/WebCore/ChangeLog	2014-02-12 01:10:21 UTC (rev 163919)
+++ trunk/Source/WebCore/ChangeLog	2014-02-12 01:53:03 UTC (rev 163920)
@@ -1,3 +1,54 @@
+2014-02-11  Ryosuke Niwa  <rn...@webkit.org>
+
+        Frame::rectForSelection shouldn't instantiate FrameSelection
+        https://bugs.webkit.org/show_bug.cgi?id=128587
+
+        Reviewed by Enrica Casucci.
+
+        Made VisiblePosition::absoluteCaretBounds more interoperable with the one in FrameSelection and made
+        iOS's Frame::rectForScrollToVisible use that function instead.
+
+        The above change allows us to remove:
+        - suppressCloseTyping(), restoreCloseTyping(), and m_closeTypingSuppressions in FrameSelection
+        - suppressSelectionNotifications() and restoreSelectionNotifications() in EditorClient
+
+        See inline comments below for more details.
+
+        * Source/WebCore/WebCore.exp.in:
+
+        * editing/FrameSelection.cpp:
+        (WebCore::FrameSelection::FrameSelection):
+        (WebCore::FrameSelection::setSelectionWithoutUpdatingAppearance):
+        (WebCore::CaretBase::updateCaretRect):
+        (WebCore::FrameSelection::caretRendererWithoutUpdatingLayout):
+        (WebCore::DragCaretController::caretRenderer):
+        (WebCore::repaintCaretForLocalRect):
+        (WebCore::FrameSelection::recomputeCaretRect): Merged FrameSelection::localCaretRect(). Modified
+        the code to update caretNode when and only when caret rect is updated. Also added an assertion to
+        ensure absoluteCaretBounds() on FrameSelection and VisiblePosition yield the same result.
+
+        (WebCore::CaretBase::paintCaret):
+        * editing/FrameSelection.h:
+
+        * editing/VisiblePosition.cpp:
+        (WebCore::VisiblePosition::absoluteCaretBounds): Fixed the bug where the old code wasn't respecting
+        the convention to use containing block as the renderer to paint caret.
+
+        * editing/htmlediting.cpp:
+        (WebCore::caretRendersInsideNode): Moved from FrameSelection.cpp.
+        (WebCore::rendererForCaretPainting): Ditto and renamed from caretRenderer.
+        (WebCore::localCaretRectInRendererForCaretPainting): Extracted from FrameSelection::updateCaretRect.
+        (WebCore::absoluteBoundsForLocalCaretRect): Ditto from CaretBase::absoluteBoundsForLocalRect.
+        * editing/htmlediting.h:
+
+        * loader/EmptyClients.h:
+        * page/EditorClient.h:
+        * page/Frame.h:
+
+        * page/ios/FrameIOS.mm:
+        (WebCore::Frame::rectForScrollToVisible): Reimplemented in its simplest form using VisiblePosition's
+        absoluteCaretBounds().
+
 2014-02-11  Enrica Casucci  <enr...@apple.com>
 
         Support WebSelections in WK2 on iOS.

Modified: trunk/Source/WebCore/WebCore.exp.in (163919 => 163920)


--- trunk/Source/WebCore/WebCore.exp.in	2014-02-12 01:10:21 UTC (rev 163919)
+++ trunk/Source/WebCore/WebCore.exp.in	2014-02-12 01:53:03 UTC (rev 163920)
@@ -2537,7 +2537,6 @@
 __ZNK7WebCore5Frame12updateLayoutEv
 __ZNK7WebCore5Frame15innerLineHeightEP7DOMNode
 __ZNK7WebCore5Frame15preferredHeightEv
-__ZNK7WebCore5Frame16rectForSelectionERNS_16VisibleSelectionE
 __ZNK7WebCore5Frame18renderRectForPointE7CGPointPbPf
 __ZNK7WebCore5Frame19rangedSelectionBaseEv
 __ZNK7WebCore5Frame21styleAtSelectionStartEv

Modified: trunk/Source/WebCore/editing/FrameSelection.cpp (163919 => 163920)


--- trunk/Source/WebCore/editing/FrameSelection.cpp	2014-02-12 01:10:21 UTC (rev 163919)
+++ trunk/Source/WebCore/editing/FrameSelection.cpp	2014-02-12 01:53:03 UTC (rev 163920)
@@ -119,7 +119,6 @@
 #if PLATFORM(IOS)
     , m_updateAppearanceEnabled(false)
     , m_caretBlinks(true)
-    , m_closeTypingSuppressions(0)
     , m_scrollingSuppressCount(0)
 #endif
 {
@@ -278,11 +277,7 @@
 
     m_granularity = granularity;
 
-#if PLATFORM(IOS)
-    if (closeTyping && m_closeTypingSuppressions == 0)
-#else
     if (closeTyping)
-#endif
         TypingCommand::closeTyping(m_frame);
 
     if (shouldClearTypingStyle)
@@ -1287,71 +1282,23 @@
     m_caretLocalRect = LayoutRect();
 }
 
-static inline bool caretRendersInsideNode(Node* node)
-{
-    return node && !isRenderedTable(node) && !editingIgnoresContent(node);
-}
-
-static RenderObject* caretRenderer(Node* node)
-{
-    if (!node)
-        return 0;
-
-    RenderObject* renderer = node->renderer();
-    if (!renderer)
-        return 0;
-
-    // if caretNode is a block and caret is inside it then caret should be painted by that block
-    bool paintedByBlock = renderer->isRenderBlockFlow() && caretRendersInsideNode(node);
-    return paintedByBlock ? renderer : renderer->containingBlock();
-}
-
 bool CaretBase::updateCaretRect(Document* document, const VisiblePosition& caretPosition)
 {
     document->updateLayoutIgnorePendingStylesheets();
-    m_caretLocalRect = LayoutRect();
-
     m_caretRectNeedsUpdate = false;
-
-    if (caretPosition.isNull())
-        return false;
-
-    ASSERT(caretPosition.deepEquivalent().deprecatedNode()->renderer());
-
-    // First compute a rect local to the renderer at the selection start.
     RenderObject* renderer;
-    LayoutRect localRect = caretPosition.localCaretRect(renderer);
-
-    // Get the renderer that will be responsible for painting the caret
-    // (which is either the renderer we just found, or one of its containers).
-    RenderObject* caretPainter = caretRenderer(caretPosition.deepEquivalent().deprecatedNode());
-
-    // Compute an offset between the renderer and the caretPainter.
-    bool unrooted = false;
-    while (renderer != caretPainter) {
-        RenderObject* containerObject = renderer->container();
-        if (!containerObject) {
-            unrooted = true;
-            break;
-        }
-        localRect.move(renderer->offsetFromContainer(containerObject, localRect.location()));
-        renderer = containerObject;
-    }
-
-    if (!unrooted)
-        m_caretLocalRect = localRect;
-
-    return true;
+    m_caretLocalRect = localCaretRectInRendererForCaretPainting(caretPosition, renderer);
+    return !m_caretLocalRect.isEmpty();
 }
 
 RenderObject* FrameSelection::caretRendererWithoutUpdatingLayout() const
 {
-    return WebCore::caretRenderer(m_selection.start().deprecatedNode());
+    return rendererForCaretPainting(m_selection.start().deprecatedNode());
 }
 
 RenderObject* DragCaretController::caretRenderer() const
 {
-    return WebCore::caretRenderer(m_position.deepEquivalent().deprecatedNode());
+    return rendererForCaretPainting(m_position.deepEquivalent().deprecatedNode());
 }
 
 static bool isNonOrphanedCaret(const VisibleSelection& selection)
@@ -1359,30 +1306,6 @@
     return selection.isCaret() && !selection.start().isOrphan() && !selection.end().isOrphan();
 }
 
-LayoutRect FrameSelection::localCaretRect()
-{
-    if (shouldUpdateCaretRect()) {
-        if (!isNonOrphanedCaret(m_selection))
-            clearCaretRect();
-        else if (updateCaretRect(m_frame->document(), VisiblePosition(m_selection.start(), m_selection.affinity())))
-            m_absCaretBoundsDirty = true;
-    }
-
-    return localCaretRectWithoutUpdate();
-}
-
-IntRect CaretBase::absoluteBoundsForLocalRect(Node* node, const LayoutRect& rect) const
-{
-    RenderObject* caretPainter = caretRenderer(node);
-    if (!caretPainter)
-        return IntRect();
-    
-    LayoutRect localRect(rect);
-    if (caretPainter->isBox())
-        toRenderBox(caretPainter)->flipForWritingMode(localRect);
-    return caretPainter->localToAbsoluteQuad(FloatRect(localRect)).enclosingBoundingBox();
-}
-
 IntRect FrameSelection::absoluteCaretBounds()
 {
     recomputeCaretRect();
@@ -1391,7 +1314,7 @@
 
 static void repaintCaretForLocalRect(Node* node, const LayoutRect& rect)
 {
-    RenderObject* caretPainter = caretRenderer(node);
+    RenderObject* caretPainter = rendererForCaretPainting(node);
     if (!caretPainter)
         return;
 
@@ -1410,16 +1333,31 @@
     if (!v)
         return false;
 
-    Node* caretNode = m_selection.start().deprecatedNode();
-
     LayoutRect oldRect = localCaretRectWithoutUpdate();
-    LayoutRect newRect = localCaretRect();
 
+    RefPtr<Node> caretNode = m_previousCaretNode;
+    if (shouldUpdateCaretRect()) {
+        if (!isNonOrphanedCaret(m_selection))
+            clearCaretRect();
+        else {
+            VisiblePosition visibleStart = m_selection.visibleStart();
+            if (updateCaretRect(m_frame->document(), visibleStart)) {
+                caretNode = visibleStart.deepEquivalent().deprecatedNode();
+                m_absCaretBoundsDirty = true;
+            }
+        }
+    }
+    LayoutRect newRect = localCaretRectWithoutUpdate();
+
     if (caretNode == m_previousCaretNode && oldRect == newRect && !m_absCaretBoundsDirty)
         return false;
 
     IntRect oldAbsCaretBounds = m_absCaretBounds;
-    m_absCaretBounds = absoluteBoundsForLocalRect(caretNode, localCaretRectWithoutUpdate());
+    m_absCaretBounds = absoluteBoundsForLocalCaretRect(rendererForCaretPainting(caretNode.get()), newRect);
+
+    if (m_absCaretBoundsDirty) // We should be able to always assert this condition.
+        ASSERT(m_absCaretBounds == m_selection.visibleStart().absoluteCaretBounds());
+
     m_absCaretBoundsDirty = false;
 
     if (caretNode == m_previousCaretNode && oldAbsCaretBounds == m_absCaretBounds)
@@ -1432,7 +1370,7 @@
             if (m_previousCaretNode)
                 repaintCaretForLocalRect(m_previousCaretNode.get(), oldRect);
             m_previousCaretNode = caretNode;
-            repaintCaretForLocalRect(caretNode, newRect);
+            repaintCaretForLocalRect(caretNode.get(), newRect);
         }
     }
 #endif
@@ -1492,7 +1430,7 @@
         return;
 
     LayoutRect drawingRect = localCaretRectWithoutUpdate();
-    RenderObject* renderer = caretRenderer(node);
+    RenderObject* renderer = rendererForCaretPainting(node);
     if (renderer && renderer->isBox())
         toRenderBox(renderer)->flipForWritingMode(drawingRect);
     drawingRect.moveBy(roundedIntPoint(paintOffset));

Modified: trunk/Source/WebCore/editing/FrameSelection.h (163919 => 163920)


--- trunk/Source/WebCore/editing/FrameSelection.h	2014-02-12 01:10:21 UTC (rev 163919)
+++ trunk/Source/WebCore/editing/FrameSelection.h	2014-02-12 01:53:03 UTC (rev 163920)
@@ -68,7 +68,6 @@
     void invalidateCaretRect(Node*, bool caretRectChanged = false);
     void clearCaretRect();
     bool updateCaretRect(Document*, const VisiblePosition& caretPosition);
-    IntRect absoluteBoundsForLocalRect(Node*, const LayoutRect&) const;
     bool shouldRepaintCaret(const RenderView*, bool isContentEditable) const;
     void paintCaret(Node*, GraphicsContext*, const LayoutPoint&, const LayoutRect& clipRect) const;
 
@@ -168,9 +167,6 @@
     // Return the renderer that is responsible for painting the caret (in the selection start node)
     RenderObject* caretRendererWithoutUpdatingLayout() const;
 
-    // Caret rect local to the caret's renderer
-    LayoutRect localCaretRect();
-
     // Bounds of (possibly transformed) caret in absolute coords
     IntRect absoluteCaretBounds();
     void setCaretRectNeedsUpdate() { CaretBase::setCaretRectNeedsUpdate(); }
@@ -229,8 +225,6 @@
     PassRefPtr<Range> rangeByMovingCurrentSelection(int amount) const;
     PassRefPtr<Range> rangeByExtendingCurrentSelection(int amount) const;
     void selectRangeOnElement(unsigned location, unsigned length, Node*);
-    void suppressCloseTyping() { ++m_closeTypingSuppressions; }
-    void restoreCloseTyping() { --m_closeTypingSuppressions; }
     void clearCurrentSelection();
     void setCaretBlinks(bool caretBlinks = true);
     void setCaretColor(const Color&);
@@ -342,7 +336,6 @@
     bool m_updateAppearanceEnabled : 1;
     bool m_caretBlinks : 1;
     Color m_caretColor;
-    int m_closeTypingSuppressions;
     int m_scrollingSuppressCount;
 #endif
 };

Modified: trunk/Source/WebCore/editing/VisiblePosition.cpp (163919 => 163920)


--- trunk/Source/WebCore/editing/VisiblePosition.cpp	2014-02-12 01:10:21 UTC (rev 163919)
+++ trunk/Source/WebCore/editing/VisiblePosition.cpp	2014-02-12 01:53:03 UTC (rev 163920)
@@ -619,12 +619,9 @@
 
 IntRect VisiblePosition::absoluteCaretBounds() const
 {
-    RenderObject* renderer;
-    LayoutRect localRect = localCaretRect(renderer);
-    if (localRect.isEmpty() || !renderer)
-        return IntRect();
-
-    return renderer->localToAbsoluteQuad(FloatRect(localRect)).enclosingBoundingBox();
+    RenderObject* renderer = nullptr;
+    LayoutRect localRect = localCaretRectInRendererForCaretPainting(*this, renderer);
+    return absoluteBoundsForLocalCaretRect(renderer, localRect);
 }
 
 int VisiblePosition::lineDirectionPointForBlockDirectionNavigation() const

Modified: trunk/Source/WebCore/editing/htmlediting.cpp (163919 => 163920)


--- trunk/Source/WebCore/editing/htmlediting.cpp	2014-02-12 01:10:21 UTC (rev 163919)
+++ trunk/Source/WebCore/editing/htmlediting.cpp	2014-02-12 01:53:03 UTC (rev 163920)
@@ -45,6 +45,7 @@
 #include "HTMLUListElement.h"
 #include "NodeTraversal.h"
 #include "PositionIterator.h"
+#include "RenderBlock.h"
 #include "RenderElement.h"
 #include "ShadowRoot.h"
 #include "Text.h"
@@ -1260,4 +1261,61 @@
     return 0;
 }
 
+static inline bool caretRendersInsideNode(Node* node)
+{
+    return node && !isRenderedTable(node) && !editingIgnoresContent(node);
+}
+
+RenderObject* rendererForCaretPainting(Node* node)
+{
+    if (!node)
+        return 0;
+
+    RenderObject* renderer = node->renderer();
+    if (!renderer)
+        return 0;
+
+    // If caretNode is a block and caret is inside it, then caret should be painted by that block.
+    bool paintedByBlock = renderer->isRenderBlockFlow() && caretRendersInsideNode(node);
+    return paintedByBlock ? renderer : renderer->containingBlock();
+}
+
+LayoutRect localCaretRectInRendererForCaretPainting(const VisiblePosition& caretPosition, RenderObject*& caretPainter)
+{
+    if (caretPosition.isNull())
+        return LayoutRect();
+
+    ASSERT(caretPosition.deepEquivalent().deprecatedNode()->renderer());
+
+    // First compute a rect local to the renderer at the selection start.
+    RenderObject* renderer;
+    LayoutRect localRect = caretPosition.localCaretRect(renderer);
+
+    // Get the renderer that will be responsible for painting the caret
+    // (which is either the renderer we just found, or one of its containers).
+    caretPainter = rendererForCaretPainting(caretPosition.deepEquivalent().deprecatedNode());
+
+    // Compute an offset between the renderer and the caretPainter.
+    while (renderer != caretPainter) {
+        RenderObject* containerObject = renderer->container();
+        if (!containerObject)
+            return LayoutRect();
+        localRect.move(renderer->offsetFromContainer(containerObject, localRect.location()));
+        renderer = containerObject;
+    }
+
+    return localRect;
+}
+
+IntRect absoluteBoundsForLocalCaretRect(RenderObject* rendererForCaretPainting, const LayoutRect& rect)
+{
+    if (!rendererForCaretPainting || rect.isEmpty())
+        return IntRect();
+
+    LayoutRect localRect(rect);
+    if (rendererForCaretPainting->isBox())
+        toRenderBox(rendererForCaretPainting)->flipForWritingMode(localRect);
+    return rendererForCaretPainting->localToAbsoluteQuad(FloatRect(localRect)).enclosingBoundingBox();
+}
+
 } // namespace WebCore

Modified: trunk/Source/WebCore/editing/htmlediting.h (163919 => 163920)


--- trunk/Source/WebCore/editing/htmlediting.h	2014-02-12 01:10:21 UTC (rev 163919)
+++ trunk/Source/WebCore/editing/htmlediting.h	2014-02-12 01:53:03 UTC (rev 163920)
@@ -256,6 +256,12 @@
 String stringWithRebalancedWhitespace(const String&, bool startIsStartOfParagraph, bool endIsEndOfParagraph);
 const String& nonBreakingSpaceString();
 
+// Miscellaaneous functions that for caret rendering
+
+RenderObject* rendererForCaretPainting(Node*);
+LayoutRect localCaretRectInRendererForCaretPainting(const VisiblePosition&, RenderObject*&);
+IntRect absoluteBoundsForLocalCaretRect(RenderObject* rendererForCaretPainting, const LayoutRect&);
+
 }
 
 #endif

Modified: trunk/Source/WebCore/loader/EmptyClients.h (163919 => 163920)


--- trunk/Source/WebCore/loader/EmptyClients.h	2014-02-12 01:10:21 UTC (rev 163919)
+++ trunk/Source/WebCore/loader/EmptyClients.h	2014-02-12 01:53:03 UTC (rev 163920)
@@ -470,8 +470,6 @@
     virtual void textDidChangeInTextArea(Element*) override { }
 
 #if PLATFORM(IOS)
-    virtual void suppressSelectionNotifications() override { }
-    virtual void restoreSelectionNotifications() override { }
     virtual void startDelayingAndCoalescingContentChangeNotifications() override { }
     virtual void stopDelayingAndCoalescingContentChangeNotifications() override { }
     virtual void writeDataToPasteboard(NSDictionary*) override { }

Modified: trunk/Source/WebCore/page/EditorClient.h (163919 => 163920)


--- trunk/Source/WebCore/page/EditorClient.h	2014-02-12 01:10:21 UTC (rev 163919)
+++ trunk/Source/WebCore/page/EditorClient.h	2014-02-12 01:53:03 UTC (rev 163920)
@@ -119,8 +119,6 @@
     virtual void textDidChangeInTextArea(Element*) = 0;
 
 #if PLATFORM(IOS)
-    virtual void suppressSelectionNotifications() = 0;
-    virtual void restoreSelectionNotifications() = 0;
     virtual void startDelayingAndCoalescingContentChangeNotifications() = 0;
     virtual void stopDelayingAndCoalescingContentChangeNotifications() = 0;
     virtual void writeDataToPasteboard(NSDictionary*) = 0;

Modified: trunk/Source/WebCore/page/Frame.h (163919 => 163920)


--- trunk/Source/WebCore/page/Frame.h	2014-02-12 01:10:21 UTC (rev 163919)
+++ trunk/Source/WebCore/page/Frame.h	2014-02-12 01:53:03 UTC (rev 163920)
@@ -256,7 +256,6 @@
         void updateLayout() const;
         NSRect caretRect() const;
         NSRect rectForScrollToVisible() const;
-        NSRect rectForSelection(VisibleSelection&) const;
         DOMCSSStyleDeclaration* styleAtSelectionStart() const;
         unsigned formElementsCharacterCount() const;
         void setTimersPaused(bool);

Modified: trunk/Source/WebCore/page/ios/FrameIOS.mm (163919 => 163920)


--- trunk/Source/WebCore/page/ios/FrameIOS.mm	2014-02-12 01:10:21 UTC (rev 163919)
+++ trunk/Source/WebCore/page/ios/FrameIOS.mm	2014-02-12 01:53:03 UTC (rev 163920)
@@ -606,51 +606,14 @@
 NSRect Frame::rectForScrollToVisible() const
 {
     VisibleSelection selection(this->selection().selection());
-    return rectForSelection(selection);
-}
 
-NSRect Frame::rectForSelection(VisibleSelection& selection) const
-{
     if (selection.isNone())
         return CGRectZero;
 
     if (selection.isCaret())
         return caretRect();
 
-    EditorClient* client = editor().client();
-    if (client)
-        client->suppressSelectionNotifications();
-
-    VisibleSelection originalSelection(selection);
-    Position position;
-
-    // The selection controllers below need to be associated with a frame in order
-    // to calculate geometry. This causes them to do more work here than we would
-    // like. Ideally, we would have a sort offline geometry-only mode for selection
-    // controllers so we could do this kind of work as cheaply as possible.
-
-    position = originalSelection.start();
-    selection.setBase(position);
-    selection.setExtent(position);
-    FrameSelection startFrameSelection(const_cast<Frame*>(this));
-    startFrameSelection.suppressCloseTyping();
-    startFrameSelection.setSelection(selection);
-    FloatRect startRect(startFrameSelection.absoluteCaretBounds());
-    startFrameSelection.restoreCloseTyping();
-
-    position = originalSelection.end();
-    selection.setBase(position);
-    selection.setExtent(position);
-    FrameSelection endFrameSelection(const_cast<Frame*>(this));
-    endFrameSelection.suppressCloseTyping();
-    endFrameSelection.setSelection(selection);
-    FloatRect endRect(endFrameSelection.absoluteCaretBounds());
-    endFrameSelection.restoreCloseTyping();
-
-    if (client)
-        client->restoreSelectionNotifications();
-
-    return unionRect(startRect, endRect);
+    return unionRect(selection.visibleStart().absoluteCaretBounds(), selection.visibleEnd().absoluteCaretBounds());
 }
 
 DOMCSSStyleDeclaration* Frame::styleAtSelectionStart() const

Modified: trunk/Source/WebKit/mac/ChangeLog (163919 => 163920)


--- trunk/Source/WebKit/mac/ChangeLog	2014-02-12 01:10:21 UTC (rev 163919)
+++ trunk/Source/WebKit/mac/ChangeLog	2014-02-12 01:53:03 UTC (rev 163920)
@@ -1,3 +1,15 @@
+2014-02-11  Ryosuke Niwa  <rn...@webkit.org>
+
+        Frame::rectForSelection shouldn't instantiate FrameSelection
+        https://bugs.webkit.org/show_bug.cgi?id=128587
+
+        Reviewed by Enrica Casucci.
+
+        * WebCoreSupport/WebEditorClient.h:
+        * WebCoreSupport/WebEditorClient.mm:
+        (WebEditorClient::WebEditorClient):
+        (WebEditorClient::respondToChangedSelection):
+
 2014-02-10  Myles C. Maxfield  <mmaxfi...@apple.com>
 
         Convert position:sticky and position:fixed properties to position:static and position:absolute upon copy

Modified: trunk/Source/WebKit/mac/WebCoreSupport/WebEditorClient.h (163919 => 163920)


--- trunk/Source/WebKit/mac/WebCoreSupport/WebEditorClient.h	2014-02-12 01:10:21 UTC (rev 163919)
+++ trunk/Source/WebKit/mac/WebCoreSupport/WebEditorClient.h	2014-02-12 01:53:03 UTC (rev 163920)
@@ -134,8 +134,6 @@
     virtual void textDidChangeInTextArea(WebCore::Element*) override;
 
 #if PLATFORM(IOS)
-    virtual void suppressSelectionNotifications() override;
-    virtual void restoreSelectionNotifications() override;
     virtual void startDelayingAndCoalescingContentChangeNotifications() override;
     virtual void stopDelayingAndCoalescingContentChangeNotifications() override;
     virtual void writeDataToPasteboard(NSDictionary*) override;
@@ -172,7 +170,6 @@
     bool m_haveUndoRedoOperations;
     RefPtr<WebCore::TextCheckingRequest> m_textCheckingRequest;
 #if PLATFORM(IOS)
-    int m_selectionNotificationSuppressions;
     bool m_delayingContentChangeNotifications;
     bool m_hasDelayedContentChangeNotification;
 #endif

Modified: trunk/Source/WebKit/mac/WebCoreSupport/WebEditorClient.mm (163919 => 163920)


--- trunk/Source/WebKit/mac/WebCoreSupport/WebEditorClient.mm	2014-02-12 01:10:21 UTC (rev 163919)
+++ trunk/Source/WebKit/mac/WebCoreSupport/WebEditorClient.mm	2014-02-12 01:53:03 UTC (rev 163920)
@@ -194,7 +194,6 @@
     , m_undoTarget([[[WebEditorUndoTarget alloc] init] autorelease])
     , m_haveUndoRedoOperations(false)
 #if PLATFORM(IOS)
-    , m_selectionNotificationSuppressions(0)
     , m_delayingContentChangeNotifications(0)
     , m_hasDelayedContentChangeNotification(0)
 #endif
@@ -357,7 +356,7 @@
 #else
     // Selection can be changed while deallocating down the WebView / Frame / Editor.  Do not post in that case because it's already too late
     // for the NSInvocation to retain the WebView.
-    if (![m_webView _isClosing] && m_selectionNotificationSuppressions == 0)
+    if (![m_webView _isClosing])
         WebThreadPostNotification(WebViewDidChangeSelectionNotification, m_webView, nil);
 #endif
 }
@@ -798,18 +797,6 @@
 
 #if PLATFORM(IOS)
 
-void WebEditorClient::suppressSelectionNotifications() 
-{
-    m_selectionNotificationSuppressions++;
-}
-
-void WebEditorClient::restoreSelectionNotifications() 
-{
-    --m_selectionNotificationSuppressions;
-    if (m_selectionNotificationSuppressions < 0)
-        m_selectionNotificationSuppressions = 0;
-}
-
 void WebEditorClient::writeDataToPasteboard(NSDictionary* representation)
 {
     if ([[m_webView _UIKitDelegateForwarder] respondsToSelector:@selector(writeDataToPasteboard:)])

Modified: trunk/Source/WebKit2/ChangeLog (163919 => 163920)


--- trunk/Source/WebKit2/ChangeLog	2014-02-12 01:10:21 UTC (rev 163919)
+++ trunk/Source/WebKit2/ChangeLog	2014-02-12 01:53:03 UTC (rev 163920)
@@ -1,3 +1,13 @@
+2014-02-11  Ryosuke Niwa  <rn...@webkit.org>
+
+        Frame::rectForSelection shouldn't instantiate FrameSelection
+        https://bugs.webkit.org/show_bug.cgi?id=128587
+
+        Reviewed by Enrica Casucci.
+
+        * WebProcess/WebCoreSupport/WebEditorClient.h:
+        * WebProcess/WebCoreSupport/ios/WebEditorClientIOS.mm:
+
 2014-02-11  Enrica Casucci  <enr...@apple.com>
 
         Support WebSelections in WK2 on iOS.

Modified: trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.h (163919 => 163920)


--- trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.h	2014-02-12 01:10:21 UTC (rev 163919)
+++ trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.h	2014-02-12 01:53:03 UTC (rev 163920)
@@ -153,8 +153,6 @@
     virtual bool shouldShowUnicodeMenu() override;
 #endif
 #if PLATFORM(IOS)
-    virtual void suppressSelectionNotifications() override;
-    virtual void restoreSelectionNotifications() override;
     virtual void startDelayingAndCoalescingContentChangeNotifications() override;
     virtual void stopDelayingAndCoalescingContentChangeNotifications() override;
     virtual void writeDataToPasteboard(NSDictionary*) override;

Modified: trunk/Source/WebKit2/WebProcess/WebCoreSupport/ios/WebEditorClientIOS.mm (163919 => 163920)


--- trunk/Source/WebKit2/WebProcess/WebCoreSupport/ios/WebEditorClientIOS.mm	2014-02-12 01:10:21 UTC (rev 163919)
+++ trunk/Source/WebKit2/WebProcess/WebCoreSupport/ios/WebEditorClientIOS.mm	2014-02-12 01:53:03 UTC (rev 163920)
@@ -75,16 +75,6 @@
     notImplemented();
 }
 
-void WebEditorClient::suppressSelectionNotifications()
-{
-    notImplemented();
-}
-
-void WebEditorClient::restoreSelectionNotifications()
-{
-    notImplemented();
-}
-
 void WebEditorClient::startDelayingAndCoalescingContentChangeNotifications()
 {
     notImplemented();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to