https://bugs.webkit.org/show_bug.cgi?id=92376
Patch by Dominik Röttsches <dominik.rottsches@intel.com> on 2012-07-30
Reviewed by Tony Chang.
.:
Added manual test to reliably reproduce assertion failure which is solved by this patch.
* ManualTests/harfbuzz-mouse-selection-crash.html: Added.
Source/WebCore:
Previously, the if condition in offsetForPosition gating the call to
characterIndexForXPosition was comparing a different value than what was actually used
as the argument to calling it. In some cases, this can lead to a minuscule difference
when comparing the two floats - enough to trigger the assertion. To resolve this,
the accuracy of the index calculation is improved by changing the types from int
to floats and rephrasing the if condition to be exactly the same as what is checked
for in the assertion.
Manual test ManualTests/harfbuzz-mouse-selection-crash.html added
which reliably reproduces the assertion failure before this change.
* platform/graphics/harfbuzz/ng/HarfBuzzShaper.cpp:
(WebCore::HarfBuzzShaper::HarfBuzzRun::characterIndexForXPosition): Types changed to float.
(WebCore::HarfBuzzShaper::offsetForPosition): Types changed to float, if condition rephrased.
* platform/graphics/harfbuzz/ng/HarfBuzzShaper.h:
(HarfBuzzRun):
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@124111
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
+2012-07-30 Dominik Röttsches <dominik.rottsches@intel.com>
+
+ Avoid Assertion Failure in HarfBuzzRun::characterIndexForXPosition
+ https://bugs.webkit.org/show_bug.cgi?id=92376
+
+ Reviewed by Tony Chang.
+
+ Added manual test to reliably reproduce assertion failure which is solved by this patch.
+
+ * ManualTests/harfbuzz-mouse-selection-crash.html: Added.
+
2012-07-29 Vsevolod Vlasov <vsevik@chromium.org>
Web Inspector: Resource agent's reference to cached resources should be weak.
--- /dev/null
+<html>
+<head>
+<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
+<style type="text/css">
+#telugu {
+font-size: 2cm;
+line-height: 1.5em;
+font-family: "Lohit Telugu";
+}
+</style>
+<script>
+var ITERATIONS = 10000;
+
+if (window.testRunner)
+ testRunner.waitUntilDone();
+
+function mouseSelection() {
+ var body = document.body;
+ body.focus();
+
+ xStart = 0;
+ yStart = 0;
+
+ if (window.eventSender) {
+ eventSender.mouseMoveTo(xStart, yStart);
+ eventSender.mouseDown();
+ for(i=0;i<ITERATIONS;i++) {
+ randomCoord = randomCoordinate();
+ eventSender.mouseMoveTo(randomCoord.x, randomCoord.y);
+ }
+ }
+ if (window.testRunnder)
+ testRunner.notifyDone();
+}
+
+function randomCoordinate(axis) {
+ return { x: Math.floor(Math.random()*window.innerWidth),
+ y: Math.floor(Math.random()* window.innerHeight) }
+}
+
+</script>
+</head>
+<body onload="mouseSelection()"><p>
+ This test fuzzes HarfBuzzShaper::offsetForPosition to trigger an assertion in the downstream function characterIndexForXPosition().
+ See https://bugs.webkit.org/show_bug.cgi?id=92376 - reason of the assertion being hit is a minuscule floating point inaccuracy
+ between an if condition that gates the call to characterIndexForXPosition and the actual argument that's then given to the function.
+ This test works on WebKit EFL, with the Ubuntu Telugu Lohit font installed to the EFL jhbuild font directory, like:</p>
+ <pre>$ cp /usr/share/fonts/truetype/ttf-telugu-fonts/lohit_te.ttf WebKitBuild/Dependencies/Source/webkitgtk-test-fonts-0.0.3</pre>
+ <p>Run with:</p>
+ <pre>$ WebKitBuild/Debug/bin/DumpRenderTree ManualTests/harfbuzz-mouse-selection-crash.html</pre>
+ <p>Before the change, this would crash 10/10 times. Test passes if it does not crash.</p>
+<p id="telugu">చేనేత కార్మికుల సమస్యలను ప్రభుత్వం దృష్టికి తీసుకువెళ్లేందుకు సిరిసిల్లలో వైఎస్ఆర్ సిపి గౌరవాధ్యక్షురాలు విజయమ్మ చేపట్టినదీక్ష విజయవంతమైంది.</p>
+</body>
+</html>
+2012-07-30 Dominik Röttsches <dominik.rottsches@intel.com>
+
+ Avoid Assertion Failure in HarfBuzzRun::characterIndexForXPosition
+ https://bugs.webkit.org/show_bug.cgi?id=92376
+
+ Reviewed by Tony Chang.
+
+ Previously, the if condition in offsetForPosition gating the call to
+ characterIndexForXPosition was comparing a different value than what was actually used
+ as the argument to calling it. In some cases, this can lead to a minuscule difference
+ when comparing the two floats - enough to trigger the assertion. To resolve this,
+ the accuracy of the index calculation is improved by changing the types from int
+ to floats and rephrasing the if condition to be exactly the same as what is checked
+ for in the assertion.
+
+ Manual test ManualTests/harfbuzz-mouse-selection-crash.html added
+ which reliably reproduces the assertion failure before this change.
+
+ * platform/graphics/harfbuzz/ng/HarfBuzzShaper.cpp:
+ (WebCore::HarfBuzzShaper::HarfBuzzRun::characterIndexForXPosition): Types changed to float.
+ (WebCore::HarfBuzzShaper::offsetForPosition): Types changed to float, if condition rephrased.
+ * platform/graphics/harfbuzz/ng/HarfBuzzShaper.h:
+ (HarfBuzzRun):
+
2012-06-29 James Robinson <jamesr@chromium.org>
[chromium] Remove WebTransformationMatrix::mapPoint overrides
m_advances[index] = advance;
}
-int HarfBuzzShaper::HarfBuzzRun::characterIndexForXPosition(int targetX)
+int HarfBuzzShaper::HarfBuzzRun::characterIndexForXPosition(float targetX)
{
- ASSERT(static_cast<unsigned>(targetX) <= m_width);
- int currentX = 0;
+ ASSERT(targetX <= m_width);
+ float currentX = 0;
float prevAdvance = 0;
for (unsigned i = 0; i < m_numGlyphs; ++i) {
float currentAdvance = m_advances[i] / 2.0;
- int nextX = currentX + roundf(prevAdvance + currentAdvance);
+ float nextX = currentX + prevAdvance + currentAdvance;
if (currentX <= targetX && targetX <= nextX)
return m_glyphToCharacterIndex[i] + (rtl() ? 1 : 0);
currentX = nextX;
int HarfBuzzShaper::offsetForPosition(float targetX)
{
int charactersSoFar = 0;
- int currentX = 0;
+ float currentX = 0;
if (m_run.rtl()) {
charactersSoFar = m_normalizedBufferLength;
for (int i = m_harfbuzzRuns.size() - 1; i >= 0; --i) {
charactersSoFar -= m_harfbuzzRuns[i]->numCharacters();
- int nextX = currentX + m_harfbuzzRuns[i]->width();
- if (currentX <= targetX && targetX <= nextX) {
+ float nextX = currentX + m_harfbuzzRuns[i]->width();
+ float offsetForRun = targetX - currentX;
+ if (offsetForRun >= 0 && offsetForRun <= m_harfbuzzRuns[i]->width()) {
// The x value in question is within this script run.
- const unsigned index = m_harfbuzzRuns[i]->characterIndexForXPosition(targetX - currentX);
+ const unsigned index = m_harfbuzzRuns[i]->characterIndexForXPosition(offsetForRun);
return charactersSoFar + index;
}
currentX = nextX;
}
} else {
for (unsigned i = 0; i < m_harfbuzzRuns.size(); ++i) {
- int nextX = currentX + m_harfbuzzRuns[i]->width();
- if (currentX <= targetX && targetX <= nextX) {
- const unsigned index = m_harfbuzzRuns[i]->characterIndexForXPosition(targetX - currentX);
+ float nextX = currentX + m_harfbuzzRuns[i]->width();
+ float offsetForRun = targetX - currentX;
+ if (offsetForRun >= 0 && offsetForRun <= m_harfbuzzRuns[i]->width()) {
+ const unsigned index = m_harfbuzzRuns[i]->characterIndexForXPosition(offsetForRun);
return charactersSoFar + index;
}
charactersSoFar += m_harfbuzzRuns[i]->numCharacters();
void setGlyphAndAdvance(unsigned index, uint16_t glyphId, float advance);
void setWidth(float width) { m_width = width; }
- int characterIndexForXPosition(int targetX);
+ int characterIndexForXPosition(float targetX);
float xPositionForOffset(unsigned offset);
const SimpleFontData* fontData() { return m_fontData; }
git://git.webkit.org / WebKit-https.git / commitdiff