AAPT2: Introduce notion of 'product' to ResourceTable (e4bb9eb5) · Commits · e / os / android_frameworks_base

tools/aapt2/Android.mk

+2 −0

Original line number	Original line	Diff line number	Diff line
	@@ -38,6 +38,7 @@ sources := \
	io/ZipArchive.cpp \		io/ZipArchive.cpp \
	link/AutoVersioner.cpp \		link/AutoVersioner.cpp \
	link/ManifestFixer.cpp \		link/ManifestFixer.cpp \
			link/ProductFilter.cpp \
	link/PrivateAttributeMover.cpp \		link/PrivateAttributeMover.cpp \
	link/ReferenceLinker.cpp \		link/ReferenceLinker.cpp \
	link/TableMerger.cpp \		link/TableMerger.cpp \
	@@ -83,6 +84,7 @@ testSources := \
	link/AutoVersioner_test.cpp \		link/AutoVersioner_test.cpp \
	link/ManifestFixer_test.cpp \		link/ManifestFixer_test.cpp \
	link/PrivateAttributeMover_test.cpp \		link/PrivateAttributeMover_test.cpp \
			link/ProductFilter_test.cpp \
	link/ReferenceLinker_test.cpp \		link/ReferenceLinker_test.cpp \
	link/TableMerger_test.cpp \		link/TableMerger_test.cpp \
	link/XmlReferenceLinker_test.cpp \		link/XmlReferenceLinker_test.cpp \

tools/aapt2/Debug.cpp

+3 −3

Original line number	Original line	Diff line number	Diff line
	@@ -144,8 +144,8 @@ void Debug::printTable(ResourceTable* table) {

	PrintVisitor visitor;		PrintVisitor visitor;
	for (const auto& value : entry->values) {		for (const auto& value : entry->values) {
	std::cout << " (" << value.config << ") ";		std::cout << " (" << value->config << ") ";
	value.value->accept(&visitor);		value->value->accept(&visitor);
	std::cout << std::endl;		std::cout << std::endl;
	}		}
	}		}
	@@ -176,7 +176,7 @@ void Debug::printStyleGraph(ResourceTable* table, const ResourceName& targetStyl
	if (result) {		if (result) {
	ResourceEntry* entry = result.value().entry;		ResourceEntry* entry = result.value().entry;
	for (const auto& value : entry->values) {		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) {		if (style->parent && style->parent.value().name) {
	parents.insert(style->parent.value().name.value());		parents.insert(style->parent.value().name.value());
	stylesToVisit.push(style->parent.value().name.value());		stylesToVisit.push(style->parent.value().name.value());

tools/aapt2/ResourceParser.cpp

+6 −40

Original line number	Original line	Diff line number	Diff line
	@@ -19,7 +19,6 @@
	#include "ResourceUtils.h"		#include "ResourceUtils.h"
	#include "ResourceValues.h"		#include "ResourceValues.h"
	#include "ValueVisitor.h"		#include "ValueVisitor.h"
	#include "util/Comparators.h"
	#include "util/ImmutableMap.h"		#include "util/ImmutableMap.h"
	#include "util/Util.h"		#include "util/Util.h"
	#include "xml/XmlPullParser.h"		#include "xml/XmlPullParser.h"
	@@ -71,6 +70,7 @@ static uint32_t parseFormatAttribute(const StringPiece16& str) {
	struct ParsedResource {		struct ParsedResource {
	ResourceName name;		ResourceName name;
	ConfigDescription config;		ConfigDescription config;
			std::string product;
	Source source;		Source source;
	ResourceId id;		ResourceId id;
	Maybe<SymbolState> symbolState;		Maybe<SymbolState> symbolState;
	@@ -79,35 +79,6 @@ struct ParsedResource {
	std::list<ParsedResource> childResources;		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.		// Recursively adds resources to the ResourceTable.
	static bool addResourcesToTable(ResourceTable* table, IDiagnostics* diag, ParsedResource* res) {		static bool addResourcesToTable(ResourceTable* table, IDiagnostics* diag, ParsedResource* res) {
	if (res->symbolState) {		if (res->symbolState) {
	@@ -125,7 +96,8 @@ static bool addResourcesToTable(ResourceTable* table, IDiagnostics* diag, Parsed
	res->value->setComment(std::move(res->comment));		res->value->setComment(std::move(res->comment));
	res->value->setSource(std::move(res->source));		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;		return false;
	}		}
	}		}
	@@ -295,9 +267,8 @@ bool ResourceParser::parseResources(xml::XmlPullParser* parser) {
	parsedResource.comment = std::move(comment);		parsedResource.comment = std::move(comment);

	// Extract the product name if it exists.		// Extract the product name if it exists.
	Maybe<std::u16string> product;
	if (Maybe<StringPiece16> maybeProduct = xml::findNonEmptyAttribute(parser, u"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.		// Parse the resource regardless of product.
	@@ -306,12 +277,7 @@ bool ResourceParser::parseResources(xml::XmlPullParser* parser) {
	continue;		continue;
	}		}

	// We successfully parsed the resource. Check if we should include it or strip it.		if (!addResourcesToTable(mTable, mDiag, &parsedResource)) {
	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)) {
	error = true;		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.		// 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>();		std::unique_ptr<Id> id = util::make_unique<Id>();
	id->setSource(mSource.withLine(beginXmlLine));		id->setSource(mSource.withLine(beginXmlLine));
	mTable->addResource(name, {}, std::move(id), mDiag);		mTable->addResource(name, {}, {}, std::move(id), mDiag);
	};		};

	// Process the raw value.		// Process the raw value.

tools/aapt2/ResourceParser.h

+0 −10

Original line number	Original line	Diff line number	Diff line
	@@ -33,13 +33,6 @@ namespace aapt {
	struct ParsedResource;		struct ParsedResource;

	struct ResourceParserOptions {		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.		* Whether the default setting for this parser is to allow translation.
	*/		*/
	@@ -106,9 +99,6 @@ private:
	bool parseArrayImpl(xml::XmlPullParser* parser, ParsedResource* outResource, uint32_t typeMask);		bool parseArrayImpl(xml::XmlPullParser* parser, ParsedResource* outResource, uint32_t typeMask);
	bool parsePlural(xml::XmlPullParser* parser, ParsedResource* outResource);		bool parsePlural(xml::XmlPullParser* parser, ParsedResource* outResource);

	bool shouldStripResource(const ResourceNameRef& name,
	const Maybe<std::u16string>& product) const;

	IDiagnostics* mDiag;		IDiagnostics* mDiag;
	ResourceTable* mTable;		ResourceTable* mTable;
	Source mSource;		Source mSource;

tools/aapt2/ResourceParser_test.cpp

+21 −39

Original line number	Original line	Diff line number	Diff line
	@@ -48,24 +48,13 @@ struct ResourceParserTest : public ::testing::Test {
	}		}

	::testing::AssertionResult testParse(const StringPiece& str) {		::testing::AssertionResult testParse(const StringPiece& str) {
	return testParse(str, ConfigDescription{}, {});		return testParse(str, ConfigDescription{});
	}		}

	::testing::AssertionResult testParse(const StringPiece& str, const ConfigDescription& config) {		::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);		std::stringstream input(kXmlPreamble);
	input << "<resources>\n" << str << "\n</resources>" << std::endl;		input << "<resources>\n" << str << "\n</resources>" << std::endl;
	ResourceParserOptions parserOptions;		ResourceParserOptions parserOptions;
	parserOptions.products = products;
	ResourceParser parser(mContext->getDiagnostics(), &mTable, Source{ "test" }, config,		ResourceParser parser(mContext->getDiagnostics(), &mTable, Source{ "test" }, config,
	parserOptions);		parserOptions);
	xml::XmlPullParser xmlParser(input);		xml::XmlPullParser xmlParser(input);
	@@ -546,7 +535,7 @@ TEST_F(ResourceParserTest, ParsePublicIdAsDefinition) {
	ASSERT_NE(nullptr, id);		ASSERT_NE(nullptr, id);
	}		}

	TEST_F(ResourceParserTest, FilterProductsThatDontMatch) {		TEST_F(ResourceParserTest, KeepAllProducts) {
	std::string input = R"EOF(		std::string input = R"EOF(
	<string name="foo" product="phone">hi</string>		<string name="foo" product="phone">hi</string>
	<string name="foo" product="no-sdcard">ho</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="bit" product="phablet">hoot</string>
	<string name="bot" product="default">yes</string>		<string name="bot" product="default">yes</string>
	)EOF";		)EOF";
	ASSERT_TRUE(testParse(input, { std::u16string(u"no-sdcard"), std::u16string(u"phablet") }));		ASSERT_TRUE(testParse(input));

	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);
	}

	TEST_F(ResourceParserTest, FailWhenProductFilterStripsOutAllVersionsOfResource) {		EXPECT_NE(nullptr, test::getValueForConfigAndProduct<String>(&mTable, u"@string/foo",
	std::string input = "<string name=\"foo\" product=\"tablet\">hello</string>\n";		ConfigDescription::defaultConfig(),
	ASSERT_FALSE(testParse(input, { std::u16string(u"phone") }));		"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) {		TEST_F(ResourceParserTest, AutoIncrementIdsInPublicGroup) {