Merge r224015 - REGRESSION(r222090): [HarfBuzz] Arabic shaping is bro…

…ken except for first word in line

https://bugs.webkit.org/show_bug.cgi?id=178625

Reviewed by Michael Catanzaro.

Source/WebCore:

Once we find the first space, which has the COMMON script, we split the run, and the next ones keep using
COMMON instead of ARABIC because we don't update the current script on every loop iteration. This patch
simplifies the script handling by moving the code back to the loop and always breaking in case of different
scripts, correctly handling INHERITED and COMMON cases and updating the current script when needed.

Covered by existing tests. This improves several tests that have been rebaselined.

* platform/graphics/harfbuzz/ComplexTextControllerHarfBuzz.cpp:
(WebCore::findNextRun):
(WebCore::scriptsAreCompatibleForCharacters): Deleted.

Source/WebCore/ChangeLog

@@ -1,3 +1,21 @@
+2017-10-26  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        REGRESSION(r222090): [HarfBuzz] Arabic shaping is broken except for first word in line
+        https://bugs.webkit.org/show_bug.cgi?id=178625
+
+        Reviewed by Michael Catanzaro.
+
+        Once we find the first space, which has the COMMON script, we split the run, and the next ones keep using
+        COMMON instead of ARABIC because we don't update the current script on every loop iteration. This patch
+        simplifies the script handling by moving the code back to the loop and always breaking in case of different
+        scripts, correctly handling INHERITED and COMMON cases and updating the current script when needed.
+
+        Covered by existing tests. This improves several tests that have been rebaselined.
+
+        * platform/graphics/harfbuzz/ComplexTextControllerHarfBuzz.cpp:
+        (WebCore::findNextRun):
+        (WebCore::scriptsAreCompatibleForCharacters): Deleted.
+
 2017-10-17  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         [GTK][Stable] BackingStoreBackendCairoImpl.h:23:10: fatal error: WebCore/HysteresisActivity.h

Source/WebCore/platform/graphics/harfbuzz/HarfBuzzShaper.cpp

@@ -364,33 +364,6 @@ FloatPoint HarfBuzzShaper::adjustStartPoint(const FloatPoint& point)
     return point + m_startOffset;
 }
 
-static bool scriptsAreCompatibleForCharacters(UScriptCode script, UScriptCode previousScript, UChar32 character, UChar32 previousCharacter)
-{
-    if (script == previousScript)
-        return true;
-
-    if (script == USCRIPT_INHERITED || previousScript == USCRIPT_COMMON)
-        return true;
-
-    if (script == USCRIPT_COMMON) {
-        // §5.1 Handling Characters with the Common Script Property.
-        // Programs must resolve any of the special Script property values, such as Common,
-        // based on the context of the surrounding characters. A simple heuristic uses the
-        // script of the preceding character, which works well in many cases.
-        // http://www.unicode.org/reports/tr24/#Common.
-        //
-        // FIXME: cover all other cases mentioned in the spec (ie. brackets or quotation marks).
-        // https://bugs.webkit.org/show_bug.cgi?id=177003.
-        //
-        // We use a slightly more conservative heuristic than the one proposed in the spec,
-        // using the script of the previous character only if both are ASCII.
-        if (isASCII(character) && isASCII(previousCharacter))
-            return true;
-    }
-
-    return uscript_hasScript(character, previousScript);
-}
-
 bool HarfBuzzShaper::collectHarfBuzzRuns()
 {
     const UChar* normalizedBufferEnd = m_normalizedBuffer.get() + m_normalizedBufferLength;
@@ -413,7 +386,6 @@ bool HarfBuzzShaper::collectHarfBuzzRuns()
         if (!currentFontData)
             currentFontData = &m_font->primaryFont();
         UScriptCode currentScript = nextScript;
-        UChar32 previousCharacter = character;
 
         for (iterator.advance(clusterLength); iterator.consume(character, clusterLength); iterator.advance(clusterLength)) {
             if (FontCascade::treatAsZeroWidthSpace(character))
@@ -447,14 +419,31 @@ bool HarfBuzzShaper::collectHarfBuzzRuns()
             if (nextFontData != currentFontData)
                 break;
 
-            if (!scriptsAreCompatibleForCharacters(nextScript, currentScript, character, previousCharacter))
-                break;
+            // §5.1 Handling Characters with the Common Script Property.
+            // Programs must resolve any of the special Script property values, such as Common,
+            // based on the context of the surrounding characters. A simple heuristic uses the
+            // script of the preceding character, which works well in many cases.
+            // http://www.unicode.org/reports/tr24/#Common.
+            //
+            // FIXME: cover all other cases mentioned in the spec (ie. brackets or quotation marks).
+            // https://bugs.webkit.org/show_bug.cgi?id=177003.
+            //
+            // If next script is inherited or common, keep using the current script.
+            if (nextScript == USCRIPT_INHERITED || nextScript == USCRIPT_COMMON) {
+                currentCharacterPosition = iterator.characters();
+                continue;
+            }
+            // If current script is inherited or common, set the next script as current.
+            if (currentScript == USCRIPT_INHERITED || currentScript == USCRIPT_COMMON) {
+                currentScript = nextScript;
+                currentCharacterPosition = iterator.characters();
+                continue;
+            }
 
-            if (nextScript == USCRIPT_INHERITED || nextScript == USCRIPT_COMMON)
-                nextScript = currentScript;
+            if (currentScript != nextScript && !uscript_hasScript(character, currentScript))
+                break;
 
             currentCharacterPosition = iterator.characters();
-            previousCharacter = character;
         }
         unsigned numCharactersOfCurrentRun = iterator.currentIndex() - startIndexOfCurrentRun;
         hb_script_t script = hb_icu_script_to_script(currentScript);