Android patch: ICU52 layout engine bug fixes.

This change was introduced in Android by the ICU52 upgrade:

https://android.googlesource.com/platform/external/icu/+/59d709d

Change-Id: I8d8ed50e1e22dc9dfe7792135626590626c7c820

6 files changed, 19 insertions, 9 deletions

diff --git a/icu4c/source/layout/ContextualSubstSubtables.cpp b/icu4c/source/layout/ContextualSubstSubtables.cpp
index 4c066f1e8..0e484d951 100644
--- a/icu4c/source/layout/ContextualSubstSubtables.cpp
+++ b/icu4c/source/layout/ContextualSubstSubtables.cpp
@@ -128,6 +128,7 @@ le_bool ContextualSubstitutionBase::matchGlyphCoverages(const Offset *coverageTa
 
     while (glyphCount > 0) {
         Offset coverageTableOffset = SWAPW(coverageTableOffsetArray[glyph]);
+        /* Google patch: Behdad says: Unsafe dereference follows. */
         const CoverageTable *coverageTable = (const CoverageTable *) (offsetBase + coverageTableOffset);
 
         if (! glyphIterator->next()) {
@@ -159,6 +160,8 @@ le_uint32 ContextualSubstitutionSubtable::process(const LookupProcessor *lookupP
     case 0:
         return 0;
 
+    /* Google patch: Behdad says: Unsafe downcasts follow. */
+
     case 1:
     {
         const ContextualSubstitutionFormat1Subtable *subtable = (const ContextualSubstitutionFormat1Subtable *) this;
@@ -199,6 +202,7 @@ le_uint32 ContextualSubstitutionFormat1Subtable::process(const LookupProcessor *
 
         if (coverageIndex < srSetCount) {
             Offset subRuleSetTableOffset = SWAPW(subRuleSetTableOffsetArray[coverageIndex]);
+            /* Google patch: Behdad says: Unsafe dereference follows. */
             const SubRuleSetTable *subRuleSetTable =
                 (const SubRuleSetTable *) ((char *) this + subRuleSetTableOffset);
             le_uint16 subRuleCount = SWAPW(subRuleSetTable->subRuleCount);
@@ -244,6 +248,7 @@ le_uint32 ContextualSubstitutionFormat2Subtable::process(const LookupProcessor *
     le_int32 coverageIndex = getGlyphCoverage(lookupProcessor->getReference(), glyph, success);
 
     if (coverageIndex >= 0) {
+        /* Google patch: Behdad says: Unsafe dereference follows. */
         const ClassDefinitionTable *classDefinitionTable =
             (const ClassDefinitionTable *) ((char *) this + SWAPW(classDefTableOffset));
         le_uint16 scSetCount = SWAPW(subClassSetCount);
@@ -251,6 +256,7 @@ le_uint32 ContextualSubstitutionFormat2Subtable::process(const LookupProcessor *
 
         if (setClass < scSetCount && subClassSetTableOffsetArray[setClass] != 0) {
             Offset subClassSetTableOffset = SWAPW(subClassSetTableOffsetArray[setClass]);
+            /* Google patch: Behdad says: Unsafe dereference follows. */
             const SubClassSetTable *subClassSetTable =
                 (const SubClassSetTable *) ((char *) this + subClassSetTableOffset);
             le_uint16 subClassRuleCount = SWAPW(subClassSetTable->subClassRuleCount);
@@ -330,6 +336,8 @@ le_uint32 ChainingContextualSubstitutionSubtable::process(const LookupProcessor
     case 0:
         return 0;
 
+    /* Google patch: Behdad says: Unsafe downcasts follow. */
+
     case 1:
     {
         const ChainingContextualSubstitutionFormat1Subtable *subtable = (const ChainingContextualSubstitutionFormat1Subtable *) this;
@@ -376,6 +384,7 @@ le_uint32 ChainingContextualSubstitutionFormat1Subtable::process(const LookupPro
 
         if (coverageIndex < srSetCount) {
             Offset chainSubRuleSetTableOffset = SWAPW(chainSubRuleSetTableOffsetArray[coverageIndex]);
+            /* Google patch: Behdad says: Unsafe dereference follows. */
             const ChainSubRuleSetTable *chainSubRuleSetTable =
                 (const ChainSubRuleSetTable *) ((char *) this + chainSubRuleSetTableOffset);
             le_uint16 chainSubRuleCount = SWAPW(chainSubRuleSetTable->chainSubRuleCount);
@@ -443,6 +452,7 @@ le_uint32 ChainingContextualSubstitutionFormat2Subtable::process(const LookupPro
     le_int32 coverageIndex = getGlyphCoverage(lookupProcessor->getReference(), glyph, success);
 
     if (coverageIndex >= 0) {
+        /* Google patch: Behdad says: Unsafe dereferences follow. */
         const ClassDefinitionTable *backtrackClassDefinitionTable =
             (const ClassDefinitionTable *) ((char *) this + SWAPW(backtrackClassDefTableOffset));
         const ClassDefinitionTable *inputClassDefinitionTable =
@@ -454,6 +464,7 @@ le_uint32 ChainingContextualSubstitutionFormat2Subtable::process(const LookupPro
 
         if (setClass < scSetCount && chainSubClassSetTableOffsetArray[setClass] != 0) {
             Offset chainSubClassSetTableOffset = SWAPW(chainSubClassSetTableOffsetArray[setClass]);
+            /* Google patch: Behdad says: Unsafe dereference follows. */
             const ChainSubClassSetTable *chainSubClassSetTable =
                 (const ChainSubClassSetTable *) ((char *) this + chainSubClassSetTableOffset);
             le_uint16 chainSubClassRuleCount = SWAPW(chainSubClassSetTable->chainSubClassRuleCount);
diff --git a/icu4c/source/layout/ExtensionSubtables.cpp b/icu4c/source/layout/ExtensionSubtables.cpp
index 8ac03a2bb..99be1cedc 100644
--- a/icu4c/source/layout/ExtensionSubtables.cpp
+++ b/icu4c/source/layout/ExtensionSubtables.cpp
@@ -23,11 +23,10 @@ static inline le_uint32 READ_LONG(le_uint32 code) {
 }
 
 // FIXME: should look at the format too... maybe have a sub-class for it?
-le_uint32 ExtensionSubtable::process(const LookupProcessor *lookupProcessor, le_uint16 lookupType,
+le_uint32 ExtensionSubtable::process(const LookupProcessor *lookupProcessor, const LETableReference &base, le_uint16 lookupType,  // Google patch: add base
                                       GlyphIterator *glyphIterator, const LEFontInstance *fontInstance, LEErrorCode& success) const
 {
-    const LEReferenceTo<ExtensionSubtable> thisRef(lookupProcessor->getReference(), success); // create a reference to this
-
+    // Google patch: remove thisRef
     if (LE_FAILURE(success)) {
         return 0;
     }
@@ -36,7 +35,7 @@ le_uint32 ExtensionSubtable::process(const LookupProcessor *lookupProcessor, le_
 
     if (elt != lookupType) {      
         le_uint32 extOffset = READ_LONG(extensionOffset);
-        LEReferenceTo<LookupSubtable> subtable(thisRef, success, extOffset);
+        LEReferenceTo<LookupSubtable> subtable(base, success, extOffset);  // Google patch: s/thisRef/base/
 
         if(LE_SUCCESS(success)) {
           return lookupProcessor->applySubtable(subtable, elt, glyphIterator, fontInstance, success);
diff --git a/icu4c/source/layout/ExtensionSubtables.h b/icu4c/source/layout/ExtensionSubtables.h
index 7d214af5e..825b4ac93 100644
--- a/icu4c/source/layout/ExtensionSubtables.h
+++ b/icu4c/source/layout/ExtensionSubtables.h
@@ -27,7 +27,7 @@ struct ExtensionSubtable //: GlyphSubstitutionSubtable
     le_uint16 extensionLookupType;
     le_uint32 extensionOffset;
 
-    le_uint32 process(const LookupProcessor *lookupProcessor, le_uint16 lookupType,
+    le_uint32 process(const LookupProcessor *lookupProcessor, const LETableReference &base, le_uint16 lookupType,  // Google patch: add base
                       GlyphIterator *glyphIterator, const LEFontInstance *fontInstance, LEErrorCode& success) const;
 };
 
diff --git a/icu4c/source/layout/GlyphPosnLookupProc.cpp b/icu4c/source/layout/GlyphPosnLookupProc.cpp
index 76bdc79aa..363f77304 100644
--- a/icu4c/source/layout/GlyphPosnLookupProc.cpp
+++ b/icu4c/source/layout/GlyphPosnLookupProc.cpp
@@ -143,7 +143,7 @@ le_uint32 GlyphPositioningLookupProcessor::applySubtable(const LEReferenceTo<Loo
     {
         LEReferenceTo<ExtensionSubtable> subtable(lookupSubtable, success);
 
-        delta = subtable->process(this, lookupType, glyphIterator, fontInstance, success);
+        delta = subtable->process(this, subtable, lookupType, glyphIterator, fontInstance, success);  // Google patch: add subtable
         break;
     }
 
diff --git a/icu4c/source/layout/GlyphSubstLookupProc.cpp b/icu4c/source/layout/GlyphSubstLookupProc.cpp
index 8698b3414..3deb3d959 100644
--- a/icu4c/source/layout/GlyphSubstLookupProc.cpp
+++ b/icu4c/source/layout/GlyphSubstLookupProc.cpp
@@ -114,7 +114,7 @@ le_uint32 GlyphSubstitutionLookupProcessor::applySubtable(const LEReferenceTo<Lo
     {
         const LEReferenceTo<ExtensionSubtable> subtable(lookupSubtable, success);
 
-        delta = subtable->process(this, lookupType, glyphIterator, fontInstance, success);
+        delta = subtable->process(this, subtable, lookupType, glyphIterator, fontInstance, success);  // Google patch: add subtable
         break;
     }
 
diff --git a/icu4c/source/layout/LookupProcessor.cpp b/icu4c/source/layout/LookupProcessor.cpp
index 43b8e44cd..401093816 100644
--- a/icu4c/source/layout/LookupProcessor.cpp
+++ b/icu4c/source/layout/LookupProcessor.cpp
@@ -36,7 +36,7 @@ le_uint32 LookupProcessor::applyLookupTable(const LEReferenceTo<LookupTable> &lo
 
         delta = applySubtable(lookupSubtable, lookupType, glyphIterator, fontInstance, success);
 
-        if (delta > 0 && LE_FAILURE(success)) {
+        if (delta > 0 || LE_FAILURE(success)) {  // Google patch: && -> ||
             return 1;
         }
 
@@ -100,7 +100,7 @@ le_uint32 LookupProcessor::applySingleLookup(le_uint16 lookupTableIndex, GlyphIt
 
     const LEReferenceTo<LookupTable> lookupTable = lookupListTable->getLookupTable(lookupListTable, lookupTableIndex, success);
     if (!lookupTable.isValid()) {
-        success = LE_INTERNAL_ERROR;
+        // Google patch: remove  success = LE_INTERNAL_ERROR;  (is this right?)
         return 0;
     }
     le_uint16 lookupFlags = SWAPW(lookupTable->lookupFlags);