From 7960898f6f886aadda1dd3d7a89106dde9f44217 Mon Sep 17 00:00:00 2001 From: Roozbeh Pournader Date: Thu, 3 Mar 2016 15:06:46 -0800 Subject: [PATCH] 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 --- include/androidfw/ResourceTypes.h | 12 ++++++--- libs/androidfw/ResourceTypes.cpp | 30 +++++++++++----------- libs/androidfw/tests/ConfigLocale_test.cpp | 18 ++++++------- tools/aapt/AaptAssets.cpp | 6 ++--- tools/aapt2/Locale.cpp | 6 ++--- 5 files changed, 38 insertions(+), 34 deletions(-) diff --git a/include/androidfw/ResourceTypes.h b/include/androidfw/ResourceTypes.h index 16bea79941771..f23fceb8f82de 100644 --- 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 c73bb584be3a9..49b3a51e705b5 100644 --- 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 4b8d65cf86b47..2bf9b12b6ce5c 100644 --- 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 3b01827e13d8d..cbd8480efc120 100644 --- a/tools/aapt/AaptAssets.cpp +++ b/tools/aapt/AaptAssets.cpp @@ -373,7 +373,7 @@ int AaptLocaleValue::initFromDirName(const Vector& 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 6acf3b09d3017..12f56fc11226b 100644 --- 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]) {