From 4de4596ac1175908657f29cdc2a56529f2ca4b25 Mon Sep 17 00:00:00 2001
From: Roozbeh Pournader <roozbeh@google.com>
Date: Thu, 11 Feb 2016 17:58:24 -0800
Subject: [PATCH] Fix locale matching algorithm for resources

We get ResTables two different ways: one is from AAPT, another from
settings-based requests from the Java side. In the settings-based
requests, localeScript will be autocomputed, but for AAPT-filled
tables (especially if they come from older versions of AAPT), we need
to compute the script.

Previously, locales that came from packages were incorrectly assumed
to have "undeterminable" scripts, rather than "undetermined" scripts.
This led to us mistakenly falling back to the old logic of requiring
the locales' countries to match, rather than just looking at computed
scripts.

Bug: 27157452

Change-Id: Id7e346d3ecfb17273ffb63de5bcb4849a6eafbbd
---
 libs/androidfw/ResourceTypes.cpp           | 29 +++++++++++++++++-----
 libs/androidfw/tests/ConfigLocale_test.cpp | 13 ++++++++++
 2 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/libs/androidfw/ResourceTypes.cpp b/libs/androidfw/ResourceTypes.cpp
index 1d9fe35bac35a..3277c36c8a332 100644
--- a/libs/androidfw/ResourceTypes.cpp
+++ b/libs/androidfw/ResourceTypes.cpp
@@ -2236,7 +2236,7 @@ bool ResTable_config::isLocaleBetterThan(const ResTable_config& o,
     // See if any of the regions is better than the other
     const int region_comparison = localeDataCompareRegions(
             country, o.country,
-            language, localeScript, requested->country);
+            language, requested->localeScript, requested->country);
     if (region_comparison != 0) {
         return (region_comparison > 0);
     }
@@ -2526,17 +2526,34 @@ bool ResTable_config::match(const ResTable_config& settings) const {
 
         // For backward compatibility and supporting private-use locales, we
         // fall back to old behavior if we couldn't determine the script for
-        // either of the desired locale or the provided locale.
-        if (localeScript[0] == '\0' || localeScript[1] == '\0') {
+        // either of the desired locale or the provided locale. But if we could determine
+        // the scripts, they should be the same for the locales to match.
+        bool countriesMustMatch = false;
+        char computed_script[4];
+        const char* script;
+        if (settings.localeScript[0] == '\0') { // could not determine the request's script
+            countriesMustMatch = true;
+        } else {
+            if (localeScript[0] == '\0') { // script was not provided, so we try to compute it
+                localeDataComputeScript(computed_script, language, country);
+                if (computed_script[0] == '\0') { // we could not compute the script
+                    countriesMustMatch = true;
+                } else {
+                    script = computed_script;
+                }
+            } else { // script was provided, so just use it
+                script = localeScript;
+            }
+        }
+
+        if (countriesMustMatch) {
             if (country[0] != '\0'
                 && (country[0] != settings.country[0]
                     || country[1] != settings.country[1])) {
                 return false;
             }
         } else {
-            // But if we could determine the scripts, they should be the same
-            // for the locales to match.
-            if (memcmp(localeScript, settings.localeScript, sizeof(localeScript)) != 0) {
+            if (memcmp(script, settings.localeScript, sizeof(settings.localeScript)) != 0) {
                 return false;
             }
         }
diff --git a/libs/androidfw/tests/ConfigLocale_test.cpp b/libs/androidfw/tests/ConfigLocale_test.cpp
index 7b386404fe8cc..4b8d65cf86b47 100644
--- a/libs/androidfw/tests/ConfigLocale_test.cpp
+++ b/libs/androidfw/tests/ConfigLocale_test.cpp
@@ -371,6 +371,19 @@ TEST(ConfigLocaleTest, match) {
     EXPECT_TRUE(supported.match(requested));
 }
 
+TEST(ConfigLocaleTest, match_emptyScript) {
+    ResTable_config supported, requested;
+
+    fillIn("fr", "FR", NULL, NULL, &supported);
+    fillIn("fr", "CA", NULL, NULL, &requested);
+
+    // emulate packages built with older AAPT
+    memset(supported.localeScript, '\0', 4);
+    supported.localeScriptWasProvided = false;
+
+    EXPECT_TRUE(supported.match(requested));
+}
+
 TEST(ConfigLocaleTest, isLocaleBetterThan_basics) {
     ResTable_config config1, config2, request;