Fix Font::writeTo to init mTypeface if needed.

mTypeface may not be initialized yet when serializing a deserialized
Font again.
This CL also adds tests for re-serialization
(serialize -> deserialize -> serialize).

Bug: 172891184
Test: atest minikin_tests
Change-Id: I910ead49b9b9929a9b6ac01da5110470448c029a

8 files changed, 78 insertions, 50 deletions

diff --git a/include/minikin/Font.h b/include/minikin/Font.h
index 36265c7..bd31429 100644
--- a/include/minikin/Font.h
+++ b/include/minikin/Font.h
@@ -119,7 +119,7 @@ public:
     template <TypefaceWriter typefaceWriter>
     void writeTo(BufferWriter* writer) const {
         mStyle.writeTo(writer);
-        typefaceWriter(writer, mTypeface.get());
+        typefaceWriter(writer, typeface().get());
     }
 
     const std::shared_ptr<MinikinFont>& typeface() const;
diff --git a/libs/minikin/FontCollection.cpp b/libs/minikin/FontCollection.cpp
index e4e41d5..26695e9 100644
--- a/libs/minikin/FontCollection.cpp
+++ b/libs/minikin/FontCollection.cpp
@@ -127,10 +127,8 @@ FontCollection::FontCollection(BufferReader* reader,
     static_assert(sizeof(Range) == 4);
     std::tie(mRanges, mRangesCount) = reader->readArray<Range>();
     std::tie(mFamilyVec, mFamilyVecCount) = reader->readArray<uint8_t>();
-    uint32_t supportedAxesCount = reader->read<uint32_t>();
-    for (uint32_t i = 0; i < supportedAxesCount; i++) {
-        mSupportedAxes.insert(reader->read<AxisTag>());
-    }
+    const auto& [axesPtr, axesCount] = reader->readArray<AxisTag>();
+    mSupportedAxes.insert(axesPtr, axesPtr + axesCount);
 }
 
 void FontCollection::writeTo(BufferWriter* writer,
@@ -150,10 +148,10 @@ void FontCollection::writeTo(BufferWriter* writer,
     writer->writeArray<Range>(mRanges, mRangesCount);
     writer->writeArray<uint8_t>(mFamilyVec, mFamilyVecCount);
     // No need to serialize mVSFamilyVec as it can be reconstructed easily from mFamilies.
-    writer->write<uint32_t>(mSupportedAxes.size());
-    for (const AxisTag& axis : mSupportedAxes) {
-        writer->write<AxisTag>(axis);
-    }
+    std::vector<AxisTag> axes(mSupportedAxes.begin(), mSupportedAxes.end());
+    // Sort axes to be deterministic.
+    std::sort(axes.begin(), axes.end());
+    writer->writeArray<AxisTag>(axes.data(), axes.size());
 }
 
 // static
diff --git a/libs/minikin/FontFamily.cpp b/libs/minikin/FontFamily.cpp
index 95b311a..743ee3a 100644
--- a/libs/minikin/FontFamily.cpp
+++ b/libs/minikin/FontFamily.cpp
@@ -18,6 +18,7 @@
 
 #include "minikin/FontFamily.h"
 
+#include <algorithm>
 #include <cstdint>
 #include <vector>
 
@@ -168,12 +169,10 @@ std::shared_ptr<FontFamily> FontFamily::readFromInternal(
     // FamilyVariant is uint8_t
     static_assert(sizeof(FamilyVariant) == 1);
     FamilyVariant variant = reader->read<FamilyVariant>();
-    uint32_t axesCount = reader->read<uint32_t>();
-    std::unordered_set<AxisTag> supportedAxes;
-    supportedAxes.reserve(axesCount);
-    for (uint32_t i = 0; i < axesCount; i++) {
-        supportedAxes.insert(reader->read<AxisTag>());
-    }
+    // AxisTag is uint32_t
+    static_assert(sizeof(AxisTag) == 4);
+    const auto& [axesPtr, axesCount] = reader->readArray<AxisTag>();
+    std::unordered_set<AxisTag> supportedAxes(axesPtr, axesPtr + axesCount);
     bool isColorEmoji = static_cast<bool>(reader->read<uint8_t>());
     bool isCustomFallback = static_cast<bool>(reader->read<uint8_t>());
     SparseBitSet coverage(reader);
@@ -195,12 +194,10 @@ std::shared_ptr<FontFamily> FontFamily::readFromInternal(
 void FontFamily::writeToInternal(BufferWriter* writer) const {
     LocaleListCache::writeTo(writer, mLocaleListId);
     writer->write<FamilyVariant>(mVariant);
-    writer->write<uint32_t>(mSupportedAxes.size());
-    // AxisTag is uint32_t
-    static_assert(sizeof(AxisTag) == 4);
-    for (const AxisTag& axis : mSupportedAxes) {
-        writer->write<AxisTag>(axis);
-    }
+    std::vector<AxisTag> axes(mSupportedAxes.begin(), mSupportedAxes.end());
+    // Sort axes to be deterministic.
+    std::sort(axes.begin(), axes.end());
+    writer->writeArray<AxisTag>(axes.data(), axes.size());
     writer->write<uint8_t>(mIsColorEmoji);
     writer->write<uint8_t>(mIsCustomFallback);
     mCoverage.writeTo(writer);
diff --git a/tests/unittest/FontCollectionTest.cpp b/tests/unittest/FontCollectionTest.cpp
index 8c0eff3..40b6bf2 100644
--- a/tests/unittest/FontCollectionTest.cpp
+++ b/tests/unittest/FontCollectionTest.cpp
@@ -177,26 +177,29 @@ TEST(FontCollectionTest, createWithVariations) {
     }
 }
 
-std::vector<std::shared_ptr<FontCollection>> copyFontCollectionsByBuffer(
+std::vector<uint8_t> writeToBuffer(
         const std::vector<std::shared_ptr<FontCollection>>& collections) {
     BufferWriter fakeWriter(nullptr);
     FontCollection::writeVector<writeFreeTypeMinikinFontForTest>(&fakeWriter, collections);
     std::vector<uint8_t> buffer(fakeWriter.size());
     BufferWriter writer(buffer.data());
     FontCollection::writeVector<writeFreeTypeMinikinFontForTest>(&writer, collections);
-    BufferReader reader(buffer.data());
-    return FontCollection::readVector<readFreeTypeMinikinFontForTest>(&reader);
+    return buffer;
 }
 
 TEST(FontCollectionTest, bufferTest) {
     {
         std::vector<std::shared_ptr<FontCollection>> original({buildFontCollection(kVsTestFont)});
-        auto copied = copyFontCollectionsByBuffer(original);
+        std::vector<uint8_t> buffer = writeToBuffer(original);
+        BufferReader reader(buffer.data());
+        auto copied = FontCollection::readVector<readFreeTypeMinikinFontForTest>(&reader);
         EXPECT_EQ(1u, copied.size());
         expectVSGlyphsForVsTestFont(copied[0].get());
         EXPECT_EQ(original[0]->getSupportedTags(), copied[0]->getSupportedTags());
         // Id will be different.
         EXPECT_NE(original[0]->getId(), copied[0]->getId());
+        std::vector<uint8_t> newBuffer = writeToBuffer(copied);
+        EXPECT_EQ(buffer, newBuffer);
     }
     {
         // Test that FontFamily instances are shared.
@@ -204,9 +207,13 @@ TEST(FontCollectionTest, bufferTest) {
         auto fc1 = std::make_shared<FontCollection>(families);
         auto fc2 = std::make_shared<FontCollection>(families);
         std::vector<std::shared_ptr<FontCollection>> original({fc1, fc2});
-        auto copied = copyFontCollectionsByBuffer(original);
+        std::vector<uint8_t> buffer = writeToBuffer(original);
+        BufferReader reader(buffer.data());
+        auto copied = FontCollection::readVector<readFreeTypeMinikinFontForTest>(&reader);
         EXPECT_EQ(2u, copied.size());
         EXPECT_EQ(copied[0]->mFamilies[0], copied[1]->mFamilies[0]);
+        std::vector<uint8_t> newBuffer = writeToBuffer(copied);
+        EXPECT_EQ(buffer, newBuffer);
     }
     {
         // Test axes.
@@ -214,12 +221,16 @@ TEST(FontCollectionTest, bufferTest) {
         const char kMultiAxisFont[] = "MultiAxis.ttf";
         std::vector<std::shared_ptr<FontCollection>> original(
                 {buildFontCollection(kMultiAxisFont)});
-        auto copied = copyFontCollectionsByBuffer(original);
+        std::vector<uint8_t> buffer = writeToBuffer(original);
+        BufferReader reader(buffer.data());
+        auto copied = FontCollection::readVector<readFreeTypeMinikinFontForTest>(&reader);
         EXPECT_EQ(1u, copied.size());
         EXPECT_EQ(1u,
                   copied[0]->getSupportedTags().count(MinikinFont::MakeTag('w', 'd', 't', 'h')));
         EXPECT_EQ(1u,
                   copied[0]->getSupportedTags().count(MinikinFont::MakeTag('w', 'g', 'h', 't')));
+        std::vector<uint8_t> newBuffer = writeToBuffer(copied);
+        EXPECT_EQ(buffer, newBuffer);
     }
 }
 
diff --git a/tests/unittest/FontFamilyTest.cpp b/tests/unittest/FontFamilyTest.cpp
index 7f575b3..fd2fc9a 100644
--- a/tests/unittest/FontFamilyTest.cpp
+++ b/tests/unittest/FontFamilyTest.cpp
@@ -794,21 +794,15 @@ TEST_F(FontFamilyTest, closestMatch) {
     }
 }
 
-std::shared_ptr<FontFamily> copyFontFamilyByBuffer(const FontFamily& fontFamily) {
-    std::vector<uint8_t> buffer =
-            allocateBuffer<FontFamily, writeFreeTypeMinikinFontForTest>(fontFamily);
-    BufferWriter writer(buffer.data());
-    fontFamily.writeTo<writeFreeTypeMinikinFontForTest>(&writer);
-
-    BufferReader reader(buffer.data());
-    return FontFamily::readFrom<readFreeTypeMinikinFontForTest>(&reader);
-}
-
 TEST_F(FontFamilyTest, bufferTest) {
     {
         // Font with variation selectors
         std::shared_ptr<FontFamily> original = buildFontFamily(kVsTestFont);
-        std::shared_ptr<FontFamily> copied = copyFontFamilyByBuffer(*original);
+        std::vector<uint8_t> buffer =
+                writeToBuffer<FontFamily, writeFreeTypeMinikinFontForTest>(*original);
+        BufferReader reader(buffer.data());
+        std::shared_ptr<FontFamily> copied =
+                FontFamily::readFrom<readFreeTypeMinikinFontForTest>(&reader);
         ASSERT_EQ(original->localeListId(), copied->localeListId());
         ASSERT_EQ(original->variant(), copied->variant());
         ASSERT_EQ(original->getNumFonts(), copied->getNumFonts());
@@ -817,13 +811,23 @@ TEST_F(FontFamilyTest, bufferTest) {
         ASSERT_EQ(original->isCustomFallback(), copied->isCustomFallback());
         ASSERT_EQ(original->hasVSTable(), copied->hasVSTable());
         expectVSGlyphsForVsTestFont(copied.get());
+        std::vector<uint8_t> newBuffer =
+                writeToBuffer<FontFamily, writeFreeTypeMinikinFontForTest>(*copied);
+        ASSERT_EQ(buffer, newBuffer);
     }
     {
         // Font with axes
         constexpr char kMultiAxisFont[] = "MultiAxis.ttf";
         std::shared_ptr<FontFamily> original = buildFontFamily(kMultiAxisFont);
-        std::shared_ptr<FontFamily> copied = copyFontFamilyByBuffer(*original);
+        std::vector<uint8_t> buffer =
+                writeToBuffer<FontFamily, writeFreeTypeMinikinFontForTest>(*original);
+        BufferReader reader(buffer.data());
+        std::shared_ptr<FontFamily> copied =
+                FontFamily::readFrom<readFreeTypeMinikinFontForTest>(&reader);
         ASSERT_EQ(original->supportedAxes(), copied->supportedAxes());
+        std::vector<uint8_t> newBuffer =
+                writeToBuffer<FontFamily, writeFreeTypeMinikinFontForTest>(*copied);
+        ASSERT_EQ(buffer, newBuffer);
     }
 }
 
diff --git a/tests/unittest/FontTest.cpp b/tests/unittest/FontTest.cpp
index 7d08e06..dc26ab2 100644
--- a/tests/unittest/FontTest.cpp
+++ b/tests/unittest/FontTest.cpp
@@ -27,15 +27,15 @@ namespace minikin {
 TEST(FontTest, BufferTest) {
     auto minikinFont = std::make_shared<FreeTypeMinikinFontForTest>(getTestFontPath("Ascii.ttf"));
     std::shared_ptr<Font> original = Font::Builder(minikinFont).build();
-    std::vector<uint8_t> buffer = allocateBuffer<Font, writeFreeTypeMinikinFontForTest>(*original);
-    BufferWriter writer(buffer.data());
-    original->writeTo<writeFreeTypeMinikinFontForTest>(&writer);
+    std::vector<uint8_t> buffer = writeToBuffer<Font, writeFreeTypeMinikinFontForTest>(*original);
 
     BufferReader reader(buffer.data());
     std::shared_ptr<Font> font = Font::readFrom<readFreeTypeMinikinFontForTest>(&reader);
     EXPECT_EQ(minikinFont->GetFontPath(), font->typeface()->GetFontPath());
     EXPECT_EQ(original->style(), font->style());
     EXPECT_NE(nullptr, font->baseFont());
+    std::vector<uint8_t> newBuffer = writeToBuffer<Font, writeFreeTypeMinikinFontForTest>(*font);
+    EXPECT_EQ(buffer, newBuffer);
 }
 
 }  // namespace minikin
diff --git a/tests/unittest/SparseBitSetTest.cpp b/tests/unittest/SparseBitSetTest.cpp
index 2d85cf8..8c67964 100644
--- a/tests/unittest/SparseBitSetTest.cpp
+++ b/tests/unittest/SparseBitSetTest.cpp
@@ -57,26 +57,26 @@ TEST(SparseBitSetTest, randomTest) {
 TEST(SparseBitSetTest, bufferTest) {
     std::vector<uint32_t> range({10, 20});
     SparseBitSet originalBitset(range.data(), range.size() / 2);
-    std::vector<uint8_t> buffer = allocateBuffer(originalBitset);
-    BufferWriter writer(buffer.data());
-    originalBitset.writeTo(&writer);
+    std::vector<uint8_t> buffer = writeToBuffer(originalBitset);
     BufferReader reader(buffer.data());
     SparseBitSet bitset(&reader);
 
     for (size_t i = 0; i < 10; ++i) ASSERT_FALSE(bitset.get(i)) << i;
     for (size_t i = 10; i < 20; ++i) ASSERT_TRUE(bitset.get(i)) << i;
     for (size_t i = 20; i < 30; ++i) ASSERT_FALSE(bitset.get(i)) << i;
+    std::vector<uint8_t> newBuffer = writeToBuffer(bitset);
+    ASSERT_EQ(buffer, newBuffer);
 }
 
 TEST(SparseBitSetTest, emptyBitSetBufferTest) {
     SparseBitSet empty;
-    std::vector<uint8_t> buffer = allocateBuffer(empty);
-    BufferWriter writer(buffer.data());
-    empty.writeTo(&writer);
+    std::vector<uint8_t> buffer = writeToBuffer(empty);
     BufferReader reader(buffer.data());
     SparseBitSet bitset(&reader);
 
     ASSERT_FALSE(bitset.get(0));
+    std::vector<uint8_t> newBuffer = writeToBuffer(bitset);
+    ASSERT_EQ(buffer, newBuffer);
 }
 
 }  // namespace minikin
diff --git a/tests/util/BufferUtils.h b/tests/util/BufferUtils.h
index 3d44499..355e74e 100644
--- a/tests/util/BufferUtils.h
+++ b/tests/util/BufferUtils.h
@@ -26,14 +26,32 @@ template <class T>
 std::vector<uint8_t> allocateBuffer(const T& t) {
     BufferWriter writer(nullptr);
     t.writeTo(&writer);
-    return std::vector<uint8_t>(writer.size());
+    // Fill with 0xFF for debugging.
+    return std::vector<uint8_t>(writer.size(), 0xFFu);
 }
 
 template <class T, auto arg>
 std::vector<uint8_t> allocateBuffer(const T& t) {
     BufferWriter writer(nullptr);
     t.template writeTo<arg>(&writer);
-    return std::vector<uint8_t>(writer.size());
+    // Fill with 0xFF for debugging.
+    return std::vector<uint8_t>(writer.size(), 0xFFu);
+}
+
+template <class T>
+std::vector<uint8_t> writeToBuffer(const T& t) {
+    std::vector<uint8_t> buffer = allocateBuffer(t);
+    BufferWriter writer(buffer.data());
+    t.writeTo(&writer);
+    return buffer;
+}
+
+template <class T, auto arg>
+std::vector<uint8_t> writeToBuffer(const T& t) {
+    std::vector<uint8_t> buffer = allocateBuffer<T, arg>(t);
+    BufferWriter writer(buffer.data());
+    t.template writeTo<arg>(&writer);
+    return buffer;
 }
 
 }  // namespace minikin