diff --git a/tools/aapt2/optimize/ResourcePathShortener.cpp b/tools/aapt2/optimize/ResourcePathShortener.cpp index 7f5d10475f93a..7ff9bf5aa8df1 100644 --- a/tools/aapt2/optimize/ResourcePathShortener.cpp +++ b/tools/aapt2/optimize/ResourcePathShortener.cpp @@ -16,6 +16,7 @@ #include "optimize/ResourcePathShortener.h" +#include #include #include "androidfw/StringPiece.h" @@ -71,10 +72,19 @@ std::string GetShortenedPath(const android::StringPiece& shortened_filename, return shortened_path; } +// implement custom comparator of FileReference pointers so as to use the +// underlying filepath as key rather than the integer address. This is to ensure +// determinism of output for colliding files. +struct PathComparator { + bool operator() (const FileReference* lhs, const FileReference* rhs) const { + return lhs->path->compare(*rhs->path); + } +}; + bool ResourcePathShortener::Consume(IAaptContext* context, ResourceTable* table) { // used to detect collisions std::unordered_set shortened_paths; - std::unordered_set file_refs; + std::set file_refs; for (auto& package : table->packages) { for (auto& type : package->types) { for (auto& entry : type->entries) { diff --git a/tools/aapt2/optimize/ResourcePathShortener_test.cpp b/tools/aapt2/optimize/ResourcePathShortener_test.cpp index 1f45694951868..f5a02be0ea5ef 100644 --- a/tools/aapt2/optimize/ResourcePathShortener_test.cpp +++ b/tools/aapt2/optimize/ResourcePathShortener_test.cpp @@ -29,6 +29,14 @@ android::StringPiece GetExtension(android::StringPiece path) { return android::StringPiece(iter, path.end() - iter); } +void FillTable(aapt::test::ResourceTableBuilder& builder, int start, int end) { + for (int i=start; i context = test::ContextBuilder().Build(); + + // 4000 resources is the limit at which the hash space is expanded to 3 + // letters to reduce collisions, we want as many collisions as possible thus + // N-1. + const auto kNumResources = 3999; + const auto kNumTries = 5; + + test::ResourceTableBuilder builder1; + FillTable(builder1, 0, kNumResources); + std::unique_ptr table1 = builder1.Build(); + std::map expected_mapping; + ASSERT_TRUE(ResourcePathShortener(expected_mapping).Consume(context.get(), table1.get())); + + // We are trying to ensure lack of non-determinism, it is not simple to prove + // a negative, thus we must try the test a few times so that the test itself + // is non-flaky. Basically create the pathmap 5 times from the same set of + // resources but a different order of addition and then ensure they are always + // mapped to the same short path. + for (int i=0; i table2 = builder2.Build(); + + std::map actual_mapping; + ASSERT_TRUE(ResourcePathShortener(actual_mapping).Consume(context.get(), table2.get())); + + for (auto& item : actual_mapping) { + ASSERT_THAT(expected_mapping[item.first], Eq(item.second)); + } + } +} + } // namespace aapt