Revert "Fix loaded apk string pool order"
This reverts commit 4e9a922ede24f7f7bfe793321f7328623ee2a061.
Reason for revert: <b/122518436>
Change-Id: I3650b2c6c9bdfa69a3034f9ca49e95a9698c3cdd
diff --git a/tools/aapt2/Debug.cpp b/tools/aapt2/Debug.cpp
index 5f664f5..9832485 100644
--- a/tools/aapt2/Debug.cpp
+++ b/tools/aapt2/Debug.cpp
@@ -435,7 +435,7 @@
const size_t NS = pool->size();
for (size_t s=0; s<NS; s++) {
String8 str = pool->string8ObjectAt(s);
- printer->Print(StringPrintf("String #%zd: %s\n", s, str.string()));
+ printer->Print(StringPrintf("String #%zd : %s\n", s, str.string()));
}
}
diff --git a/tools/aapt2/ResourceUtils.cpp b/tools/aapt2/ResourceUtils.cpp
index ab4805f..0032960 100644
--- a/tools/aapt2/ResourceUtils.cpp
+++ b/tools/aapt2/ResourceUtils.cpp
@@ -727,7 +727,7 @@
// This must be a FileReference.
std::unique_ptr<FileReference> file_ref =
util::make_unique<FileReference>(dst_pool->MakeRef(
- str, StringPool::Context(StringPool::Context::kHighPriority, config), data));
+ str, StringPool::Context(StringPool::Context::kHighPriority, config)));
if (type == ResourceType::kRaw) {
file_ref->type = ResourceFile::Type::kUnknown;
} else if (util::EndsWith(*file_ref->path, ".xml")) {
@@ -739,7 +739,7 @@
}
// There are no styles associated with this string, so treat it as a simple string.
- return util::make_unique<String>(dst_pool->MakeRef(str, StringPool::Context(config), data));
+ return util::make_unique<String>(dst_pool->MakeRef(str, StringPool::Context(config)));
}
} break;
diff --git a/tools/aapt2/StringPool.cpp b/tools/aapt2/StringPool.cpp
index a8c2666..8eabd32 100644
--- a/tools/aapt2/StringPool.cpp
+++ b/tools/aapt2/StringPool.cpp
@@ -165,13 +165,12 @@
return MakeRefImpl(str, Context{}, true);
}
-StringPool::Ref StringPool::MakeRef(const StringPiece& str, const Context& context,
- Maybe<size_t> index) {
- return MakeRefImpl(str, context, true, index);
+StringPool::Ref StringPool::MakeRef(const StringPiece& str, const Context& context) {
+ return MakeRefImpl(str, context, true);
}
StringPool::Ref StringPool::MakeRefImpl(const StringPiece& str, const Context& context,
- bool unique, Maybe<size_t> index) {
+ bool unique) {
if (unique) {
auto range = indexed_strings_.equal_range(str);
for (auto iter = range.first; iter != range.second; ++iter) {
@@ -181,26 +180,15 @@
}
}
- const size_t size = strings_.size();
- // Insert the string at the end of the string vector if no index is specified
- const size_t insertion_index = index ? index.value() : size;
-
std::unique_ptr<Entry> entry(new Entry());
entry->value = str.to_string();
entry->context = context;
- entry->index_ = insertion_index;
+ entry->index_ = strings_.size();
entry->ref_ = 0;
entry->pool_ = this;
Entry* borrow = entry.get();
- if (insertion_index == size) {
- strings_.emplace_back(std::move(entry));
- } else {
- // Allocate enough space for the string at the index
- strings_.resize(std::max(insertion_index + 1, size));
- strings_[insertion_index] = std::move(entry);
- }
-
+ strings_.emplace_back(std::move(entry));
indexed_strings_.insert(std::make_pair(StringPiece(borrow->value), borrow));
return Ref(borrow);
}
diff --git a/tools/aapt2/StringPool.h b/tools/aapt2/StringPool.h
index 115d5d3..1006ca9 100644
--- a/tools/aapt2/StringPool.h
+++ b/tools/aapt2/StringPool.h
@@ -166,8 +166,7 @@
// Adds a string to the pool, unless it already exists, with a context object that can be used
// when sorting the string pool. Returns a reference to the string in the pool.
- Ref MakeRef(const android::StringPiece& str, const Context& context,
- Maybe<size_t> index = {});
+ Ref MakeRef(const android::StringPiece& str, const Context& context);
// Adds a string from another string pool. Returns a reference to the string in the string pool.
Ref MakeRef(const Ref& ref);
@@ -211,8 +210,7 @@
static bool Flatten(BigBuffer* out, const StringPool& pool, bool utf8, IDiagnostics* diag);
- Ref MakeRefImpl(const android::StringPiece& str, const Context& context, bool unique,
- Maybe<size_t> index = {});
+ Ref MakeRefImpl(const android::StringPiece& str, const Context& context, bool unique);
void ReAssignIndices();
std::vector<std::unique_ptr<Entry>> strings_;
diff --git a/tools/aapt2/StringPool_test.cpp b/tools/aapt2/StringPool_test.cpp
index 648be7d..9a7238b 100644
--- a/tools/aapt2/StringPool_test.cpp
+++ b/tools/aapt2/StringPool_test.cpp
@@ -84,24 +84,6 @@
EXPECT_THAT(ref_c.index(), Eq(2u));
}
-TEST(StringPoolTest, AssignStringIndex) {
- StringPool pool;
-
- StringPool::Ref ref_a = pool.MakeRef("0", StringPool::Context{}, 0u);
- StringPool::Ref ref_b = pool.MakeRef("1", StringPool::Context{}, 1u);
- StringPool::Ref ref_c = pool.MakeRef("5", StringPool::Context{}, 5u);
- StringPool::Ref ref_d = pool.MakeRef("2", StringPool::Context{}, 2u);
- StringPool::Ref ref_e = pool.MakeRef("4", StringPool::Context{}, 4u);
- StringPool::Ref ref_f = pool.MakeRef("3", StringPool::Context{}, 3u);
-
- EXPECT_THAT(ref_a.index(), Eq(0u));
- EXPECT_THAT(ref_b.index(), Eq(1u));
- EXPECT_THAT(ref_d.index(), Eq(2u));
- EXPECT_THAT(ref_f.index(), Eq(3u));
- EXPECT_THAT(ref_e.index(), Eq(4u));
- EXPECT_THAT(ref_c.index(), Eq(5u));
-}
-
TEST(StringPoolTest, PruneStringsWithNoReferences) {
StringPool pool;
diff --git a/tools/aapt2/cmd/Convert.cpp b/tools/aapt2/cmd/Convert.cpp
index 7a74ba9..0cf86cc 100644
--- a/tools/aapt2/cmd/Convert.cpp
+++ b/tools/aapt2/cmd/Convert.cpp
@@ -43,7 +43,8 @@
class IApkSerializer {
public:
- IApkSerializer(IAaptContext* context, const Source& source) : context_(context), source_(source) {}
+ IApkSerializer(IAaptContext* context, const Source& source) : context_(context),
+ source_(source) {}
virtual bool SerializeXml(const xml::XmlResource* xml, const std::string& path, bool utf16,
IArchiveWriter* writer, uint32_t compression_flags) = 0;
@@ -167,7 +168,7 @@
std::unique_ptr<io::IData> data = file->file->OpenAsData();
if (!data) {
context_->GetDiagnostics()->Error(DiagMessage(source_)
- << "failed to open file " << *file->path);
+ << "failed to open file " << *file->path);
return false;
}
@@ -175,7 +176,7 @@
std::unique_ptr<xml::XmlResource> xml = xml::Inflate(data->data(), data->size(), &error);
if (xml == nullptr) {
context_->GetDiagnostics()->Error(DiagMessage(source_) << "failed to parse binary XML: "
- << error);
+ << error);
return false;
}
@@ -256,9 +257,6 @@
int Convert(IAaptContext* context, LoadedApk* apk, IArchiveWriter* output_writer,
ApkFormat output_format, TableFlattenerOptions table_flattener_options,
XmlFlattenerOptions xml_flattener_options) {
- // Do not change the ordering of strings in the values string pool
- table_flattener_options.sort_stringpool_entries = false;
-
unique_ptr<IApkSerializer> serializer;
if (output_format == ApkFormat::kBinary) {
serializer.reset(new BinaryApkSerializer(context, apk->GetSource(), table_flattener_options,
@@ -274,7 +272,7 @@
io::IFile* manifest = apk->GetFileCollection()->FindFile(kAndroidManifestPath);
if (!serializer->SerializeXml(apk->GetManifest(), kAndroidManifestPath, true /*utf16*/,
output_writer, (manifest != nullptr && manifest->WasCompressed())
- ? ArchiveEntry::kCompress : 0u)) {
+ ? ArchiveEntry::kCompress : 0u)) {
context->GetDiagnostics()->Error(DiagMessage(apk->GetSource())
<< "failed to serialize AndroidManifest.xml");
return 1;
@@ -303,8 +301,7 @@
if (files_written.insert(*file->path).second) {
if (!serializer->SerializeFile(file, output_writer)) {
context->GetDiagnostics()->Error(DiagMessage(apk->GetSource())
- << "failed to serialize file "
- << *file->path);
+ << "failed to serialize file " << *file->path);
return 1;
}
}
@@ -338,7 +335,7 @@
if (!io::CopyFileToArchivePreserveCompression(context, file, path, output_writer)) {
context->GetDiagnostics()->Error(DiagMessage(apk->GetSource())
- << "failed to copy file " << path);
+ << "failed to copy file " << path);
return 1;
}
}
diff --git a/tools/aapt2/format/binary/TableFlattener.cpp b/tools/aapt2/format/binary/TableFlattener.cpp
index 9d341cc..c8bb465 100644
--- a/tools/aapt2/format/binary/TableFlattener.cpp
+++ b/tools/aapt2/format/binary/TableFlattener.cpp
@@ -702,17 +702,15 @@
} // namespace
bool TableFlattener::Consume(IAaptContext* context, ResourceTable* table) {
- if (options_.sort_stringpool_entries) {
- // We must do this before writing the resources, since the string pool IDs may change.
- table->string_pool.Prune();
- table->string_pool.Sort([](const StringPool::Context &a, const StringPool::Context &b) -> int {
- int diff = util::compare(a.priority, b.priority);
- if (diff == 0) {
- diff = a.config.compare(b.config);
- }
- return diff;
- });
- }
+ // We must do this before writing the resources, since the string pool IDs may change.
+ table->string_pool.Prune();
+ table->string_pool.Sort([](const StringPool::Context& a, const StringPool::Context& b) -> int {
+ int diff = util::compare(a.priority, b.priority);
+ if (diff == 0) {
+ diff = a.config.compare(b.config);
+ }
+ return diff;
+ });
// Write the ResTable header.
ChunkWriter table_writer(buffer_);
diff --git a/tools/aapt2/format/binary/TableFlattener.h b/tools/aapt2/format/binary/TableFlattener.h
index 71330e3..73c1729 100644
--- a/tools/aapt2/format/binary/TableFlattener.h
+++ b/tools/aapt2/format/binary/TableFlattener.h
@@ -44,9 +44,6 @@
// Set of whitelisted resource names to avoid altering in key stringpool
std::set<std::string> whitelisted_resources;
- // When true, sort the entries in the values string pool by priority and configuration.
- bool sort_stringpool_entries = true;
-
// Map from original resource paths to shortened resource paths.
std::map<std::string, std::string> shortened_path_map;
};