Diff
Modified: trunk/LayoutTests/ChangeLog (122138 => 122139)
--- trunk/LayoutTests/ChangeLog 2012-07-09 19:33:24 UTC (rev 122138)
+++ trunk/LayoutTests/ChangeLog 2012-07-09 19:36:57 UTC (rev 122139)
@@ -1,3 +1,17 @@
+2012-07-09 Leandro Gracia Gil <[email protected]>
+
+ SurroundingText should not advance character iterators if they are at end.
+ https://bugs.webkit.org/show_bug.cgi?id=90560
+
+ Reviewed by Ryosuke Niwa.
+
+ Add a new test case where character iterators are already at end when
+ trying to advance. This was caught by Chromium's address sanitizer
+ here: http://code.google.com/p/chromium/issues/detail?id=135705
+
+ * platform/chromium/editing/surrounding-text/surrounding-text-expected.txt:
+ * platform/chromium/editing/surrounding-text/surrounding-text.html:
+
2012-07-09 Rafael Weinstein <[email protected]>
Unreviewed gardening. Update TestExpectations after filename change in r122109.
Modified: trunk/LayoutTests/platform/chromium/editing/surrounding-text/surrounding-text-expected.txt (122138 => 122139)
--- trunk/LayoutTests/platform/chromium/editing/surrounding-text/surrounding-text-expected.txt 2012-07-09 19:33:24 UTC (rev 122138)
+++ trunk/LayoutTests/platform/chromium/editing/surrounding-text/surrounding-text-expected.txt 2012-07-09 19:36:57 UTC (rev 122139)
@@ -15,6 +15,7 @@
PASS surroundingText('<button>.</button><div id="here">012345678901234567890123456789</div><button>.</button>', 15, 12) is "901234567890"
PASS surroundingText('<option>.</option>12345<button id="here">test</button><option>.</option>', 0, 100) is ""
PASS surroundingText('<option>.</option>12345<button>te<span id="here">st</span></button><option>.</option>', 0, 100) is ""
+PASS surroundingText('<p id="here">.</p>', 0, 2) is "."
PASS successfullyParsed is true
TEST COMPLETE
Modified: trunk/LayoutTests/platform/chromium/editing/surrounding-text/surrounding-text.html (122138 => 122139)
--- trunk/LayoutTests/platform/chromium/editing/surrounding-text/surrounding-text.html 2012-07-09 19:33:24 UTC (rev 122138)
+++ trunk/LayoutTests/platform/chromium/editing/surrounding-text/surrounding-text.html 2012-07-09 19:36:57 UTC (rev 122139)
@@ -40,6 +40,7 @@
shouldBeEqualToString('surroundingText(\'<button>.</button><div id="here">012345678901234567890123456789</div><button>.</button>\', 15, 12)', '901234567890');
shouldBeEqualToString('surroundingText(\'<option>.</option>12345<button id="here">test</button><option>.</option>\', 0, 100)', '');
shouldBeEqualToString('surroundingText(\'<option>.</option>12345<button>te<span id="here">st</span></button><option>.</option>\', 0, 100)', '');
+ shouldBeEqualToString('surroundingText(\'<p id="here">.</p>\', 0, 2)', '.');
document.body.removeChild(document.getElementById('test'));
finishJSTest();
Modified: trunk/Source/WebCore/ChangeLog (122138 => 122139)
--- trunk/Source/WebCore/ChangeLog 2012-07-09 19:33:24 UTC (rev 122138)
+++ trunk/Source/WebCore/ChangeLog 2012-07-09 19:36:57 UTC (rev 122139)
@@ -1,3 +1,20 @@
+2012-07-09 Leandro Gracia Gil <[email protected]>
+
+ SurroundingText should not advance character iterators if they are at end.
+ https://bugs.webkit.org/show_bug.cgi?id=90560
+
+ Reviewed by Ryosuke Niwa.
+
+ CharacterIterator and BackwardsCharacterIterator try to advance their
+ internal TextIterator without checking if they already are at end.
+ This can cause crashes in TextIterator::advance.
+
+ Test: platform/chromium/editing/surrounding-text/surrounding-text.html
+
+ * editing/SurroundingText.cpp:
+ (WebCore::SurroundingText::SurroundingText):
+ (WebCore::SurroundingText::rangeFromContentOffsets):
+
2012-07-09 Sudarsana Nagineni <[email protected]>
[EFL] [WK2] Ecore errors from ecore_evas_screen_geometry_get()
Modified: trunk/Source/WebCore/editing/SurroundingText.cpp (122138 => 122139)
--- trunk/Source/WebCore/editing/SurroundingText.cpp 2012-07-09 19:33:24 UTC (rev 122138)
+++ trunk/Source/WebCore/editing/SurroundingText.cpp 2012-07-09 19:36:57 UTC (rev 122139)
@@ -43,20 +43,35 @@
SurroundingText::SurroundingText(const VisiblePosition& visiblePosition, unsigned maxLength)
: m_positionOffsetInContent(0)
{
+ if (visiblePosition.isNull())
+ return;
+
const unsigned halfMaxLength = maxLength / 2;
CharacterIterator forwardIterator(makeRange(visiblePosition, endOfDocument(visiblePosition)).get(), TextIteratorStopsOnFormControls);
- forwardIterator.advance(maxLength - halfMaxLength);
+ if (!forwardIterator.atEnd())
+ forwardIterator.advance(maxLength - halfMaxLength);
Position position = visiblePosition.deepEquivalent().parentAnchoredEquivalent();
Document* document = position.document();
- if (!Range::create(document, position, forwardIterator.range()->startPosition())->text().length())
+ RefPtr<Range> forwardRange = forwardIterator.range();
+ if (!forwardRange || !Range::create(document, position, forwardRange->startPosition())->text().length()) {
+ ASSERT(forwardRange);
return;
+ }
BackwardsCharacterIterator backwardsIterator(makeRange(startOfDocument(visiblePosition), visiblePosition).get(), TextIteratorStopsOnFormControls);
- backwardsIterator.advance(halfMaxLength);
+ if (!backwardsIterator.atEnd())
+ backwardsIterator.advance(halfMaxLength);
- m_positionOffsetInContent = Range::create(document, backwardsIterator.range()->endPosition(), position)->text().length();
- m_contentRange = Range::create(document, backwardsIterator.range()->endPosition(), forwardIterator.range()->startPosition());
+ RefPtr<Range> backwardsRange = backwardsIterator.range();
+ if (!backwardsRange) {
+ ASSERT(backwardsRange);
+ return;
+ }
+
+ m_positionOffsetInContent = Range::create(document, backwardsRange->endPosition(), position)->text().length();
+ m_contentRange = Range::create(document, backwardsRange->endPosition(), forwardRange->startPosition());
+ ASSERT(m_contentRange);
}
PassRefPtr<Range> SurroundingText::rangeFromContentOffsets(unsigned startOffsetInContent, unsigned endOffsetInContent)
@@ -65,11 +80,19 @@
return 0;
CharacterIterator iterator(m_contentRange.get());
+
+ ASSERT(!iterator.atEnd());
iterator.advance(startOffsetInContent);
+ ASSERT(iterator.range());
Position start = iterator.range()->startPosition();
+
+ ASSERT(!iterator.atEnd());
iterator.advance(endOffsetInContent - startOffsetInContent);
+
+ ASSERT(iterator.range());
Position end = iterator.range()->startPosition();
+
return Range::create(start.document(), start, end);
}
Modified: trunk/Source/WebKit/chromium/ChangeLog (122138 => 122139)
--- trunk/Source/WebKit/chromium/ChangeLog 2012-07-09 19:33:24 UTC (rev 122138)
+++ trunk/Source/WebKit/chromium/ChangeLog 2012-07-09 19:36:57 UTC (rev 122139)
@@ -1,3 +1,16 @@
+2012-07-09 Leandro Gracia Gil <[email protected]>
+
+ SurroundingText should not advance character iterators if they are at end.
+ https://bugs.webkit.org/show_bug.cgi?id=90560
+
+ Reviewed by Ryosuke Niwa.
+
+ Moving the check for null visible positions to WebCore as it makes no
+ sense to be in a platform-specific code.
+
+ * src/WebSurroundingText.cpp:
+ (WebKit::WebSurroundingText::initialize):
+
2012-07-09 Dana Jansens <[email protected]>
[chromium] Create render surfaces on main thread only for the current frame
Modified: trunk/Source/WebKit/chromium/src/WebSurroundingText.cpp (122138 => 122139)
--- trunk/Source/WebKit/chromium/src/WebSurroundingText.cpp 2012-07-09 19:33:24 UTC (rev 122138)
+++ trunk/Source/WebKit/chromium/src/WebSurroundingText.cpp 2012-07-09 19:36:57 UTC (rev 122139)
@@ -46,11 +46,7 @@
if (!node || !node->renderer())
return;
- VisiblePosition visiblePosition(node->renderer()->positionForPoint(static_cast<IntPoint>(hitTestInfo.localPoint())));
- if (visiblePosition.isNull())
- return;
-
- m_private.reset(new SurroundingText(visiblePosition, maxLength));
+ m_private.reset(new SurroundingText(VisiblePosition(node->renderer()->positionForPoint(static_cast<IntPoint>(hitTestInfo.localPoint()))), maxLength));
}
void WebSurroundingText::initialize(WebNode textNode, size_t offset, size_t maxLength)
