Changeset 88456 in webkit

Timestamp:

Jun 9, 2011, 9:15:29 AM (14 years ago)

Author:

inferno@chromium.org

Message:

2011-06-08 Abhishek Arya <​inferno@chromium.org>

Reviewed by Ryosuke Niwa.

Make indexForVisiblePosition and isSelectableElement static.
​https://bugs.webkit.org/show_bug.cgi?id=62329

This protects us when converting frame->selection->start() or end()
to VisiblePosition which blows away the RenderTextControl from
underneath (due to layout update).

Test: fast/forms/text-control-selection-crash.html

• accessibility/AccessibilityRenderObject.cpp: (WebCore::AccessibilityRenderObject::indexForVisiblePosition):
• rendering/RenderTextControl.cpp: (WebCore::RenderTextControl::selectionStart): (WebCore::RenderTextControl::selectionEnd): (WebCore::RenderTextControl::isSelectableElement): (WebCore::RenderTextControl::indexForVisiblePosition):
• rendering/RenderTextControl.h:

2011-06-08 Abhishek Arya <​inferno@chromium.org>

Reviewed by Ryosuke Niwa.

Tests that setting selection on a text control does not result in crash.
​https://bugs.webkit.org/show_bug.cgi?id=62329

• fast/forms/text-control-selection-crash-expected.txt: Added.
• fast/forms/text-control-selection-crash.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/forms/text-control-selection-crash-expected.txt (added)
• LayoutTests/fast/forms/text-control-selection-crash.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/accessibility/AccessibilityRenderObject.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderTextControl.cpp (modified) (4 diffs)
• Source/WebCore/rendering/RenderTextControl.h (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2011-06-08  Abhishek Arya  <inferno@chromium.org>
+
+        Reviewed by Ryosuke Niwa.
+
+        Tests that setting selection on a text control does not result in crash.
+        https://bugs.webkit.org/show_bug.cgi?id=62329
+
+        * fast/forms/text-control-selection-crash-expected.txt: Added.
+        * fast/forms/text-control-selection-crash.html: Added.
+
 2011-06-09  Csaba Osztrogonác  <ossy@webkit.org>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-06-08  Abhishek Arya  <inferno@chromium.org>
+
+        Reviewed by Ryosuke Niwa.
+
+        Make indexForVisiblePosition and isSelectableElement static.
+        https://bugs.webkit.org/show_bug.cgi?id=62329
+
+        This protects us when converting frame->selection->start() or end()
+        to VisiblePosition which blows away the RenderTextControl from
+        underneath (due to layout update).
+
+        Test: fast/forms/text-control-selection-crash.html
+
+        * accessibility/AccessibilityRenderObject.cpp:
+        (WebCore::AccessibilityRenderObject::indexForVisiblePosition):
+        * rendering/RenderTextControl.cpp:
+        (WebCore::RenderTextControl::selectionStart):
+        (WebCore::RenderTextControl::selectionEnd):
+        (WebCore::RenderTextControl::isSelectableElement):
+        (WebCore::RenderTextControl::indexForVisiblePosition):
+        * rendering/RenderTextControl.h:
+
 2011-06-09  Ben Murdoch  <benm@google.com>
 

trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp

@@ -2500 +2500 @@
 {
     if (isNativeTextControl())
-        return toRenderTextControl(m_renderer)->indexForVisiblePosition(pos);
+        return RenderTextControl::indexForVisiblePosition(toRenderTextControl(m_renderer)->innerTextElement(), pos);
     
     if (!isTextControl())

trunk/Source/WebCore/rendering/RenderTextControl.cpp

@@ -178 +178 @@
     if (!frame)
         return 0;
-    return indexForVisiblePosition(frame->selection()->start());
+    
+    HTMLElement* innerText = innerTextElement();
+    // Do not call innerTextElement() in the function arguments as creating a VisiblePosition
+    // from frame->selection->start() can blow us from underneath. Also, function ordering is
+    // usually dependent on the compiler.
+    return RenderTextControl::indexForVisiblePosition(innerText, frame->selection()->start());
 }
 
@@ -186 +191 @@
     if (!frame)
         return 0;
-    return indexForVisiblePosition(frame->selection()->end());
+
+    HTMLElement* innerText = innerTextElement();
+    // Do not call innerTextElement() in the function arguments as creating a VisiblePosition
+    // from frame->selection->end() can blow us from underneath. Also, function ordering is
+    // usually dependent on the compiler.
+    return RenderTextControl::indexForVisiblePosition(innerText, frame->selection()->end());
 }
 
@@ -230 +240 @@
 }
 
-bool RenderTextControl::isSelectableElement(Node* node) const
-{
-    if (!node)
+bool RenderTextControl::isSelectableElement(HTMLElement* innerText, Node* node)
+{
+    if (!node || !innerText)
         return false;
 
-    HTMLElement* innerText = innerTextElement();
-    if (!innerText)
-        return false;
-    
     if (node->rootEditableElement() == innerText)
         return true;
@@ -313 +319 @@
 }
 
-int RenderTextControl::indexForVisiblePosition(const VisiblePosition& pos) const
+int RenderTextControl::indexForVisiblePosition(HTMLElement* innerTextElement, const VisiblePosition& pos)
 {
     Position indexPosition = pos.deepEquivalent();
-    if (!isSelectableElement(indexPosition.deprecatedNode()))
+    if (!RenderTextControl::isSelectableElement(innerTextElement, indexPosition.deprecatedNode()))
         return 0;
     ExceptionCode ec = 0;
-    RefPtr<Range> range = Range::create(document());
-    range->setStart(innerTextElement(), 0, ec);
+    RefPtr<Range> range = Range::create(indexPosition.document());
+    range->setStart(innerTextElement, 0, ec);
     ASSERT(!ec);
     range->setEnd(indexPosition.deprecatedNode(), indexPosition.deprecatedEditingOffset(), ec);

trunk/Source/WebCore/rendering/RenderTextControl.h

@@ -51 +51 @@
 
     VisiblePosition visiblePositionForIndex(int index) const;
-    int indexForVisiblePosition(const VisiblePosition&) const;
+    static int indexForVisiblePosition(HTMLElement*, const VisiblePosition&);
 
     void updatePlaceholderVisibility(bool, bool);
@@ -103 +103 @@
     bool hasVisibleTextArea() const;
     friend void setSelectionRange(Node*, int start, int end);
-    bool isSelectableElement(Node*) const;
+    static bool isSelectableElement(HTMLElement*, Node*);
     
     virtual int textBlockInsetLeft() const = 0;