Frame::rectForSelection shouldn't instantiate FrameSelection
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 12 Feb 2014 01:53:03 +0000 (01:53 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 12 Feb 2014 01:53:03 +0000 (01:53 +0000)
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:

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

17 files changed:
Source/WebCore/ChangeLog
Source/WebCore/WebCore.exp.in
Source/WebCore/editing/FrameSelection.cpp
Source/WebCore/editing/FrameSelection.h
Source/WebCore/editing/VisiblePosition.cpp
Source/WebCore/editing/htmlediting.cpp
Source/WebCore/editing/htmlediting.h
Source/WebCore/loader/EmptyClients.h
Source/WebCore/page/EditorClient.h
Source/WebCore/page/Frame.h
Source/WebCore/page/ios/FrameIOS.mm
Source/WebKit/mac/ChangeLog
Source/WebKit/mac/WebCoreSupport/WebEditorClient.h
Source/WebKit/mac/WebCoreSupport/WebEditorClient.mm
Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.h
Source/WebKit2/WebProcess/WebCoreSupport/ios/WebEditorClientIOS.mm

index c38a1cefb95ccbde7f200026dc088085a7803304..cd79739cf3b93c9d36172a8b20cef578e615beab 100644 (file)
@@ -1,3 +1,54 @@
+2014-02-11  Ryosuke Niwa  <rniwa@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  <enrica@apple.com>
 
         Support WebSelections in WK2 on iOS.
index 20112462a2a92539f009093814d623b8f974b7dd..d55aa5f43d89983cd0f3c9b9f26feef22e2a308b 100644 (file)
@@ -2537,7 +2537,6 @@ __ZNK7WebCore4Node25isEditableToAccessibilityENS0_13EditableLevelE
 __ZNK7WebCore5Frame12updateLayoutEv
 __ZNK7WebCore5Frame15innerLineHeightEP7DOMNode
 __ZNK7WebCore5Frame15preferredHeightEv
-__ZNK7WebCore5Frame16rectForSelectionERNS_16VisibleSelectionE
 __ZNK7WebCore5Frame18renderRectForPointE7CGPointPbPf
 __ZNK7WebCore5Frame19rangedSelectionBaseEv
 __ZNK7WebCore5Frame21styleAtSelectionStartEv
index 49c1140b82367ab9d7fb5f5113d2446b43f817ab..c5e3cd8b75ceb69662bbbc9a369d5d8b58d38066 100644 (file)
@@ -119,7 +119,6 @@ FrameSelection::FrameSelection(Frame* frame)
 #if PLATFORM(IOS)
     , m_updateAppearanceEnabled(false)
     , m_caretBlinks(true)
-    , m_closeTypingSuppressions(0)
     , m_scrollingSuppressCount(0)
 #endif
 {
@@ -278,11 +277,7 @@ bool FrameSelection::setSelectionWithoutUpdatingAppearance(const VisibleSelectio
 
     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 @@ void CaretBase::clearCaretRect()
     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 @@ static bool isNonOrphanedCaret(const VisibleSelection& selection)
     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 @@ IntRect FrameSelection::absoluteCaretBounds()
 
 static void repaintCaretForLocalRect(Node* node, const LayoutRect& rect)
 {
-    RenderObject* caretPainter = caretRenderer(node);
+    RenderObject* caretPainter = rendererForCaretPainting(node);
     if (!caretPainter)
         return;
 
@@ -1410,16 +1333,31 @@ bool FrameSelection::recomputeCaretRect()
     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 @@ bool FrameSelection::recomputeCaretRect()
             if (m_previousCaretNode)
                 repaintCaretForLocalRect(m_previousCaretNode.get(), oldRect);
             m_previousCaretNode = caretNode;
-            repaintCaretForLocalRect(caretNode, newRect);
+            repaintCaretForLocalRect(caretNode.get(), newRect);
         }
     }
 #endif
@@ -1492,7 +1430,7 @@ void CaretBase::paintCaret(Node* node, GraphicsContext* context, const LayoutPoi
         return;
 
     LayoutRect drawingRect = localCaretRectWithoutUpdate();
-    RenderObject* renderer = caretRenderer(node);
+    RenderObject* renderer = rendererForCaretPainting(node);
     if (renderer && renderer->isBox())
         toRenderBox(renderer)->flipForWritingMode(drawingRect);
     drawingRect.moveBy(roundedIntPoint(paintOffset));
index 98ddc9d8ade4d900e05e3a6a694e2033394ef489..327873b935e8d341f50542bdb4726866c9916808 100644 (file)
@@ -68,7 +68,6 @@ protected:
     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 @@ public:
     // 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 @@ public:
     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 @@ private:
     bool m_updateAppearanceEnabled : 1;
     bool m_caretBlinks : 1;
     Color m_caretColor;
-    int m_closeTypingSuppressions;
     int m_scrollingSuppressCount;
 #endif
 };
index 9155aaef38ba085554a336c8a915c1e95d08195c..6df35a8dfbb03db37c1d1747f8858dd654a1caf1 100644 (file)
@@ -619,12 +619,9 @@ LayoutRect VisiblePosition::localCaretRect(RenderObject*& renderer) const
 
 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
index e33f5bd228460604a1a6d294aaaf8ad81ec53857..a89029d275cf4a116efbea89a3511df223fac753 100644 (file)
@@ -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 @@ Element* deprecatedEnclosingBlockFlowElement(Node* node)
     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
index 62e5f18ac01c28e6dcacc599328d367fb3413bc3..317e9698b7a9ad8492033443eb231d00f809b734 100644 (file)
@@ -256,6 +256,12 @@ inline bool isAmbiguousBoundaryCharacter(UChar character)
 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
index 6c7865a96ab1d1b9e92c61327623a767a5cf59d9..3f1d82a52fe21fd37c7256fc2db52c7741cd064e 100644 (file)
@@ -470,8 +470,6 @@ public:
     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 { }
index 1561263bbfc98032f6086650d2b7915bd064f169..5a34e4d6faf87a9034288699b1f8631e7dc2f188 100644 (file)
@@ -119,8 +119,6 @@ public:
     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;
index 3e892079d9736bc85cbd73efced9b49095610e8b..29944bc59e126c9078286e7eba2162a51bf4b740 100644 (file)
@@ -256,7 +256,6 @@ namespace WebCore {
         void updateLayout() const;
         NSRect caretRect() const;
         NSRect rectForScrollToVisible() const;
-        NSRect rectForSelection(VisibleSelection&) const;
         DOMCSSStyleDeclaration* styleAtSelectionStart() const;
         unsigned formElementsCharacterCount() const;
         void setTimersPaused(bool);
index 5fa56e3a872341c2fdca38a1420ac221d091f0ce..39ba4684e0e91847f2452b3af824dbed2dba8a91 100644 (file)
@@ -606,51 +606,14 @@ NSRect Frame::caretRect() const
 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
index 5e7f35231fa339819963add819bcabe2f309f9a9..2c243116c9fcfe00d437d616a4b41f423d0299f6 100644 (file)
@@ -1,3 +1,15 @@
+2014-02-11  Ryosuke Niwa  <rniwa@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  <mmaxfield@apple.com>
 
         Convert position:sticky and position:fixed properties to position:static and position:absolute upon copy
index 79f883e258d86befe483a194911b4dd9ddc5325a..077571d9305fd71a10dfaa464c30a4331c08c794 100644 (file)
@@ -134,8 +134,6 @@ private:
     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 @@ private:
     bool m_haveUndoRedoOperations;
     RefPtr<WebCore::TextCheckingRequest> m_textCheckingRequest;
 #if PLATFORM(IOS)
-    int m_selectionNotificationSuppressions;
     bool m_delayingContentChangeNotifications;
     bool m_hasDelayedContentChangeNotification;
 #endif
index 5078ce951ba0057e6aa3c2c7cc8105db97d7e161..8a07c84caa1ba5d856a0c8210688fa10192920a3 100644 (file)
@@ -194,7 +194,6 @@ WebEditorClient::WebEditorClient(WebView *webView)
     , 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 @@ void WebEditorClient::respondToChangedSelection(Frame* frame)
 #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 @@ void WebEditorClient::textDidChangeInTextArea(Element* element)
 
 #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:)])
index 9a9150345fec1308e66a54c14259df6b439417f3..96464d164bc8f6e52a92c009467461ae4506f1f3 100644 (file)
@@ -1,3 +1,13 @@
+2014-02-11  Ryosuke Niwa  <rniwa@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  <enrica@apple.com>
 
         Support WebSelections in WK2 on iOS.
index c20f66fcdc856e4878b006a07ca0788aadb5e25b..f080a7145949c910c172609624f8626aac89ea27 100644 (file)
@@ -153,8 +153,6 @@ private:
     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;
index c3374da5736eacdc4a0a0e8d5ed4a322b04d7509..f5a8fd152c9135c6191e32938478dae33011297f 100644 (file)
@@ -75,16 +75,6 @@ void WebEditorClient::setInsertionPasteboard(const String&)
     notImplemented();
 }
 
-void WebEditorClient::suppressSelectionNotifications()
-{
-    notImplemented();
-}
-
-void WebEditorClient::restoreSelectionNotifications()
-{
-    notImplemented();
-}
-
 void WebEditorClient::startDelayingAndCoalescingContentChangeNotifications()
 {
     notImplemented();