Fix script-related parts of locale resource matching

Previously, a bit was kept to find if the script of a locale was
explicitly "provided" in a resource. This was not backward
compatible, and failed in some edge cases when the package was
created with older versions of AAPT that did not set the bit.

The cases would happen when the old resource had an explicit script
specified in its locale, but since the "provided" bit was not set in
the package, we would assume that the script was computed by us.

This CL replaces the "provided" bit with a "computed" bit, so the
default value of the bit (set to "false" for old packages) would be
correct.

Bug: 27156990
Change-Id: I99e7f1ad8f70c90e25ab3640ed34cc1a6f8d1d64

diff --git a/include/androidfw/ResourceTypes.h b/include/androidfw/ResourceTypes.h
index 16bea79..f23fceb 100644 (file)
--- a/include/androidfw/ResourceTypes.h
+++ b/include/androidfw/ResourceTypes.h
@@ -1151,9 +1151,13 @@ struct ResTable_config
         uint32_t screenConfig2;
     };
 
-    // If true, it means that the script of the locale was explicitly provided.
-    // If false, it means that the script was automatically computed.
-    bool localeScriptWasProvided;
+    // If false and localeScript is set, it means that the script of the locale
+    // was explicitly provided.
+    //
+    // If true, it means that localeScript was automatically computed.
+    // localeScript may still not be set in this case, which means that we
+    // tried but could not compute a script.
+    bool localeScriptWasComputed;
 
     void copyFromDeviceNoSwap(const ResTable_config& o);
     
@@ -1233,7 +1237,7 @@ struct ResTable_config
 
     inline void clearLocale() {
         locale = 0;
-        localeScriptWasProvided = false;
+        localeScriptWasComputed = false;
         memset(localeScript, 0, sizeof(localeScript));
         memset(localeVariant, 0, sizeof(localeVariant));
     }

diff --git a/libs/androidfw/ResourceTypes.cpp b/libs/androidfw/ResourceTypes.cpp
index c73bb58..49b3a51 100644 (file)
--- a/libs/androidfw/ResourceTypes.cpp
+++ b/libs/androidfw/ResourceTypes.cpp
@@ -1870,8 +1870,8 @@ void ResTable_config::swapHtoD() {
 
     // The language & region are equal, so compare the scripts and variants.
     const char emptyScript[sizeof(l.localeScript)] = {'\0', '\0', '\0', '\0'};
-    const char *lScript = l.localeScriptWasProvided ? l.localeScript : emptyScript;
-    const char *rScript = r.localeScriptWasProvided ? r.localeScript : emptyScript;
+    const char *lScript = l.localeScriptWasComputed ? emptyScript : l.localeScript;
+    const char *rScript = r.localeScriptWasComputed ? emptyScript : r.localeScript;
     int script = memcmp(lScript, rScript, sizeof(l.localeScript));
     if (script) {
         return script;
@@ -2016,11 +2016,11 @@ int ResTable_config::isLocaleMoreSpecificThan(const ResTable_config& o) const {
     // scripts since it seems more useful to do so. We will consider
     // "en-US-POSIX" to be more specific than "en-Latn-US".
 
-    const int score = (localeScriptWasProvided ? 1 : 0) +
-        ((localeVariant[0] != 0) ? 2 : 0);
+    const int score = ((localeScript[0] != '\0' && !localeScriptWasComputed) ? 1 : 0) +
+        ((localeVariant[0] != '\0') ? 2 : 0);
 
-    const int oScore = (o.localeScriptWasProvided ? 1 : 0) +
-        ((o.localeVariant[0] != 0) ? 2 : 0);
+    const int oScore = (o.localeScript[0] != '\0' && !o.localeScriptWasComputed ? 1 : 0) +
+        ((o.localeVariant[0] != '\0') ? 2 : 0);
 
     return score - oScore;
 
@@ -2535,7 +2535,8 @@ bool ResTable_config::match(const ResTable_config& settings) const {
         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
+            if (localeScript[0] == '\0' && !localeScriptWasComputed) {
+                // script was not provided or computed, so we try to compute it
                 localeDataComputeScript(computed_script, language, country);
                 if (computed_script[0] == '\0') { // we could not compute the script
                     countriesMustMatch = true;
@@ -2684,8 +2685,8 @@ void ResTable_config::appendDirLocale(String8& out) const {
     if (!language[0]) {
         return;
     }
-
-    if (!localeScriptWasProvided && !localeVariant[0]) {
+    const bool scriptWasProvided = localeScript[0] != '\0' && !localeScriptWasComputed;
+    if (!scriptWasProvided && !localeVariant[0]) {
         // Legacy format.
         if (out.size() > 0) {
             out.append("-");
@@ -2715,7 +2716,7 @@ void ResTable_config::appendDirLocale(String8& out) const {
     size_t len = unpackLanguage(buf);
     out.append(buf, len);
 
-    if (localeScriptWasProvided) {
+    if (scriptWasProvided) {
         out.append("+");
         out.append(localeScript, sizeof(localeScript));
     }
@@ -2746,7 +2747,7 @@ void ResTable_config::getBcp47Locale(char str[RESTABLE_MAX_LOCALE_LEN]) const {
         charsWritten += unpackLanguage(str);
     }
 
-    if (localeScriptWasProvided) {
+    if (localeScript[0] && !localeScriptWasComputed) {
         if (charsWritten) {
             str[charsWritten++] = '-';
         }
@@ -2787,7 +2788,6 @@ void ResTable_config::getBcp47Locale(char str[RESTABLE_MAX_LOCALE_LEN]) const {
                for (size_t i = 1; i < 4; ++i) {
                    config->localeScript[i] = tolower(start[i]);
                }
-               config->localeScriptWasProvided = true;
                break;
            }
        case 5:
@@ -2807,7 +2807,6 @@ void ResTable_config::getBcp47Locale(char str[RESTABLE_MAX_LOCALE_LEN]) const {
 
 void ResTable_config::setBcp47Locale(const char* in) {
     locale = 0;
-    localeScriptWasProvided = false;
     memset(localeScript, 0, sizeof(localeScript));
     memset(localeVariant, 0, sizeof(localeVariant));
 
@@ -2824,9 +2823,10 @@ void ResTable_config::setBcp47Locale(const char* in) {
 
     const size_t size = in + strlen(in) - start;
     assignLocaleComponent(this, start, size);
-    if (localeScript[0] == '\0') {
+    localeScriptWasComputed = (localeScript[0] == '\0');
+    if (localeScriptWasComputed) {
         computeScript();
-    };
+    }
 }
 
 String8 ResTable_config::toString() const {

diff --git a/libs/androidfw/tests/ConfigLocale_test.cpp b/libs/androidfw/tests/ConfigLocale_test.cpp
index 4b8d65c..2bf9b12 100644 (file)
--- a/libs/androidfw/tests/ConfigLocale_test.cpp
+++ b/libs/androidfw/tests/ConfigLocale_test.cpp
@@ -125,10 +125,10 @@ TEST(ConfigLocaleTest, packAndUnpack3LetterRegion) {
 
      if (script != NULL) {
          memcpy(out->localeScript, script, 4);
-         out->localeScriptWasProvided = true;
+         out->localeScriptWasComputed = false;
      } else {
          out->computeScript();
-         out->localeScriptWasProvided = false;
+         out->localeScriptWasComputed = true;
      }
 
      if (variant != NULL) {
@@ -182,7 +182,7 @@ TEST(ConfigLocaleTest, setLocale) {
     EXPECT_EQ('n', test.language[1]);
     EXPECT_EQ('U', test.country[0]);
     EXPECT_EQ('S', test.country[1]);
-    EXPECT_FALSE(test.localeScriptWasProvided);
+    EXPECT_TRUE(test.localeScriptWasComputed);
     EXPECT_EQ(0, memcmp("Latn", test.localeScript, 4));
     EXPECT_EQ(0, test.localeVariant[0]);
 
@@ -203,7 +203,7 @@ TEST(ConfigLocaleTest, setLocale) {
     EXPECT_EQ('e', test.language[0]);
     EXPECT_EQ('n', test.language[1]);
     EXPECT_EQ(0, memcmp("Latn", test.localeScript, 4));
-    EXPECT_TRUE(test.localeScriptWasProvided);
+    EXPECT_FALSE(test.localeScriptWasComputed);
     memset(out, 1, 4);
     test.unpackRegion(out);
     EXPECT_EQ('4', out[0]);
@@ -216,7 +216,7 @@ TEST(ConfigLocaleTest, setLocale) {
     EXPECT_EQ('d', out[0]);
     EXPECT_EQ('e', out[1]);
     EXPECT_EQ('\0', out[2]);
-    EXPECT_FALSE(test.localeScriptWasProvided);
+    EXPECT_TRUE(test.localeScriptWasComputed);
     EXPECT_EQ(0, memcmp("Latn", test.localeScript, 4));
     memset(out, 1, 4);
     test.unpackRegion(out);
@@ -229,7 +229,7 @@ TEST(ConfigLocaleTest, setLocale) {
     EXPECT_EQ('d', out[0]);
     EXPECT_EQ('e', out[1]);
     EXPECT_EQ('\0', out[2]);
-    EXPECT_TRUE(test.localeScriptWasProvided);
+    EXPECT_FALSE(test.localeScriptWasComputed);
     EXPECT_EQ(0, memcmp("Latn", test.localeScript, 4));
     memset(out, 1, 4);
     test.unpackRegion(out);
@@ -270,11 +270,11 @@ TEST(ConfigLocaleTest, getBcp47Locale_script) {
     fillIn("en", NULL, "Latn", NULL, &config);
 
     char out[RESTABLE_MAX_LOCALE_LEN];
-    config.localeScriptWasProvided = true;
+    config.localeScriptWasComputed = false;
     config.getBcp47Locale(out);
     EXPECT_EQ(0, strcmp("en-Latn", out));
 
-    config.localeScriptWasProvided = false;
+    config.localeScriptWasComputed = true;
     config.getBcp47Locale(out);
     EXPECT_EQ(0, strcmp("en", out));
 }
@@ -379,7 +379,7 @@ TEST(ConfigLocaleTest, match_emptyScript) {
 
     // emulate packages built with older AAPT
     memset(supported.localeScript, '\0', 4);
-    supported.localeScriptWasProvided = false;
+    supported.localeScriptWasComputed = false;
 
     EXPECT_TRUE(supported.match(requested));
 }

diff --git a/tools/aapt/AaptAssets.cpp b/tools/aapt/AaptAssets.cpp
index 3b01827..cbd8480 100644 (file)
--- a/tools/aapt/AaptAssets.cpp
+++ b/tools/aapt/AaptAssets.cpp
@@ -373,7 +373,7 @@ int AaptLocaleValue::initFromDirName(const Vector<String8>& parts, const int sta
 void AaptLocaleValue::initFromResTable(const ResTable_config& config) {
     config.unpackLanguage(language);
     config.unpackRegion(region);
-    if (config.localeScriptWasProvided) {
+    if (config.localeScript[0] && !config.localeScriptWasComputed) {
         memcpy(script, config.localeScript, sizeof(config.localeScript));
     }
 
@@ -388,10 +388,10 @@ void AaptLocaleValue::writeTo(ResTable_config* out) const {
 
     if (script[0]) {
         memcpy(out->localeScript, script, sizeof(out->localeScript));
-        out->localeScriptWasProvided = true;
+        out->localeScriptWasComputed = false;
     } else {
         out->computeScript();
-        out->localeScriptWasProvided = false;
+        out->localeScriptWasComputed = true;
     }
 
     if (variant[0]) {

diff --git a/tools/aapt2/Locale.cpp b/tools/aapt2/Locale.cpp
index 6acf3b0..12f56fc 100644 (file)
--- a/tools/aapt2/Locale.cpp
+++ b/tools/aapt2/Locale.cpp
@@ -253,7 +253,7 @@ std::string LocaleValue::toDirName() const {
 void LocaleValue::initFromResTable(const ResTable_config& config) {
     config.unpackLanguage(language);
     config.unpackRegion(region);
-    if (config.localeScriptWasProvided) {
+    if (config.localeScript[0] && !config.localeScriptWasComputed) {
         memcpy(script, config.localeScript, sizeof(config.localeScript));
     }
 
@@ -268,10 +268,10 @@ void LocaleValue::writeTo(ResTable_config* out) const {
 
     if (script[0]) {
         memcpy(out->localeScript, script, sizeof(out->localeScript));
-        out->localeScriptWasProvided = true;
+        out->localeScriptWasComputed = false;
     } else {
         out->computeScript();
-        out->localeScriptWasProvided = false;
+        out->localeScriptWasComputed = true;
     }
 
     if (variant[0]) {