diff options
30 files changed, 746 insertions, 331 deletions
diff --git a/tools/aapt2/Android.mk b/tools/aapt2/Android.mk index 88b6270fad60..cb82ac37a3b2 100644 --- a/tools/aapt2/Android.mk +++ b/tools/aapt2/Android.mk @@ -38,6 +38,7 @@ sources := \ io/ZipArchive.cpp \ link/AutoVersioner.cpp \ link/ManifestFixer.cpp \ + link/ProductFilter.cpp \ link/PrivateAttributeMover.cpp \ link/ReferenceLinker.cpp \ link/TableMerger.cpp \ @@ -83,6 +84,7 @@ testSources := \ link/AutoVersioner_test.cpp \ link/ManifestFixer_test.cpp \ link/PrivateAttributeMover_test.cpp \ + link/ProductFilter_test.cpp \ link/ReferenceLinker_test.cpp \ link/TableMerger_test.cpp \ link/XmlReferenceLinker_test.cpp \ diff --git a/tools/aapt2/Debug.cpp b/tools/aapt2/Debug.cpp index b4e75f9be3a9..4bea12973692 100644 --- a/tools/aapt2/Debug.cpp +++ b/tools/aapt2/Debug.cpp @@ -144,8 +144,8 @@ void Debug::printTable(ResourceTable* table) { PrintVisitor visitor; for (const auto& value : entry->values) { - std::cout << " (" << value.config << ") "; - value.value->accept(&visitor); + std::cout << " (" << value->config << ") "; + value->value->accept(&visitor); std::cout << std::endl; } } @@ -176,7 +176,7 @@ void Debug::printStyleGraph(ResourceTable* table, const ResourceName& targetStyl if (result) { ResourceEntry* entry = result.value().entry; for (const auto& value : entry->values) { - if (Style* style = valueCast<Style>(value.value.get())) { + if (Style* style = valueCast<Style>(value->value.get())) { if (style->parent && style->parent.value().name) { parents.insert(style->parent.value().name.value()); stylesToVisit.push(style->parent.value().name.value()); diff --git a/tools/aapt2/ResourceParser.cpp b/tools/aapt2/ResourceParser.cpp index b37d366a7887..b100e84f8a46 100644 --- a/tools/aapt2/ResourceParser.cpp +++ b/tools/aapt2/ResourceParser.cpp @@ -19,7 +19,6 @@ #include "ResourceUtils.h" #include "ResourceValues.h" #include "ValueVisitor.h" -#include "util/Comparators.h" #include "util/ImmutableMap.h" #include "util/Util.h" #include "xml/XmlPullParser.h" @@ -71,6 +70,7 @@ static uint32_t parseFormatAttribute(const StringPiece16& str) { struct ParsedResource { ResourceName name; ConfigDescription config; + std::string product; Source source; ResourceId id; Maybe<SymbolState> symbolState; @@ -79,35 +79,6 @@ struct ParsedResource { std::list<ParsedResource> childResources; }; -bool ResourceParser::shouldStripResource(const ResourceNameRef& name, - const Maybe<std::u16string>& product) const { - if (product) { - for (const std::u16string& productToMatch : mOptions.products) { - if (product.value() == productToMatch) { - // We specified a product, and it is in the list, so don't strip. - return false; - } - } - } - - // Nothing matched, try 'default'. Default only matches if we didn't already use another - // product variant. - if (!product || product.value() == u"default") { - if (Maybe<ResourceTable::SearchResult> result = mTable->findResource(name)) { - const ResourceEntry* entry = result.value().entry; - auto iter = std::lower_bound(entry->values.begin(), entry->values.end(), mConfig, - cmp::lessThanConfig); - if (iter != entry->values.end() && iter->config == mConfig && !iter->value->isWeak()) { - // We have a value for this config already, and it is not weak, - // so filter out this default. - return true; - } - } - return false; - } - return true; -} - // Recursively adds resources to the ResourceTable. static bool addResourcesToTable(ResourceTable* table, IDiagnostics* diag, ParsedResource* res) { if (res->symbolState) { @@ -125,7 +96,8 @@ static bool addResourcesToTable(ResourceTable* table, IDiagnostics* diag, Parsed res->value->setComment(std::move(res->comment)); res->value->setSource(std::move(res->source)); - if (!table->addResource(res->name, res->id, res->config, std::move(res->value), diag)) { + if (!table->addResource(res->name, res->id, res->config, res->product, + std::move(res->value), diag)) { return false; } } @@ -295,9 +267,8 @@ bool ResourceParser::parseResources(xml::XmlPullParser* parser) { parsedResource.comment = std::move(comment); // Extract the product name if it exists. - Maybe<std::u16string> product; if (Maybe<StringPiece16> maybeProduct = xml::findNonEmptyAttribute(parser, u"product")) { - product = maybeProduct.value().toString(); + parsedResource.product = util::utf16ToUtf8(maybeProduct.value()); } // Parse the resource regardless of product. @@ -306,12 +277,7 @@ bool ResourceParser::parseResources(xml::XmlPullParser* parser) { continue; } - // We successfully parsed the resource. Check if we should include it or strip it. - if (shouldStripResource(parsedResource.name, product)) { - // Record that we stripped out this resource name. - // We will check that at least one variant of this resource was included. - strippedResources.insert(parsedResource.name); - } else if (!addResourcesToTable(mTable, mDiag, &parsedResource)) { + if (!addResourcesToTable(mTable, mDiag, &parsedResource)) { error = true; } } @@ -524,7 +490,7 @@ std::unique_ptr<Item> ResourceParser::parseXml(xml::XmlPullParser* parser, const // name.package can be empty here, as it will assume the package name of the table. std::unique_ptr<Id> id = util::make_unique<Id>(); id->setSource(mSource.withLine(beginXmlLine)); - mTable->addResource(name, {}, std::move(id), mDiag); + mTable->addResource(name, {}, {}, std::move(id), mDiag); }; // Process the raw value. diff --git a/tools/aapt2/ResourceParser.h b/tools/aapt2/ResourceParser.h index 51cbbe19384e..ee5b33788312 100644 --- a/tools/aapt2/ResourceParser.h +++ b/tools/aapt2/ResourceParser.h @@ -34,13 +34,6 @@ struct ParsedResource; struct ResourceParserOptions { /** - * Optional product names by which to filter resources. - * This is like a preprocessor definition in that we strip out resources - * that don't match before we compile them. - */ - std::vector<std::u16string> products; - - /** * Whether the default setting for this parser is to allow translation. */ bool translatable = true; @@ -106,9 +99,6 @@ private: bool parseArrayImpl(xml::XmlPullParser* parser, ParsedResource* outResource, uint32_t typeMask); bool parsePlural(xml::XmlPullParser* parser, ParsedResource* outResource); - bool shouldStripResource(const ResourceNameRef& name, - const Maybe<std::u16string>& product) const; - IDiagnostics* mDiag; ResourceTable* mTable; Source mSource; diff --git a/tools/aapt2/ResourceParser_test.cpp b/tools/aapt2/ResourceParser_test.cpp index cf0fcd11b903..3450de9078bb 100644 --- a/tools/aapt2/ResourceParser_test.cpp +++ b/tools/aapt2/ResourceParser_test.cpp @@ -48,24 +48,13 @@ struct ResourceParserTest : public ::testing::Test { } ::testing::AssertionResult testParse(const StringPiece& str) { - return testParse(str, ConfigDescription{}, {}); + return testParse(str, ConfigDescription{}); } ::testing::AssertionResult testParse(const StringPiece& str, const ConfigDescription& config) { - return testParse(str, config, {}); - } - - ::testing::AssertionResult testParse(const StringPiece& str, - std::initializer_list<std::u16string> products) { - return testParse(str, {}, std::move(products)); - } - - ::testing::AssertionResult testParse(const StringPiece& str, const ConfigDescription& config, - std::initializer_list<std::u16string> products) { std::stringstream input(kXmlPreamble); input << "<resources>\n" << str << "\n</resources>" << std::endl; ResourceParserOptions parserOptions; - parserOptions.products = products; ResourceParser parser(mContext->getDiagnostics(), &mTable, Source{ "test" }, config, parserOptions); xml::XmlPullParser xmlParser(input); @@ -546,7 +535,7 @@ TEST_F(ResourceParserTest, ParsePublicIdAsDefinition) { ASSERT_NE(nullptr, id); } -TEST_F(ResourceParserTest, FilterProductsThatDontMatch) { +TEST_F(ResourceParserTest, KeepAllProducts) { std::string input = R"EOF( <string name="foo" product="phone">hi</string> <string name="foo" product="no-sdcard">ho</string> @@ -555,33 +544,26 @@ TEST_F(ResourceParserTest, FilterProductsThatDontMatch) { <string name="bit" product="phablet">hoot</string> <string name="bot" product="default">yes</string> )EOF"; - ASSERT_TRUE(testParse(input, { std::u16string(u"no-sdcard"), std::u16string(u"phablet") })); - - String* fooStr = test::getValue<String>(&mTable, u"@string/foo"); - ASSERT_NE(nullptr, fooStr); - EXPECT_EQ(StringPiece16(u"ho"), *fooStr->value); - - EXPECT_NE(nullptr, test::getValue<String>(&mTable, u"@string/bar")); - EXPECT_NE(nullptr, test::getValue<String>(&mTable, u"@string/baz")); - EXPECT_NE(nullptr, test::getValue<String>(&mTable, u"@string/bit")); - EXPECT_NE(nullptr, test::getValue<String>(&mTable, u"@string/bot")); -} - -TEST_F(ResourceParserTest, FilterProductsThatBothMatchInOrder) { - std::string input = R"EOF( - <string name="foo" product="phone">phone</string> - <string name="foo" product="default">default</string> - )EOF"; - ASSERT_TRUE(testParse(input, { std::u16string(u"phone") })); - - String* foo = test::getValue<String>(&mTable, u"@string/foo"); - ASSERT_NE(nullptr, foo); - EXPECT_EQ(std::u16string(u"phone"), *foo->value); -} + ASSERT_TRUE(testParse(input)); -TEST_F(ResourceParserTest, FailWhenProductFilterStripsOutAllVersionsOfResource) { - std::string input = "<string name=\"foo\" product=\"tablet\">hello</string>\n"; - ASSERT_FALSE(testParse(input, { std::u16string(u"phone") })); + EXPECT_NE(nullptr, test::getValueForConfigAndProduct<String>(&mTable, u"@string/foo", + ConfigDescription::defaultConfig(), + "phone")); + EXPECT_NE(nullptr, test::getValueForConfigAndProduct<String>(&mTable, u"@string/foo", + ConfigDescription::defaultConfig(), + "no-sdcard")); + EXPECT_NE(nullptr, test::getValueForConfigAndProduct<String>(&mTable, u"@string/bar", + ConfigDescription::defaultConfig(), + "")); + EXPECT_NE(nullptr, test::getValueForConfigAndProduct<String>(&mTable, u"@string/baz", + ConfigDescription::defaultConfig(), + "")); + EXPECT_NE(nullptr, test::getValueForConfigAndProduct<String>(&mTable, u"@string/bit", + ConfigDescription::defaultConfig(), + "phablet")); + EXPECT_NE(nullptr, test::getValueForConfigAndProduct<String>(&mTable, u"@string/bot", + ConfigDescription::defaultConfig(), + "default")); } TEST_F(ResourceParserTest, AutoIncrementIdsInPublicGroup) { diff --git a/tools/aapt2/ResourceTable.cpp b/tools/aapt2/ResourceTable.cpp index 8a3d047f6e8d..3e73be4dbc54 100644 --- a/tools/aapt2/ResourceTable.cpp +++ b/tools/aapt2/ResourceTable.cpp @@ -19,8 +19,6 @@ #include "ResourceTable.h" #include "ResourceValues.h" #include "ValueVisitor.h" - -#include "util/Comparators.h" #include "util/Util.h" #include <algorithm> @@ -124,6 +122,73 @@ ResourceEntry* ResourceTableType::findOrCreateEntry(const StringPiece16& name) { return entries.emplace(iter, new ResourceEntry(name))->get(); } +ResourceConfigValue* ResourceEntry::findValue(const ConfigDescription& config) { + return findValue(config, StringPiece()); +} + +struct ConfigKey { + const ConfigDescription* config; + const StringPiece& product; +}; + +bool ltConfigKeyRef(const std::unique_ptr<ResourceConfigValue>& lhs, const ConfigKey& rhs) { + int cmp = lhs->config.compare(*rhs.config); + if (cmp == 0) { + cmp = StringPiece(lhs->product).compare(rhs.product); + } + return cmp < 0; +} + +ResourceConfigValue* ResourceEntry::findValue(const ConfigDescription& config, + const StringPiece& product) { + auto iter = std::lower_bound(values.begin(), values.end(), + ConfigKey{ &config, product }, ltConfigKeyRef); + if (iter != values.end()) { + ResourceConfigValue* value = iter->get(); + if (value->config == config && StringPiece(value->product) == product) { + return value; + } + } + return nullptr; +} + +ResourceConfigValue* ResourceEntry::findOrCreateValue(const ConfigDescription& config, + const StringPiece& product) { + auto iter = std::lower_bound(values.begin(), values.end(), + ConfigKey{ &config, product }, ltConfigKeyRef); + if (iter != values.end()) { + ResourceConfigValue* value = iter->get(); + if (value->config == config && StringPiece(value->product) == product) { + return value; + } + } + ResourceConfigValue* newValue = values.insert( + iter, util::make_unique<ResourceConfigValue>(config, product))->get(); + return newValue; +} + +std::vector<ResourceConfigValue*> ResourceEntry::findAllValues(const ConfigDescription& config) { + std::vector<ResourceConfigValue*> results; + + auto iter = values.begin(); + for (; iter != values.end(); ++iter) { + ResourceConfigValue* value = iter->get(); + if (value->config == config) { + results.push_back(value); + ++iter; + break; + } + } + + for (; iter != values.end(); ++iter) { + ResourceConfigValue* value = iter->get(); + if (value->config == config) { + results.push_back(value); + } + } + return results; +} + /** * The default handler for collisions. A return value of -1 means keep the * existing value, 0 means fail, and +1 means take the incoming value. @@ -188,56 +253,69 @@ int ResourceTable::resolveValueCollision(Value* existing, Value* incoming) { static constexpr const char16_t* kValidNameChars = u"._-"; static constexpr const char16_t* kValidNameMangledChars = u"._-$"; -bool ResourceTable::addResource(const ResourceNameRef& name, const ConfigDescription& config, - std::unique_ptr<Value> value, IDiagnostics* diag) { - return addResourceImpl(name, {}, config, std::move(value), kValidNameChars, +bool ResourceTable::addResource(const ResourceNameRef& name, + const ConfigDescription& config, + const StringPiece& product, + std::unique_ptr<Value> value, + IDiagnostics* diag) { + return addResourceImpl(name, {}, config, product, std::move(value), kValidNameChars, resolveValueCollision, diag); } -bool ResourceTable::addResource(const ResourceNameRef& name, const ResourceId resId, - const ConfigDescription& config, std::unique_ptr<Value> value, +bool ResourceTable::addResource(const ResourceNameRef& name, + const ResourceId resId, + const ConfigDescription& config, + const StringPiece& product, + std::unique_ptr<Value> value, IDiagnostics* diag) { - return addResourceImpl(name, resId, config, std::move(value), kValidNameChars, + return addResourceImpl(name, resId, config, product, std::move(value), kValidNameChars, resolveValueCollision, diag); } -bool ResourceTable::addFileReference(const ResourceNameRef& name, const ConfigDescription& config, - const Source& source, const StringPiece16& path, +bool ResourceTable::addFileReference(const ResourceNameRef& name, + const ConfigDescription& config, + const Source& source, + const StringPiece16& path, IDiagnostics* diag) { return addFileReference(name, config, source, path, resolveValueCollision, diag); } -bool ResourceTable::addFileReference(const ResourceNameRef& name, const ConfigDescription& config, - const Source& source, const StringPiece16& path, +bool ResourceTable::addFileReference(const ResourceNameRef& name, + const ConfigDescription& config, + const Source& source, + const StringPiece16& path, std::function<int(Value*,Value*)> conflictResolver, IDiagnostics* diag) { std::unique_ptr<FileReference> fileRef = util::make_unique<FileReference>( stringPool.makeRef(path)); fileRef->setSource(source); - return addResourceImpl(name, ResourceId{}, config, std::move(fileRef), kValidNameChars, - conflictResolver, diag); + return addResourceImpl(name, ResourceId{}, config, StringPiece{}, std::move(fileRef), + kValidNameChars, conflictResolver, diag); } bool ResourceTable::addResourceAllowMangled(const ResourceNameRef& name, const ConfigDescription& config, + const StringPiece& product, std::unique_ptr<Value> value, IDiagnostics* diag) { - return addResourceImpl(name, ResourceId{}, config, std::move(value), kValidNameMangledChars, - resolveValueCollision, diag); + return addResourceImpl(name, ResourceId{}, config, product, std::move(value), + kValidNameMangledChars, resolveValueCollision, diag); } bool ResourceTable::addResourceAllowMangled(const ResourceNameRef& name, const ResourceId id, const ConfigDescription& config, + const StringPiece& product, std::unique_ptr<Value> value, IDiagnostics* diag) { - return addResourceImpl(name, id, config, std::move(value), kValidNameMangledChars, + return addResourceImpl(name, id, config, product, std::move(value), kValidNameMangledChars, resolveValueCollision, diag); } bool ResourceTable::addResourceImpl(const ResourceNameRef& name, const ResourceId resId, const ConfigDescription& config, + const StringPiece& product, std::unique_ptr<Value> value, const char16_t* validChars, std::function<int(Value*,Value*)> conflictResolver, @@ -298,21 +376,21 @@ bool ResourceTable::addResourceImpl(const ResourceNameRef& name, return false; } - const auto endIter = entry->values.end(); - auto iter = std::lower_bound(entry->values.begin(), endIter, config, cmp::lessThanConfig); - if (iter == endIter || iter->config != config) { - // This resource did not exist before, add it. - entry->values.insert(iter, ResourceConfigValue{ config, std::move(value) }); + ResourceConfigValue* configValue = entry->findOrCreateValue(config, product); + if (!configValue->value) { + // Resource does not exist, add it now. + configValue->value = std::move(value); + } else { - int collisionResult = conflictResolver(iter->value.get(), value.get()); + int collisionResult = conflictResolver(configValue->value.get(), value.get()); if (collisionResult > 0) { // Take the incoming value. - iter->value = std::move(value); + configValue->value = std::move(value); } else if (collisionResult == 0) { diag->error(DiagMessage(value->getSource()) << "duplicate value for resource '" << name << "' " << "with config '" << config << "'"); - diag->error(DiagMessage(iter->value->getSource()) + diag->error(DiagMessage(configValue->value->getSource()) << "resource previously defined here"); return false; } diff --git a/tools/aapt2/ResourceTable.h b/tools/aapt2/ResourceTable.h index 6b7b07ea3a93..8ffff1f70710 100644 --- a/tools/aapt2/ResourceTable.h +++ b/tools/aapt2/ResourceTable.h @@ -24,9 +24,12 @@ #include "Source.h" #include "StringPool.h" +#include <android-base/macros.h> +#include <map> #include <memory> #include <string> #include <tuple> +#include <unordered_map> #include <vector> namespace aapt { @@ -46,19 +49,36 @@ struct Symbol { std::u16string comment; }; -/** - * Represents a value defined for a given configuration. - */ -struct ResourceConfigValue { - ConfigDescription config; +class ResourceConfigValue { +public: + /** + * The configuration for which this value is defined. + */ + const ConfigDescription config; + + /** + * The product for which this value is defined. + */ + const std::string product; + + /** + * The actual Value. + */ std::unique_ptr<Value> value; + + ResourceConfigValue(const ConfigDescription& config, const StringPiece& product) : + config(config), product(product.toString()) { } + +private: + DISALLOW_COPY_AND_ASSIGN(ResourceConfigValue); }; /** * Represents a resource entry, which may have * varying values for each defined configuration. */ -struct ResourceEntry { +class ResourceEntry { +public: /** * The name of the resource. Immutable, as * this determines the order of this resource @@ -72,24 +92,33 @@ struct ResourceEntry { Maybe<uint16_t> id; /** - * Whether this resource is public (and must maintain the same - * entry ID across builds). + * Whether this resource is public (and must maintain the same entry ID across builds). */ Symbol symbolStatus; /** * The resource's values for each configuration. */ - std::vector<ResourceConfigValue> values; + std::vector<std::unique_ptr<ResourceConfigValue>> values; ResourceEntry(const StringPiece16& name) : name(name.toString()) { } + + ResourceConfigValue* findValue(const ConfigDescription& config); + ResourceConfigValue* findValue(const ConfigDescription& config, const StringPiece& product); + ResourceConfigValue* findOrCreateValue(const ConfigDescription& config, + const StringPiece& product); + std::vector<ResourceConfigValue*> findAllValues(const ConfigDescription& config); + +private: + DISALLOW_COPY_AND_ASSIGN(ResourceEntry); }; /** * Represents a resource type, which holds entries defined * for this type. */ -struct ResourceTableType { +class ResourceTableType { +public: /** * The logical type of resource (string, drawable, layout, etc.). */ @@ -114,8 +143,10 @@ struct ResourceTableType { explicit ResourceTableType(const ResourceType type) : type(type) { } ResourceEntry* findEntry(const StringPiece16& name); - ResourceEntry* findOrCreateEntry(const StringPiece16& name); + +private: + DISALLOW_COPY_AND_ASSIGN(ResourceTableType); }; enum class PackageType { @@ -125,16 +156,20 @@ enum class PackageType { Dynamic }; -struct ResourceTablePackage { +class ResourceTablePackage { +public: PackageType type = PackageType::App; Maybe<uint8_t> id; std::u16string name; std::vector<std::unique_ptr<ResourceTableType>> types; + ResourceTablePackage() = default; ResourceTableType* findType(ResourceType type); - ResourceTableType* findOrCreateType(const ResourceType type); + +private: + DISALLOW_COPY_AND_ASSIGN(ResourceTablePackage); }; /** @@ -144,8 +179,6 @@ struct ResourceTablePackage { class ResourceTable { public: ResourceTable() = default; - ResourceTable(const ResourceTable&) = delete; - ResourceTable& operator=(const ResourceTable&) = delete; /** * When a collision of resources occurs, this method decides which value to keep. @@ -155,38 +188,59 @@ public: */ static int resolveValueCollision(Value* existing, Value* incoming); - bool addResource(const ResourceNameRef& name, const ConfigDescription& config, - std::unique_ptr<Value> value, IDiagnostics* diag); + bool addResource(const ResourceNameRef& name, + const ConfigDescription& config, + const StringPiece& product, + std::unique_ptr<Value> value, + IDiagnostics* diag); - bool addResource(const ResourceNameRef& name, const ResourceId resId, - const ConfigDescription& config, std::unique_ptr<Value> value, + bool addResource(const ResourceNameRef& name, + const ResourceId resId, + const ConfigDescription& config, + const StringPiece& product, + std::unique_ptr<Value> value, IDiagnostics* diag); - bool addFileReference(const ResourceNameRef& name, const ConfigDescription& config, - const Source& source, const StringPiece16& path, + bool addFileReference(const ResourceNameRef& name, + const ConfigDescription& config, + const Source& source, + const StringPiece16& path, IDiagnostics* diag); - bool addFileReference(const ResourceNameRef& name, const ConfigDescription& config, - const Source& source, const StringPiece16& path, - std::function<int(Value*,Value*)> conflictResolver, IDiagnostics* diag); + bool addFileReference(const ResourceNameRef& name, + const ConfigDescription& config, + const Source& source, + const StringPiece16& path, + std::function<int(Value*,Value*)> conflictResolver, + IDiagnostics* diag); /** * Same as addResource, but doesn't verify the validity of the name. This is used * when loading resources from an existing binary resource table that may have mangled * names. */ - bool addResourceAllowMangled(const ResourceNameRef& name, const ConfigDescription& config, - std::unique_ptr<Value> value, IDiagnostics* diag); + bool addResourceAllowMangled(const ResourceNameRef& name, + const ConfigDescription& config, + const StringPiece& product, + std::unique_ptr<Value> value, + IDiagnostics* diag); - bool addResourceAllowMangled(const ResourceNameRef& name, const ResourceId id, - const ConfigDescription& config, std::unique_ptr<Value> value, + bool addResourceAllowMangled(const ResourceNameRef& name, + const ResourceId id, + const ConfigDescription& config, + const StringPiece& product, + std::unique_ptr<Value> value, IDiagnostics* diag); - bool setSymbolState(const ResourceNameRef& name, const ResourceId resId, - const Symbol& symbol, IDiagnostics* diag); + bool setSymbolState(const ResourceNameRef& name, + const ResourceId resId, + const Symbol& symbol, + IDiagnostics* diag); - bool setSymbolStateAllowMangled(const ResourceNameRef& name, const ResourceId resId, - const Symbol& symbol, IDiagnostics* diag); + bool setSymbolStateAllowMangled(const ResourceNameRef& name, + const ResourceId resId, + const Symbol& symbol, + IDiagnostics* diag); struct SearchResult { ResourceTablePackage* package; @@ -229,13 +283,19 @@ private: bool addResourceImpl(const ResourceNameRef& name, ResourceId resId, const ConfigDescription& config, + const StringPiece& product, std::unique_ptr<Value> value, const char16_t* validChars, std::function<int(Value*,Value*)> conflictResolver, IDiagnostics* diag); - bool setSymbolStateImpl(const ResourceNameRef& name, ResourceId resId, - const Symbol& symbol, const char16_t* validChars, IDiagnostics* diag); + bool setSymbolStateImpl(const ResourceNameRef& name, + ResourceId resId, + const Symbol& symbol, + const char16_t* validChars, + IDiagnostics* diag); + + DISALLOW_COPY_AND_ASSIGN(ResourceTable); }; } // namespace aapt diff --git a/tools/aapt2/ResourceTable_test.cpp b/tools/aapt2/ResourceTable_test.cpp index 42508fe154b8..180bd11275df 100644 --- a/tools/aapt2/ResourceTable_test.cpp +++ b/tools/aapt2/ResourceTable_test.cpp @@ -43,13 +43,13 @@ TEST_F(ResourceTableTest, FailToAddResourceWithBadName) { EXPECT_FALSE(table.addResource( ResourceNameRef(u"android", ResourceType::kId, u"hey,there"), - ConfigDescription{}, + ConfigDescription{}, "", test::ValueBuilder<Id>().setSource("test.xml", 21u).build(), &mDiagnostics)); EXPECT_FALSE(table.addResource( ResourceNameRef(u"android", ResourceType::kId, u"hey:there"), - ConfigDescription{}, + ConfigDescription{}, "", test::ValueBuilder<Id>().setSource("test.xml", 21u).build(), &mDiagnostics)); } @@ -59,6 +59,7 @@ TEST_F(ResourceTableTest, AddOneResource) { EXPECT_TRUE(table.addResource(test::parseNameOrDie(u"@android:attr/id"), ConfigDescription{}, + "", test::ValueBuilder<Id>() .setSource("test/path/file.xml", 23u).build(), &mDiagnostics)); @@ -76,24 +77,28 @@ TEST_F(ResourceTableTest, AddMultipleResources) { EXPECT_TRUE(table.addResource( test::parseNameOrDie(u"@android:attr/layout_width"), config, + "", test::ValueBuilder<Id>().setSource("test/path/file.xml", 10u).build(), &mDiagnostics)); EXPECT_TRUE(table.addResource( test::parseNameOrDie(u"@android:attr/id"), config, + "", test::ValueBuilder<Id>().setSource("test/path/file.xml", 12u).build(), &mDiagnostics)); EXPECT_TRUE(table.addResource( test::parseNameOrDie(u"@android:string/ok"), config, + "", test::ValueBuilder<Id>().setSource("test/path/file.xml", 14u).build(), &mDiagnostics)); EXPECT_TRUE(table.addResource( test::parseNameOrDie(u"@android:string/ok"), languageConfig, + "", test::ValueBuilder<BinaryPrimitive>(android::Res_value{}) .setSource("test/path/file.xml", 20u) .build(), @@ -110,18 +115,49 @@ TEST_F(ResourceTableTest, OverrideWeakResourceValue) { ResourceTable table; ASSERT_TRUE(table.addResource(test::parseNameOrDie(u"@android:attr/foo"), ConfigDescription{}, - util::make_unique<Attribute>(true), &mDiagnostics)); + "", util::make_unique<Attribute>(true), &mDiagnostics)); Attribute* attr = test::getValue<Attribute>(&table, u"@android:attr/foo"); ASSERT_NE(nullptr, attr); EXPECT_TRUE(attr->isWeak()); ASSERT_TRUE(table.addResource(test::parseNameOrDie(u"@android:attr/foo"), ConfigDescription{}, - util::make_unique<Attribute>(false), &mDiagnostics)); + "", util::make_unique<Attribute>(false), &mDiagnostics)); attr = test::getValue<Attribute>(&table, u"@android:attr/foo"); ASSERT_NE(nullptr, attr); EXPECT_FALSE(attr->isWeak()); } +TEST_F(ResourceTableTest, ProductVaryingValues) { + ResourceTable table; + + EXPECT_TRUE(table.addResource(test::parseNameOrDie(u"@android:string/foo"), + test::parseConfigOrDie("land"), + "tablet", + util::make_unique<Id>(), + &mDiagnostics)); + EXPECT_TRUE(table.addResource(test::parseNameOrDie(u"@android:string/foo"), + test::parseConfigOrDie("land"), + "phone", + util::make_unique<Id>(), + &mDiagnostics)); + + EXPECT_NE(nullptr, test::getValueForConfigAndProduct<Id>(&table, u"@android:string/foo", + test::parseConfigOrDie("land"), + "tablet")); + EXPECT_NE(nullptr, test::getValueForConfigAndProduct<Id>(&table, u"@android:string/foo", + test::parseConfigOrDie("land"), + "phone")); + + Maybe<ResourceTable::SearchResult> sr = table.findResource( + test::parseNameOrDie(u"@android:string/foo")); + AAPT_ASSERT_TRUE(sr); + std::vector<ResourceConfigValue*> values = sr.value().entry->findAllValues( + test::parseConfigOrDie("land")); + ASSERT_EQ(2u, values.size()); + EXPECT_EQ(std::string("phone"), values[0]->product); + EXPECT_EQ(std::string("tablet"), values[1]->product); +} + } // namespace aapt diff --git a/tools/aapt2/ValueVisitor.h b/tools/aapt2/ValueVisitor.h index 549303939351..ea2aa55764c1 100644 --- a/tools/aapt2/ValueVisitor.h +++ b/tools/aapt2/ValueVisitor.h @@ -146,7 +146,7 @@ inline void visitAllValuesInPackage(ResourceTablePackage* pkg, RawValueVisitor* for (auto& type : pkg->types) { for (auto& entry : type->entries) { for (auto& configValue : entry->values) { - configValue.value->accept(visitor); + configValue->value->accept(visitor); } } } diff --git a/tools/aapt2/compile/Compile.cpp b/tools/aapt2/compile/Compile.cpp index 1eefb821768e..0dd8e1859661 100644 --- a/tools/aapt2/compile/Compile.cpp +++ b/tools/aapt2/compile/Compile.cpp @@ -107,7 +107,6 @@ static Maybe<ResourcePathData> extractResourcePathData(const std::string& path, struct CompileOptions { std::string outputPath; Maybe<std::string> resDir; - std::vector<std::u16string> products; bool pseudolocalize = false; bool legacyMode = false; bool verbose = false; @@ -198,7 +197,6 @@ static bool compileTable(IAaptContext* context, const CompileOptions& options, xml::XmlPullParser xmlParser(fin); ResourceParserOptions parserOptions; - parserOptions.products = options.products; parserOptions.errorOnPositionalArguments = !options.legacyMode; // If the filename includes donottranslate, then the default translatable is false. @@ -457,11 +455,8 @@ public: int compile(const std::vector<StringPiece>& args) { CompileOptions options; - Maybe<std::string> productList; Flags flags = Flags() .requiredFlag("-o", "Output path", &options.outputPath) - .optionalFlag("--product", "Comma separated list of product types to compile", - &productList) .optionalFlag("--dir", "Directory to scan for resources", &options.resDir) .optionalSwitch("--pseudo-localize", "Generate resources for pseudo-locales " "(en-XA and ar-XB)", &options.pseudolocalize) @@ -472,12 +467,6 @@ int compile(const std::vector<StringPiece>& args) { return 1; } - if (productList) { - for (StringPiece part : util::tokenize<char>(productList.value(), ',')) { - options.products.push_back(util::utf8ToUtf16(part)); - } - } - CompileContext context; std::unique_ptr<IArchiveWriter> archiveWriter; diff --git a/tools/aapt2/compile/PseudolocaleGenerator.cpp b/tools/aapt2/compile/PseudolocaleGenerator.cpp index 2963d135cbca..be26b528b184 100644 --- a/tools/aapt2/compile/PseudolocaleGenerator.cpp +++ b/tools/aapt2/compile/PseudolocaleGenerator.cpp @@ -19,7 +19,6 @@ #include "ValueVisitor.h" #include "compile/PseudolocaleGenerator.h" #include "compile/Pseudolocalizer.h" -#include "util/Comparators.h" namespace aapt { @@ -208,10 +207,12 @@ ConfigDescription modifyConfigForPseudoLocale(const ConfigDescription& base, return modified; } -void pseudolocalizeIfNeeded(std::vector<ResourceConfigValue>* configValues, - Pseudolocalizer::Method method, StringPool* pool, Value* value) { +void pseudolocalizeIfNeeded(const Pseudolocalizer::Method method, + ResourceConfigValue* originalValue, + StringPool* pool, + ResourceEntry* entry) { Visitor visitor(pool, method); - value->accept(&visitor); + originalValue->value->accept(&visitor); std::unique_ptr<Value> localizedValue; if (visitor.mValue) { @@ -220,16 +221,18 @@ void pseudolocalizeIfNeeded(std::vector<ResourceConfigValue>* configValues, localizedValue = std::move(visitor.mItem); } - if (localizedValue) { - ConfigDescription pseudolocalizedConfig = modifyConfigForPseudoLocale(ConfigDescription{}, - method); - auto iter = std::lower_bound(configValues->begin(), configValues->end(), - pseudolocalizedConfig, cmp::lessThanConfig); - if (iter == configValues->end() || iter->config != pseudolocalizedConfig) { - // The pseudolocalized config doesn't exist, add it. - configValues->insert(iter, ResourceConfigValue{ pseudolocalizedConfig, - std::move(localizedValue) }); - } + if (!localizedValue) { + return; + } + + ConfigDescription configWithAccent = modifyConfigForPseudoLocale( + originalValue->config, method); + + ResourceConfigValue* newConfigValue = entry->findOrCreateValue( + configWithAccent, originalValue->product); + if (!newConfigValue->value) { + // Only use auto-generated pseudo-localization if none is defined. + newConfigValue->value = std::move(localizedValue); } } @@ -239,18 +242,13 @@ bool PseudolocaleGenerator::consume(IAaptContext* context, ResourceTable* table) for (auto& package : table->packages) { for (auto& type : package->types) { for (auto& entry : type->entries) { - auto iter = std::lower_bound(entry->values.begin(), entry->values.end(), - ConfigDescription{}, cmp::lessThanConfig); - if (iter != entry->values.end() && iter->config == ConfigDescription{}) { - // Only pseudolocalize the default configuration. - - // The iterator will be invalidated, so grab a pointer to the value. - Value* originalValue = iter->value.get(); - - pseudolocalizeIfNeeded(&entry->values, Pseudolocalizer::Method::kAccent, - &table->stringPool, originalValue); - pseudolocalizeIfNeeded(&entry->values, Pseudolocalizer::Method::kBidi, - &table->stringPool, originalValue); + std::vector<ResourceConfigValue*> values = entry->findAllValues( + ConfigDescription::defaultConfig()); + for (ResourceConfigValue* value : values) { + pseudolocalizeIfNeeded(Pseudolocalizer::Method::kAccent, value, + &table->stringPool, entry.get()); + pseudolocalizeIfNeeded(Pseudolocalizer::Method::kBidi, value, + &table->stringPool, entry.get()); } } } diff --git a/tools/aapt2/flatten/TableFlattener.cpp b/tools/aapt2/flatten/TableFlattener.cpp index 71ab3dbf52ca..da81046b2e42 100644 --- a/tools/aapt2/flatten/TableFlattener.cpp +++ b/tools/aapt2/flatten/TableFlattener.cpp @@ -402,10 +402,10 @@ private: const size_t configCount = entry->values.size(); for (size_t i = 0; i < configCount; i++) { - const ConfigDescription& config = entry->values[i].config; + const ConfigDescription& config = entry->values[i]->config; for (size_t j = i + 1; j < configCount; j++) { configMasks[entry->id.value()] |= util::hostToDevice32( - config.diff(entry->values[j].config)); + config.diff(entry->values[j]->config)); } } } @@ -445,8 +445,8 @@ private: // Group values by configuration. for (auto& configValue : entry->values) { - configToEntryListMap[configValue.config].push_back(FlatEntry{ - entry, configValue.value.get(), keyIndex }); + configToEntryListMap[configValue->config].push_back(FlatEntry{ + entry, configValue->value.get(), keyIndex }); } } diff --git a/tools/aapt2/java/JavaClassGenerator.cpp b/tools/aapt2/java/JavaClassGenerator.cpp index 7280f3a968a0..6e340a2a2742 100644 --- a/tools/aapt2/java/JavaClassGenerator.cpp +++ b/tools/aapt2/java/JavaClassGenerator.cpp @@ -23,7 +23,6 @@ #include "java/AnnotationProcessor.h" #include "java/ClassDefinitionWriter.h" #include "java/JavaClassGenerator.h" -#include "util/Comparators.h" #include "util/StringPiece.h" #include <algorithm> @@ -258,12 +257,12 @@ bool JavaClassGenerator::writeEntriesForClass(ClassDefinitionWriter* outClassDef } for (const auto& configValue : entry->values) { - processor.appendComment(configValue.value->getComment()); + processor.appendComment(configValue->value->getComment()); } // If this is an Attribute, append the format Javadoc. if (!entry->values.empty()) { - if (Attribute* attr = valueCast<Attribute>(entry->values.front().value.get())) { + if (Attribute* attr = valueCast<Attribute>(entry->values.front()->value.get())) { // We list out the available values for the given attribute. addAttributeFormatDoc(&processor, attr); } @@ -272,7 +271,7 @@ bool JavaClassGenerator::writeEntriesForClass(ClassDefinitionWriter* outClassDef if (type->type == ResourceType::kStyleable) { assert(!entry->values.empty()); const Styleable* styleable = static_cast<const Styleable*>( - entry->values.front().value.get()); + entry->values.front()->value.get()); writeStyleableEntryForClass(outClassDef, &processor, packageNameToGenerate, unmangledName, styleable); } else { @@ -311,11 +310,10 @@ bool JavaClassGenerator::generate(const StringPiece16& packageNameToGenerate, if (type->type == ResourceType::kAttr) { // Also include private attributes in this same class. - auto iter = std::lower_bound(package->types.begin(), package->types.end(), - ResourceType::kAttrPrivate, cmp::lessThanType); - if (iter != package->types.end() && (*iter)->type == ResourceType::kAttrPrivate) { + ResourceTableType* privType = package->findType(ResourceType::kAttrPrivate); + if (privType) { result = writeEntriesForClass(&classDef, packageNameToGenerate, - package.get(), iter->get()); + package.get(), privType); if (!result) { return false; } diff --git a/tools/aapt2/link/AutoVersioner.cpp b/tools/aapt2/link/AutoVersioner.cpp index c7e603ea3774..459c330cfbdc 100644 --- a/tools/aapt2/link/AutoVersioner.cpp +++ b/tools/aapt2/link/AutoVersioner.cpp @@ -18,9 +18,7 @@ #include "ResourceTable.h" #include "SdkConstants.h" #include "ValueVisitor.h" - #include "link/Linkers.h" -#include "util/Comparators.h" #include <algorithm> #include <cassert> @@ -31,7 +29,12 @@ bool shouldGenerateVersionedResource(const ResourceEntry* entry, const ConfigDes const int sdkVersionToGenerate) { assert(sdkVersionToGenerate > config.sdkVersion); const auto endIter = entry->values.end(); - auto iter = std::lower_bound(entry->values.begin(), endIter, config, cmp::lessThanConfig); + auto iter = entry->values.begin(); + for (; iter != endIter; ++iter) { + if ((*iter)->config == config) { + break; + } + } // The source config came from this list, so it should be here. assert(iter != entry->values.end()); @@ -45,10 +48,10 @@ bool shouldGenerateVersionedResource(const ResourceEntry* entry, const ConfigDes // are no higher sdk level versions of this resource. ConfigDescription tempConfig(config); for (; iter != endIter; ++iter) { - tempConfig.sdkVersion = iter->config.sdkVersion; - if (tempConfig == iter->config) { + tempConfig.sdkVersion = (*iter)->config.sdkVersion; + if (tempConfig == (*iter)->config) { // The two configs are the same, check the sdk version. - return sdkVersionToGenerate < iter->config.sdkVersion; + return sdkVersionToGenerate < (*iter)->config.sdkVersion; } } @@ -65,14 +68,14 @@ bool AutoVersioner::consume(IAaptContext* context, ResourceTable* table) { for (auto& entry : type->entries) { for (size_t i = 0; i < entry->values.size(); i++) { - ResourceConfigValue& configValue = entry->values[i]; - if (configValue.config.sdkVersion >= SDK_LOLLIPOP_MR1) { + ResourceConfigValue* configValue = entry->values[i].get(); + if (configValue->config.sdkVersion >= SDK_LOLLIPOP_MR1) { // If this configuration is only used on L-MR1 then we don't need // to do anything since we use private attributes since that version. continue; } - if (Style* style = valueCast<Style>(configValue.value.get())) { + if (Style* style = valueCast<Style>(configValue->value.get())) { Maybe<size_t> minSdkStripped; std::vector<Style::Entry> stripped; @@ -82,7 +85,7 @@ bool AutoVersioner::consume(IAaptContext* context, ResourceTable* table) { // Find the SDK level that is higher than the configuration allows. const size_t sdkLevel = findAttributeSdkLevel(iter->key.id.value()); - if (sdkLevel > std::max<size_t>(configValue.config.sdkVersion, 1)) { + if (sdkLevel > std::max<size_t>(configValue->config.sdkVersion, 1)) { // Record that we are about to strip this. stripped.emplace_back(std::move(*iter)); @@ -105,10 +108,10 @@ bool AutoVersioner::consume(IAaptContext* context, ResourceTable* table) { // there is no other defined resource for the version we want to // generate. if (shouldGenerateVersionedResource(entry.get(), - configValue.config, + configValue->config, minSdkStripped.value())) { // Let's create a new Style for this versioned resource. - ConfigDescription newConfig(configValue.config); + ConfigDescription newConfig(configValue->config); newConfig.sdkVersion = minSdkStripped.value(); std::unique_ptr<Style> newStyle(style->clone(&table->stringPool)); @@ -121,14 +124,8 @@ bool AutoVersioner::consume(IAaptContext* context, ResourceTable* table) { std::make_move_iterator(stripped.end())); // Insert the new Resource into the correct place. - auto iter = std::lower_bound(entry->values.begin(), - entry->values.end(), - newConfig, - cmp::lessThanConfig); - - entry->values.insert( - iter, - ResourceConfigValue{ newConfig, std::move(newStyle) }); + entry->findOrCreateValue(newConfig, {})->value = + std::move(newStyle); } } } diff --git a/tools/aapt2/link/AutoVersioner_test.cpp b/tools/aapt2/link/AutoVersioner_test.cpp index 29bcc93518cf..9b3a87c4eed0 100644 --- a/tools/aapt2/link/AutoVersioner_test.cpp +++ b/tools/aapt2/link/AutoVersioner_test.cpp @@ -15,9 +15,7 @@ */ #include "ConfigDescription.h" - #include "link/Linkers.h" - #include "test/Builders.h" #include "test/Context.h" @@ -31,9 +29,9 @@ TEST(AutoVersionerTest, GenerateVersionedResources) { const ConfigDescription sw600dpLandConfig = test::parseConfigOrDie("sw600dp-land"); ResourceEntry entry(u"foo"); - entry.values.push_back(ResourceConfigValue{ defaultConfig }); - entry.values.push_back(ResourceConfigValue{ landConfig }); - entry.values.push_back(ResourceConfigValue{ sw600dpLandConfig }); + entry.values.push_back(util::make_unique<ResourceConfigValue>(defaultConfig, "")); + entry.values.push_back(util::make_unique<ResourceConfigValue>(landConfig, "")); + entry.values.push_back(util::make_unique<ResourceConfigValue>(sw600dpLandConfig, "")); EXPECT_TRUE(shouldGenerateVersionedResource(&entry, defaultConfig, 17)); EXPECT_TRUE(shouldGenerateVersionedResource(&entry, landConfig, 17)); @@ -45,9 +43,9 @@ TEST(AutoVersionerTest, GenerateVersionedResourceWhenHigherVersionExists) { const ConfigDescription v21Config = test::parseConfigOrDie("v21"); ResourceEntry entry(u"foo"); - entry.values.push_back(ResourceConfigValue{ defaultConfig }); - entry.values.push_back(ResourceConfigValue{ sw600dpV13Config }); - entry.values.push_back(ResourceConfigValue{ v21Config }); + entry.values.push_back(util::make_unique<ResourceConfigValue>(defaultConfig, "")); + entry.values.push_back(util::make_unique<ResourceConfigValue>(sw600dpV13Config, "")); + entry.values.push_back(util::make_unique<ResourceConfigValue>(v21Config, "")); EXPECT_TRUE(shouldGenerateVersionedResource(&entry, defaultConfig, 17)); EXPECT_FALSE(shouldGenerateVersionedResource(&entry, defaultConfig, 22)); diff --git a/tools/aapt2/link/Link.cpp b/tools/aapt2/link/Link.cpp index 8e321798787a..3437ac0d9a7f 100644 --- a/tools/aapt2/link/Link.cpp +++ b/tools/aapt2/link/Link.cpp @@ -31,6 +31,7 @@ #include "java/ManifestClassGenerator.h" #include "java/ProguardRules.h" #include "link/Linkers.h" +#include "link/ProductFilter.h" #include "link/ReferenceLinker.h" #include "link/ManifestFixer.h" #include "link/TableMerger.h" @@ -70,6 +71,7 @@ struct LinkOptions { Maybe<std::u16string> privateSymbols; ManifestFixerOptions manifestFixerOptions; IConfigFilter* configFilter = nullptr; + std::unordered_set<std::string> products; }; struct LinkContext : public IAaptContext { @@ -292,16 +294,16 @@ public: for (const auto& configValue : entry->values) { // Special case the occurrence of an ID that is being generated for the // 'android' package. This is due to legacy reasons. - if (valueCast<Id>(configValue.value.get()) && + if (valueCast<Id>(configValue->value.get()) && package->name == u"android") { mContext->getDiagnostics()->warn( - DiagMessage(configValue.value->getSource()) + DiagMessage(configValue->value->getSource()) << "generated id '" << resName << "' for external package '" << package->name << "'"); } else { mContext->getDiagnostics()->error( - DiagMessage(configValue.value->getSource()) + DiagMessage(configValue->value->getSource()) << "defined resource '" << resName << "' for external package '" << package->name << "'"); @@ -512,7 +514,10 @@ public: std::unique_ptr<Id> id = util::make_unique<Id>(); id->setSource(fileDesc->source.withLine(exportedSymbol.line)); - bool result = mFinalTable.addResourceAllowMangled(resName, {}, std::move(id), + bool result = mFinalTable.addResourceAllowMangled(resName, + ConfigDescription::defaultConfig(), + std::string(), + std::move(id), mContext->getDiagnostics()); if (!result) { return false; @@ -681,6 +686,12 @@ public: mContext->getDiagnostics()->error(DiagMessage() << "failed linking references"); return 1; } + + ProductFilter productFilter(mOptions.products); + if (!productFilter.consume(mContext, &mFinalTable)) { + mContext->getDiagnostics()->error(DiagMessage() << "failed stripping products"); + return 1; + } } proguard::KeepSet proguardKeepSet; @@ -931,6 +942,7 @@ int link(const std::vector<StringPiece>& args) { Maybe<std::string> customJavaPackage; std::vector<std::string> extraJavaPackages; Maybe<std::string> configs; + Maybe<std::string> productList; bool legacyXFlag = false; bool requireLocalization = false; Flags flags = Flags() @@ -954,6 +966,8 @@ int link(const std::vector<StringPiece>& args) { &requireLocalization) .optionalFlag("-c", "Comma separated list of configurations to include. The default\n" "is all configurations", &configs) + .optionalFlag("--product", "Comma separated list of product names to keep", + &productList) .optionalSwitch("--output-to-dir", "Outputs the APK contents to a directory specified " "by -o", &options.outputToDirectory) @@ -1039,6 +1053,14 @@ int link(const std::vector<StringPiece>& args) { } } + if (productList) { + for (StringPiece product : util::tokenize<char>(productList.value(), ',')) { + if (product != "" && product != "default") { + options.products.insert(product.toString()); + } + } + } + AxisConfigFilter filter; if (configs) { for (const StringPiece& configStr : util::tokenize<char>(configs.value(), ',')) { diff --git a/tools/aapt2/link/Linkers.h b/tools/aapt2/link/Linkers.h index 4d3a483c6b82..ec532aba465f 100644 --- a/tools/aapt2/link/Linkers.h +++ b/tools/aapt2/link/Linkers.h @@ -26,7 +26,7 @@ namespace aapt { class ResourceTable; -struct ResourceEntry; +class ResourceEntry; struct ConfigDescription; /** diff --git a/tools/aapt2/link/ProductFilter.cpp b/tools/aapt2/link/ProductFilter.cpp new file mode 100644 index 000000000000..8784e891b293 --- /dev/null +++ b/tools/aapt2/link/ProductFilter.cpp @@ -0,0 +1,118 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "link/ProductFilter.h" + +namespace aapt { + +ProductFilter::ResourceConfigValueIter +ProductFilter::selectProductToKeep(const ResourceNameRef& name, + const ResourceConfigValueIter begin, + const ResourceConfigValueIter end, + IDiagnostics* diag) { + ResourceConfigValueIter defaultProductIter = end; + ResourceConfigValueIter selectedProductIter = end; + + for (ResourceConfigValueIter iter = begin; iter != end; ++iter) { + ResourceConfigValue* configValue = iter->get(); + if (mProducts.find(configValue->product) != mProducts.end()) { + if (selectedProductIter != end) { + // We have two possible values for this product! + diag->error(DiagMessage(configValue->value->getSource()) + << "selection of product '" << configValue->product + << "' for resource " << name << " is ambiguous"); + + ResourceConfigValue* previouslySelectedConfigValue = selectedProductIter->get(); + diag->note(DiagMessage(previouslySelectedConfigValue->value->getSource()) + << "product '" << previouslySelectedConfigValue->product + << "' is also a candidate"); + return end; + } + + // Select this product. + selectedProductIter = iter; + } + + if (configValue->product.empty() || configValue->product == "default") { + if (defaultProductIter != end) { + // We have two possible default values. + diag->error(DiagMessage(configValue->value->getSource()) + << "multiple default products defined for resource " << name); + + ResourceConfigValue* previouslyDefaultConfigValue = defaultProductIter->get(); + diag->note(DiagMessage(previouslyDefaultConfigValue->value->getSource()) + << "default product also defined here"); + return end; + } + + // Mark the default. + defaultProductIter = iter; + } + } + + if (defaultProductIter == end) { + diag->error(DiagMessage() << "no default product defined for resource " << name); + return end; + } + + if (selectedProductIter == end) { + selectedProductIter = defaultProductIter; + } + return selectedProductIter; +} + +bool ProductFilter::consume(IAaptContext* context, ResourceTable* table) { + bool error = false; + for (auto& pkg : table->packages) { + for (auto& type : pkg->types) { + for (auto& entry : type->entries) { + std::vector<std::unique_ptr<ResourceConfigValue>> newValues; + + ResourceConfigValueIter iter = entry->values.begin(); + ResourceConfigValueIter startRangeIter = iter; + while (iter != entry->values.end()) { + ++iter; + if (iter == entry->values.end() || + (*iter)->config != (*startRangeIter)->config) { + + // End of the array, or we saw a different config, + // so this must be the end of a range of products. + // Select the product to keep from the set of products defined. + ResourceNameRef name(pkg->name, type->type, entry->name); + auto valueToKeep = selectProductToKeep(name, startRangeIter, iter, + context->getDiagnostics()); + if (valueToKeep == iter) { + // An error occurred, we could not pick a product. + error = true; + } else { + // We selected a product to keep. Move it to the new array. + newValues.push_back(std::move(*valueToKeep)); + } + + // Start the next range of products. + startRangeIter = iter; + } + } + + // Now move the new values in to place. + entry->values = std::move(newValues); + } + } + } + return !error; +} + +} // namespace aapt diff --git a/tools/aapt2/link/ProductFilter.h b/tools/aapt2/link/ProductFilter.h new file mode 100644 index 000000000000..d2edd38289dc --- /dev/null +++ b/tools/aapt2/link/ProductFilter.h @@ -0,0 +1,49 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef AAPT_LINK_PRODUCTFILTER_H +#define AAPT_LINK_PRODUCTFILTER_H + +#include "ResourceTable.h" +#include "process/IResourceTableConsumer.h" + +#include <android-base/macros.h> +#include <unordered_set> + +namespace aapt { + +class ProductFilter { +public: + using ResourceConfigValueIter = std::vector<std::unique_ptr<ResourceConfigValue>>::iterator; + + ProductFilter(std::unordered_set<std::string> products) : mProducts(products) { } + + ResourceConfigValueIter selectProductToKeep(const ResourceNameRef& name, + const ResourceConfigValueIter begin, + const ResourceConfigValueIter end, + IDiagnostics* diag); + + bool consume(IAaptContext* context, ResourceTable* table); + +private: + std::unordered_set<std::string> mProducts; + + DISALLOW_COPY_AND_ASSIGN(ProductFilter); +}; + +} // namespace aapt + +#endif /* AAPT_LINK_PRODUCTFILTER_H */ diff --git a/tools/aapt2/link/ProductFilter_test.cpp b/tools/aapt2/link/ProductFilter_test.cpp new file mode 100644 index 000000000000..f4f756ae4519 --- /dev/null +++ b/tools/aapt2/link/ProductFilter_test.cpp @@ -0,0 +1,136 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "link/ProductFilter.h" +#include "test/Builders.h" +#include "test/Context.h" + +#include <gtest/gtest.h> + +namespace aapt { + +TEST(ProductFilterTest, SelectTwoProducts) { + std::unique_ptr<IAaptContext> context = test::ContextBuilder().build(); + + const ConfigDescription land = test::parseConfigOrDie("land"); + const ConfigDescription port = test::parseConfigOrDie("port"); + + ResourceTable table; + ASSERT_TRUE(table.addResource(test::parseNameOrDie(u"@android:string/one"), + land, "", + test::ValueBuilder<Id>() + .setSource(Source("land/default.xml")).build(), + context->getDiagnostics())); + ASSERT_TRUE(table.addResource(test::parseNameOrDie(u"@android:string/one"), + land, "tablet", + test::ValueBuilder<Id>() + .setSource(Source("land/tablet.xml")).build(), + context->getDiagnostics())); + + ASSERT_TRUE(table.addResource(test::parseNameOrDie(u"@android:string/one"), + port, "", + test::ValueBuilder<Id>() + .setSource(Source("port/default.xml")).build(), + context->getDiagnostics())); + ASSERT_TRUE(table.addResource(test::parseNameOrDie(u"@android:string/one"), + port, "tablet", + test::ValueBuilder<Id>() + .setSource(Source("port/tablet.xml")).build(), + context->getDiagnostics())); + + ProductFilter filter({ "tablet" }); + ASSERT_TRUE(filter.consume(context.get(), &table)); + + EXPECT_EQ(nullptr, test::getValueForConfigAndProduct<Id>(&table, u"@android:string/one", + land, "")); + EXPECT_NE(nullptr, test::getValueForConfigAndProduct<Id>(&table, u"@android:string/one", + land, "tablet")); + EXPECT_EQ(nullptr, test::getValueForConfigAndProduct<Id>(&table, u"@android:string/one", + port, "")); + EXPECT_NE(nullptr, test::getValueForConfigAndProduct<Id>(&table, u"@android:string/one", + port, "tablet")); +} + +TEST(ProductFilterTest, SelectDefaultProduct) { + std::unique_ptr<IAaptContext> context = test::ContextBuilder().build(); + + ResourceTable table; + ASSERT_TRUE(table.addResource(test::parseNameOrDie(u"@android:string/one"), + ConfigDescription::defaultConfig(), "", + test::ValueBuilder<Id>() + .setSource(Source("default.xml")).build(), + context->getDiagnostics())); + ASSERT_TRUE(table.addResource(test::parseNameOrDie(u"@android:string/one"), + ConfigDescription::defaultConfig(), "tablet", + test::ValueBuilder<Id>() + .setSource(Source("tablet.xml")).build(), + context->getDiagnostics())); + + ProductFilter filter({}); + ASSERT_TRUE(filter.consume(context.get(), &table)); + + EXPECT_NE(nullptr, test::getValueForConfigAndProduct<Id>(&table, u"@android:string/one", + ConfigDescription::defaultConfig(), + "")); + EXPECT_EQ(nullptr, test::getValueForConfigAndProduct<Id>(&table, u"@android:string/one", + ConfigDescription::defaultConfig(), + "tablet")); +} + +TEST(ProductFilterTest, FailOnAmbiguousProduct) { + std::unique_ptr<IAaptContext> context = test::ContextBuilder().build(); + + ResourceTable table; + ASSERT_TRUE(table.addResource(test::parseNameOrDie(u"@android:string/one"), + ConfigDescription::defaultConfig(), "", + test::ValueBuilder<Id>() + .setSource(Source("default.xml")).build(), + context->getDiagnostics())); + ASSERT_TRUE(table.addResource(test::parseNameOrDie(u"@android:string/one"), + ConfigDescription::defaultConfig(), "tablet", + test::ValueBuilder<Id>() + .setSource(Source("tablet.xml")).build(), + context->getDiagnostics())); + ASSERT_TRUE(table.addResource(test::parseNameOrDie(u"@android:string/one"), + ConfigDescription::defaultConfig(), "no-sdcard", + test::ValueBuilder<Id>() + .setSource(Source("no-sdcard.xml")).build(), + context->getDiagnostics())); + + ProductFilter filter({ "tablet", "no-sdcard" }); + ASSERT_FALSE(filter.consume(context.get(), &table)); +} + +TEST(ProductFilterTest, FailOnMultipleDefaults) { + std::unique_ptr<IAaptContext> context = test::ContextBuilder().build(); + + ResourceTable table; + ASSERT_TRUE(table.addResource(test::parseNameOrDie(u"@android:string/one"), + ConfigDescription::defaultConfig(), "", + test::ValueBuilder<Id>() + .setSource(Source(".xml")).build(), + context->getDiagnostics())); + ASSERT_TRUE(table.addResource(test::parseNameOrDie(u"@android:string/one"), + ConfigDescription::defaultConfig(), "default", + test::ValueBuilder<Id>() + .setSource(Source("default.xml")).build(), + context->getDiagnostics())); + + ProductFilter filter({}); + ASSERT_FALSE(filter.consume(context.get(), &table)); +} + +} // namespace aapt diff --git a/tools/aapt2/link/ReferenceLinker.cpp b/tools/aapt2/link/ReferenceLinker.cpp index 27435398c408..ef3fe4f58d41 100644 --- a/tools/aapt2/link/ReferenceLinker.cpp +++ b/tools/aapt2/link/ReferenceLinker.cpp @@ -316,7 +316,7 @@ bool ReferenceLinker::consume(IAaptContext* context, ResourceTable* table) { &table->stringPool, &declStack, &callSite); for (auto& configValue : entry->values) { - configValue.value->accept(&visitor); + configValue->value->accept(&visitor); } if (visitor.hasError()) { diff --git a/tools/aapt2/link/TableMerger.cpp b/tools/aapt2/link/TableMerger.cpp index e01a00401133..2ecd5b018691 100644 --- a/tools/aapt2/link/TableMerger.cpp +++ b/tools/aapt2/link/TableMerger.cpp @@ -18,9 +18,7 @@ #include "ResourceUtils.h" #include "ResourceValues.h" #include "ValueVisitor.h" - #include "link/TableMerger.h" -#include "util/Comparators.h" #include "util/Util.h" #include <cassert> @@ -197,28 +195,28 @@ bool TableMerger::doMerge(const Source& src, ResourceNameRef resName(mMasterPackage->name, dstType->type, dstEntry->name); - for (ResourceConfigValue& srcValue : srcEntry->values) { - auto iter = std::lower_bound(dstEntry->values.begin(), dstEntry->values.end(), - srcValue.config, cmp::lessThanConfig); + for (auto& srcValue : srcEntry->values) { + ResourceConfigValue* dstValue = dstEntry->findValue(srcValue->config, + srcValue->product); const bool stripConfig = mOptions.filter ? - !mOptions.filter->match(srcValue.config) : false; + !mOptions.filter->match(srcValue->config) : false; - if (iter != dstEntry->values.end() && iter->config == srcValue.config) { + if (dstValue) { const int collisionResult = ResourceTable::resolveValueCollision( - iter->value.get(), srcValue.value.get()); + dstValue->value.get(), srcValue->value.get()); if (collisionResult == 0 && !overlay) { // Error! ResourceNameRef resourceName(srcPackage->name, srcType->type, srcEntry->name); - mContext->getDiagnostics()->error(DiagMessage(srcValue.value->getSource()) + mContext->getDiagnostics()->error(DiagMessage(srcValue->value->getSource()) << "resource '" << resourceName << "' has a conflicting value for " << "configuration (" - << srcValue.config << ")"); - mContext->getDiagnostics()->note(DiagMessage(iter->value->getSource()) + << srcValue->config << ")"); + mContext->getDiagnostics()->note(DiagMessage(dstValue->value->getSource()) << "originally defined here"); error = true; continue; @@ -227,16 +225,18 @@ bool TableMerger::doMerge(const Source& src, continue; } - } else if (!stripConfig){ - // Insert a place holder value. We will fill it in below. - iter = dstEntry->values.insert(iter, ResourceConfigValue{ srcValue.config }); } if (stripConfig) { continue; } - if (FileReference* f = valueCast<FileReference>(srcValue.value.get())) { + if (!dstValue) { + // Force create the entry if we didn't have it. + dstValue = dstEntry->findOrCreateValue(srcValue->config, srcValue->product); + } + + if (FileReference* f = valueCast<FileReference>(srcValue->value.get())) { std::unique_ptr<FileReference> newFileRef; if (manglePackage) { newFileRef = cloneAndMangleFile(srcPackage->name, *f); @@ -246,15 +246,15 @@ bool TableMerger::doMerge(const Source& src, } if (callback) { - if (!callback(resName, iter->config, newFileRef.get(), f)) { + if (!callback(resName, srcValue->config, newFileRef.get(), f)) { error = true; continue; } } - iter->value = std::move(newFileRef); + dstValue->value = std::move(newFileRef); } else { - iter->value = std::unique_ptr<Value>(srcValue.value->clone( + dstValue->value = std::unique_ptr<Value>(srcValue->value->clone( &mMasterTable->stringPool)); } } @@ -290,7 +290,8 @@ bool TableMerger::mergeFileImpl(const ResourceFile& fileDesc, io::IFile* file, b ResourceTablePackage* pkg = table.createPackage(fileDesc.name.package, 0x0); pkg->findOrCreateType(fileDesc.name.type) ->findOrCreateEntry(fileDesc.name.entry) - ->values.push_back(ResourceConfigValue{ fileDesc.config, std::move(fileRef) }); + ->findOrCreateValue(fileDesc.config, {}) + ->value = std::move(fileRef); auto callback = [&](const ResourceNameRef& name, const ConfigDescription& config, FileReference* newFile, FileReference* oldFile) -> bool { diff --git a/tools/aapt2/process/SymbolTable.cpp b/tools/aapt2/process/SymbolTable.cpp index 6ad2f9c10d22..b6030a2874a3 100644 --- a/tools/aapt2/process/SymbolTable.cpp +++ b/tools/aapt2/process/SymbolTable.cpp @@ -17,9 +17,7 @@ #include "ConfigDescription.h" #include "Resource.h" #include "ValueVisitor.h" - #include "process/SymbolTable.h" -#include "util/Comparators.h" #include "util/Util.h" #include <androidfw/AssetManager.h> @@ -55,12 +53,10 @@ const ISymbolTable::Symbol* SymbolTableWrapper::findByName(const ResourceName& n if (name.type == ResourceType::kAttr || name.type == ResourceType::kAttrPrivate) { const ConfigDescription kDefaultConfig; - auto iter = std::lower_bound(sr.entry->values.begin(), sr.entry->values.end(), - kDefaultConfig, cmp::lessThanConfig); - - if (iter != sr.entry->values.end() && iter->config == kDefaultConfig) { + ResourceConfigValue* configValue = sr.entry->findValue(kDefaultConfig); + if (configValue) { // This resource has an Attribute. - if (Attribute* attr = valueCast<Attribute>(iter->value.get())) { + if (Attribute* attr = valueCast<Attribute>(configValue->value.get())) { symbol->attribute = util::make_unique<Attribute>(*attr); } else { return {}; diff --git a/tools/aapt2/proto/TableProtoDeserializer.cpp b/tools/aapt2/proto/TableProtoDeserializer.cpp index 1310aa6774bb..9856a00d7f67 100644 --- a/tools/aapt2/proto/TableProtoDeserializer.cpp +++ b/tools/aapt2/proto/TableProtoDeserializer.cpp @@ -19,7 +19,6 @@ #include "ValueVisitor.h" #include "proto/ProtoHelpers.h" #include "proto/ProtoSerialize.h" -#include "util/Comparators.h" #include <androidfw/ResourceTypes.h> @@ -134,21 +133,19 @@ public: return {}; } - auto iter = std::lower_bound(entry->values.begin(), entry->values.end(), - config, cmp::lessThanConfig); - if (iter != entry->values.end() && iter->config == config) { + ResourceConfigValue* configValue = entry->findOrCreateValue(config, + pbConfig.product()); + if (configValue->value) { // Duplicate config. mDiag->error(DiagMessage(mSource) << "duplicate configuration"); return {}; } - std::unique_ptr<Value> value = deserializeValueFromPb(pbConfigValue.value(), - config, - &table->stringPool); - if (!value) { + configValue->value = deserializeValueFromPb(pbConfigValue.value(), + config, &table->stringPool); + if (!configValue->value) { return {}; } - entry->values.insert(iter, ResourceConfigValue{ config, std::move(value) }); } } } diff --git a/tools/aapt2/proto/TableProtoSerializer.cpp b/tools/aapt2/proto/TableProtoSerializer.cpp index 4a2176d20db9..bba2da42c666 100644 --- a/tools/aapt2/proto/TableProtoSerializer.cpp +++ b/tools/aapt2/proto/TableProtoSerializer.cpp @@ -242,21 +242,24 @@ std::unique_ptr<pb::ResourceTable> serializeTableToPb(ResourceTable* table) { for (auto& configValue : entry->values) { pb::ConfigValue* pbConfigValue = pbEntry->add_config_values(); - serializeConfig(configValue.config, pbConfigValue->mutable_config()); + serializeConfig(configValue->config, pbConfigValue->mutable_config()); + if (!configValue->product.empty()) { + pbConfigValue->mutable_config()->set_product(configValue->product); + } pb::Value* pbValue = pbConfigValue->mutable_value(); - serializeSourceToPb(configValue.value->getSource(), &sourcePool, + serializeSourceToPb(configValue->value->getSource(), &sourcePool, pbValue->mutable_source()); - if (!configValue.value->getComment().empty()) { - pbValue->set_comment(util::utf16ToUtf8(configValue.value->getComment())); + if (!configValue->value->getComment().empty()) { + pbValue->set_comment(util::utf16ToUtf8(configValue->value->getComment())); } - if (configValue.value->isWeak()) { + if (configValue->value->isWeak()) { pbValue->set_weak(true); } PbSerializerVisitor visitor(&sourcePool, &symbolPool, pbValue); - configValue.value->accept(&visitor); + configValue->value->accept(&visitor); } } } diff --git a/tools/aapt2/proto/TableProtoSerializer_test.cpp b/tools/aapt2/proto/TableProtoSerializer_test.cpp index 1061b8f76f57..70a33f795f87 100644 --- a/tools/aapt2/proto/TableProtoSerializer_test.cpp +++ b/tools/aapt2/proto/TableProtoSerializer_test.cpp @@ -49,9 +49,19 @@ TEST(TableProtoSerializer, SerializeSinglePackage) { std::unique_ptr<Plural> plural = util::make_unique<Plural>(); plural->values[Plural::One] = util::make_unique<String>(table->stringPool.makeRef(u"one")); ASSERT_TRUE(table->addResource(test::parseNameOrDie(u"@com.app.a:plurals/hey"), - ConfigDescription{}, std::move(plural), + ConfigDescription{}, std::string(), std::move(plural), context->getDiagnostics())); + // Make a resource with different products. + ASSERT_TRUE(table->addResource(test::parseNameOrDie(u"@com.app.a:integer/one"), + test::parseConfigOrDie("land"), std::string(), + test::buildPrimitive(android::Res_value::TYPE_INT_DEC, 123u), + context->getDiagnostics())); + ASSERT_TRUE(table->addResource(test::parseNameOrDie(u"@com.app.a:integer/one"), + test::parseConfigOrDie("land"), std::string("tablet"), + test::buildPrimitive(android::Res_value::TYPE_INT_DEC, 321u), + context->getDiagnostics())); + std::unique_ptr<pb::ResourceTable> pbTable = serializeTableToPb(table.get()); ASSERT_NE(nullptr, pbTable); @@ -69,6 +79,17 @@ TEST(TableProtoSerializer, SerializeSinglePackage) { AAPT_ASSERT_TRUE(result); EXPECT_EQ(SymbolState::kPublic, result.value().type->symbolStatus.state); EXPECT_EQ(SymbolState::kPublic, result.value().entry->symbolStatus.state); + + // Find the product-dependent values + BinaryPrimitive* prim = test::getValueForConfigAndProduct<BinaryPrimitive>( + newTable.get(), u"@com.app.a:integer/one", test::parseConfigOrDie("land"), ""); + ASSERT_NE(nullptr, prim); + EXPECT_EQ(123u, prim->value.data); + + prim = test::getValueForConfigAndProduct<BinaryPrimitive>( + newTable.get(), u"@com.app.a:integer/one", test::parseConfigOrDie("land"), "tablet"); + ASSERT_NE(nullptr, prim); + EXPECT_EQ(321u, prim->value.data); } TEST(TableProtoSerializer, SerializeFileHeader) { diff --git a/tools/aapt2/test/Builders.h b/tools/aapt2/test/Builders.h index 579a46ec230f..834caf8b9a49 100644 --- a/tools/aapt2/test/Builders.h +++ b/tools/aapt2/test/Builders.h @@ -104,8 +104,8 @@ public: const ConfigDescription& config, std::unique_ptr<Value> value) { ResourceName resName = parseNameOrDie(name); - bool result = mTable->addResourceAllowMangled(resName, id, config, std::move(value), - &mDiagnostics); + bool result = mTable->addResourceAllowMangled(resName, id, config, std::string(), + std::move(value), &mDiagnostics); assert(result); return *this; } @@ -132,6 +132,14 @@ inline std::unique_ptr<Reference> buildReference(const StringPiece16& ref, return reference; } +inline std::unique_ptr<BinaryPrimitive> buildPrimitive(uint8_t type, uint32_t data) { + android::Res_value value = {}; + value.size = sizeof(value); + value.dataType = type; + value.data = data; + return util::make_unique<BinaryPrimitive>(value); +} + template <typename T> class ValueBuilder { private: diff --git a/tools/aapt2/test/Common.h b/tools/aapt2/test/Common.h index 51e2dd44e521..348c32a04e88 100644 --- a/tools/aapt2/test/Common.h +++ b/tools/aapt2/test/Common.h @@ -52,6 +52,11 @@ struct DummyDiagnosticsImpl : public IDiagnostics { void note(const DiagMessage& message) override {} }; +inline IDiagnostics* getDiagnostics() { + static DummyDiagnosticsImpl diag; + return &diag; +} + inline ResourceName parseNameOrDie(const StringPiece16& str) { ResourceNameRef ref; bool result = ResourceUtils::tryParseReference(str, &ref); @@ -66,23 +71,25 @@ inline ConfigDescription parseConfigOrDie(const StringPiece& str) { return config; } -template <typename T> T* getValueForConfig(ResourceTable* table, const StringPiece16& resName, - const ConfigDescription& config) { +template <typename T> T* getValueForConfigAndProduct(ResourceTable* table, + const StringPiece16& resName, + const ConfigDescription& config, + const StringPiece& product) { Maybe<ResourceTable::SearchResult> result = table->findResource(parseNameOrDie(resName)); if (result) { - ResourceEntry* entry = result.value().entry; - auto iter = std::lower_bound(entry->values.begin(), entry->values.end(), config, - [](const ResourceConfigValue& a, const ConfigDescription& b) - -> bool { - return a.config < b; - }); - if (iter != entry->values.end() && iter->config == config) { - return valueCast<T>(iter->value.get()); + ResourceConfigValue* configValue = result.value().entry->findValue(config, product); + if (configValue) { + return valueCast<T>(configValue->value.get()); } } return nullptr; } +template <typename T> T* getValueForConfig(ResourceTable* table, const StringPiece16& resName, + const ConfigDescription& config) { + return getValueForConfigAndProduct<T>(table, resName, config, {}); +} + template <typename T> T* getValue(ResourceTable* table, const StringPiece16& resName) { return getValueForConfig<T>(table, resName, {}); } diff --git a/tools/aapt2/unflatten/BinaryResourceParser.cpp b/tools/aapt2/unflatten/BinaryResourceParser.cpp index 341770360068..33b505ed2eb4 100644 --- a/tools/aapt2/unflatten/BinaryResourceParser.cpp +++ b/tools/aapt2/unflatten/BinaryResourceParser.cpp @@ -352,7 +352,7 @@ bool BinaryResourceParser::parseType(const ResourceTablePackage* package, return false; } - if (!mTable->addResourceAllowMangled(name, config, std::move(resourceValue), + if (!mTable->addResourceAllowMangled(name, config, {}, std::move(resourceValue), mContext->getDiagnostics())) { return false; } diff --git a/tools/aapt2/util/Comparators.h b/tools/aapt2/util/Comparators.h deleted file mode 100644 index 0ee0bf35457d..000000000000 --- a/tools/aapt2/util/Comparators.h +++ /dev/null @@ -1,37 +0,0 @@ -/* - * Copyright (C) 2015 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#ifndef AAPT_UTIL_COMPARATORS_H -#define AAPT_UTIL_COMPARATORS_H - -#include "ConfigDescription.h" -#include "ResourceTable.h" - -namespace aapt { -namespace cmp { - -inline bool lessThanConfig(const ResourceConfigValue& a, const ConfigDescription& b) { - return a.config < b; -} - -inline bool lessThanType(const std::unique_ptr<ResourceTableType>& a, ResourceType b) { - return a->type < b; -} - -} // namespace cmp -} // namespace aapt - -#endif /* AAPT_UTIL_COMPARATORS_H */ |