Avoid infinite loop when laying out text with unconvertible chars

When the stringToCMap() fails, it can be because it did not have enough
space in the layout, or it can because of other errors. In order to
implement "try-again" processing in a simple way, we had an infinite
loop which assumed that stringToCMap() would always succeed in the
second run (which would be the case if the only possible error was
"not enough space".)

Since there are other possible failures not related to the number of
glyphs, you could easily get into an infinite loop here, e.g. when
laying out text that contains the Byte Order Mark.

The fix changes the implementation to explictly try stringToCMap()
twice at max, and is also how it's implemented in the default
qtextengine.cpp.

Task-number: QTBUG-4680
Reviewed-by: Trond

2 files changed, 48 insertions, 38 deletions

diff --git a/src/gui/text/qtextengine_mac.cpp b/src/gui/text/qtextengine_mac.cpp
index eeccc723b7..54be53b053 100644
--- a/src/gui/text/qtextengine_mac.cpp
+++ b/src/gui/text/qtextengine_mac.cpp
@@ -594,53 +594,50 @@ void QTextEngine::shapeTextMac(int item) const
         str = reinterpret_cast<const QChar *>(uc);
     }
 
-    while (true) {
-        ensureSpace(num_glyphs);
-        num_glyphs = layoutData->glyphLayout.numGlyphs - layoutData->used;
-
-        QGlyphLayout g = availableGlyphs(&si);
-        g.numGlyphs = num_glyphs;
-        unsigned short *log_clusters = logClusters(&si);
+    ensureSpace(num_glyphs);
+    num_glyphs = layoutData->glyphLayout.numGlyphs - layoutData->used;
 
-        if (fe->stringToCMap(str,
-                             len,
-                             &g,
-                             &num_glyphs,
-                             flags,
-                             log_clusters,
-                             attributes())) {
+    QGlyphLayout g = availableGlyphs(&si);
+    g.numGlyphs = num_glyphs;
+    unsigned short *log_clusters = logClusters(&si);
 
-            heuristicSetGlyphAttributes(str, len, &g, log_clusters, num_glyphs);
-            break;
-        }
+    bool stringToCMapFailed = false;
+    if (!fe->stringToCMap(str, len, &g, &num_glyphs, flags, log_clusters, attributes())) {
+        ensureSpace(num_glyphs);
+        stringToCMapFailed = fe->stringToCMap(str, len, &g, &num_glyphs, flags, log_clusters,
+                                              attributes());
     }
 
-    si.num_glyphs = num_glyphs;
+    if (!stringToCMapFailed) {
+        heuristicSetGlyphAttributes(str, len, &g, log_clusters, num_glyphs);
 
-    layoutData->used += si.num_glyphs;
+        si.num_glyphs = num_glyphs;
 
-    QGlyphLayout g = shapedGlyphs(&si);
+        layoutData->used += si.num_glyphs;
 
-    if (si.analysis.script == QUnicodeTables::Arabic) {
-        QVarLengthArray<QArabicProperties> props(len + 2);
-        QArabicProperties *properties = props.data();
-        int f = si.position;
-        int l = len;
-        if (f > 0) {
-            --f;
-            ++l;
-            ++properties;
-        }
-        if (f + l < layoutData->string.length()) {
-            ++l;
-        }
-        qt_getArabicProperties((const unsigned short *)(layoutData->string.unicode()+f), l, props.data());
+        QGlyphLayout g = shapedGlyphs(&si);
 
-        unsigned short *log_clusters = logClusters(&si);
+        if (si.analysis.script == QUnicodeTables::Arabic) {
+            QVarLengthArray<QArabicProperties> props(len + 2);
+            QArabicProperties *properties = props.data();
+            int f = si.position;
+            int l = len;
+            if (f > 0) {
+                --f;
+                ++l;
+                ++properties;
+            }
+            if (f + l < layoutData->string.length()) {
+                ++l;
+            }
+            qt_getArabicProperties((const unsigned short *)(layoutData->string.unicode()+f), l, props.data());
 
-        for (int i = 0; i < len; ++i) {
-            int gpos = log_clusters[i];
-            g.attributes[gpos].justification = properties[i].justification;
+            unsigned short *log_clusters = logClusters(&si);
+
+            for (int i = 0; i < len; ++i) {
+                int gpos = log_clusters[i];
+                g.attributes[gpos].justification = properties[i].justification;
+            }
         }
     }
 
diff --git a/tests/auto/qtextlayout/tst_qtextlayout.cpp b/tests/auto/qtextlayout/tst_qtextlayout.cpp
index fe87dfb175..7c3f4f2481 100644
--- a/tests/auto/qtextlayout/tst_qtextlayout.cpp
+++ b/tests/auto/qtextlayout/tst_qtextlayout.cpp
@@ -118,6 +118,7 @@ private slots:
     void smallTextLengthWordWrap();
     void smallTextLengthWrapAtWordBoundaryOrAnywhere();
     void testLineBreakingAllSpaces();
+    void lineWidthFromBOM();
 
 
 private:
@@ -1306,6 +1307,18 @@ void tst_QTextLayout::columnWrapWithTabs()
 
 }
 
+void tst_QTextLayout::lineWidthFromBOM()
+{
+    const QString string(QChar(0xfeff)); // BYTE ORDER MARK
+    QTextLayout layout(string);
+    layout.beginLayout();
+    QTextLine line = layout.createLine();
+    line.setLineWidth(INT_MAX / 256);
+    layout.endLayout();
+
+    // Don't spin into an infinite loop
+ }
+
 
 QTEST_MAIN(tst_QTextLayout)
 #include "tst_qtextlayout.moc"