Merge "AAPT2: Fix regression in Manifest.java permissions" (159d6776) · Commits · e / os / android_frameworks_base

tools/aapt2/java/ClassDefinition.cpp

+11 −0

Original line number	Diff line number	Diff line
		@@ -39,6 +39,17 @@ void MethodDefinition::WriteToStream(const StringPiece& prefix, bool final,
		*out << prefix << "}";
		}

		ClassDefinition::Result ClassDefinition::AddMember(std::unique_ptr<ClassMember> member) {
		Result result = Result::kAdded;
		auto iter = members_.find(member);
		if (iter != members_.end()) {
		members_.erase(iter);
		result = Result::kOverridden;
		}
		members_.insert(std::move(member));
		return result;
		}

		bool ClassDefinition::empty() const {
		for (const std::unique_ptr<ClassMember>& member : members_) {
		if (!member->empty()) {

tools/aapt2/java/ClassDefinition.h

+54 −9

Original line number	Diff line number	Diff line
		@@ -18,6 +18,7 @@
		#define AAPT_JAVA_CLASSDEFINITION_H

		#include <ostream>
		#include <set>
		#include <string>

		#include "android-base/macros.h"
		@@ -37,10 +38,14 @@ class ClassMember {
		public:
		virtual ~ClassMember() = default;

		AnnotationProcessor* GetCommentBuilder() { return &processor_; }
		AnnotationProcessor* GetCommentBuilder() {
		return &processor_;
		}

		virtual bool empty() const = 0;

		virtual const std::string& GetName() const = 0;

		// Writes the class member to the out stream. Subclasses should derive this method
		// to write their own data. Call this base method from the subclass to write out
		// this member's comments/annotations.
		@@ -57,7 +62,13 @@ class PrimitiveMember : public ClassMember {
		PrimitiveMember(const android::StringPiece& name, const T& val)
		: name_(name.to_string()), val_(val) {}

		bool empty() const override { return false; }
		bool empty() const override {
		return false;
		}

		const std::string& GetName() const override {
		return name_;
		}

		void WriteToStream(const android::StringPiece& prefix, bool final,
		std::ostream* out) const override {
		@@ -83,7 +94,13 @@ class PrimitiveMember<std::string> : public ClassMember {
		PrimitiveMember(const android::StringPiece& name, const std::string& val)
		: name_(name.to_string()), val_(val) {}

		bool empty() const override { return false; }
		bool empty() const override {
		return false;
		}

		const std::string& GetName() const override {
		return name_;
		}

		void WriteToStream(const android::StringPiece& prefix, bool final,
		std::ostream* out) const override {
		@@ -109,9 +126,17 @@ class PrimitiveArrayMember : public ClassMember {
		public:
		explicit PrimitiveArrayMember(const android::StringPiece& name) : name_(name.to_string()) {}

		void AddElement(const T& val) { elements_.push_back(val); }
		void AddElement(const T& val) {
		elements_.push_back(val);
		}

		bool empty() const override { return false; }
		bool empty() const override {
		return false;
		}

		const std::string& GetName() const override {
		return name_;
		}

		void WriteToStream(const android::StringPiece& prefix, bool final,
		std::ostream* out) const override {
		@@ -154,6 +179,11 @@ class MethodDefinition : public ClassMember {
		// formatting may be broken.
		void AppendStatement(const android::StringPiece& statement);

		// Not quite the same as a name, but good enough.
		const std::string& GetName() const override {
		return signature_;
		}

		// Even if the method is empty, we always want to write the method signature.
		bool empty() const override { return false; }

		@@ -175,19 +205,34 @@ class ClassDefinition : public ClassMember {
		ClassDefinition(const android::StringPiece& name, ClassQualifier qualifier, bool createIfEmpty)
		: name_(name.to_string()), qualifier_(qualifier), create_if_empty_(createIfEmpty) {}

		void AddMember(std::unique_ptr<ClassMember> member) {
		members_.push_back(std::move(member));
		}
		enum class Result {
		kAdded,
		kOverridden,
		};

		Result AddMember(std::unique_ptr<ClassMember> member);

		bool empty() const override;

		const std::string& GetName() const override {
		return name_;
		}

		void WriteToStream(const android::StringPiece& prefix, bool final,
		std::ostream* out) const override;

		private:
		struct ClassMemberCompare {
		using T = std::unique_ptr<ClassMember>;
		bool operator()(const T& a, const T& b) const {
		return a->GetName() < b->GetName();
		}
		};

		std::string name_;
		ClassQualifier qualifier_;
		bool create_if_empty_;
		std::vector<std::unique_ptr<ClassMember>> members_;
		std::set<std::unique_ptr<ClassMember>, ClassMemberCompare> members_;

		DISALLOW_COPY_AND_ASSIGN(ClassDefinition);
		};

tools/aapt2/java/ManifestClassGenerator.cpp

+4 −1

Original line number	Diff line number	Diff line
		@@ -68,7 +68,10 @@ static bool WriteSymbol(const Source& source, IDiagnostics* diag, xml::Element*
		util::make_unique<StringMember>(result.value(), attr->value);
		string_member->GetCommentBuilder()->AppendComment(el->comment);

		class_def->AddMember(std::move(string_member));
		if (class_def->AddMember(std::move(string_member)) == ClassDefinition::Result::kOverridden) {
		diag->Warn(DiagMessage(source.WithLine(el->line_number))
		<< "duplicate definitions of '" << result.value() << "', overriding previous");
		}
		return true;
		}

tools/aapt2/java/ManifestClassGenerator_test.cpp

+15 −0

Original line number	Diff line number	Diff line
		@@ -121,6 +121,21 @@ TEST(ManifestClassGeneratorTest, CommentsAndAnnotationsArePresent) {
		EXPECT_THAT(actual, HasSubstr(expected_test));
		}

		// This is bad but part of public API behaviour so we need to preserve it.
		TEST(ManifestClassGeneratorTest, LastSeenPermissionWithSameLeafNameTakesPrecedence) {
		std::unique_ptr<IAaptContext> context = test::ContextBuilder().Build();
		std::unique_ptr<xml::XmlResource> manifest = test::BuildXmlDom(R"(
		<manifest xmlns:android="http://schemas.android.com/apk/res/android">
		<permission android:name="android.permission.ACCESS_INTERNET" />
		<permission android:name="com.android.aapt.test.ACCESS_INTERNET" />
		</manifest>)");

		std::string actual;
		ASSERT_TRUE(GetManifestClassText(context.get(), manifest.get(), &actual));
		EXPECT_THAT(actual, HasSubstr("ACCESS_INTERNET=\"com.android.aapt.test.ACCESS_INTERNET\";"));
		EXPECT_THAT(actual, Not(HasSubstr("ACCESS_INTERNET=\"android.permission.ACCESS_INTERNET\";")));
		}

		static ::testing::AssertionResult GetManifestClassText(IAaptContext* context, xml::XmlResource* res,
		std::string* out_str) {
		std::unique_ptr<ClassDefinition> manifest_class =