Merge "AAPT2: Allow merging of Style attributes from overlays" into oc-dev

am: f42c86660d

Change-Id: I4a52a16fff3d544d8d7d63d8a482ba7e2111d01a

tools/aapt2/Android.bp

@@ -160,7 +160,7 @@ cc_library_host_shared {
 cc_test_host {
     name: "aapt2_tests",
     srcs: ["**/*_test.cpp"],
-    static_libs: ["libaapt2"],
+    static_libs: ["libaapt2", "libgmock"],
     defaults: ["aapt_defaults"],
 }
 

tools/aapt2/ResourceUtils.cpp

@@ -496,19 +496,17 @@ Maybe<int> ParseSdkVersion(const StringPiece& str) {
 
 std::unique_ptr<BinaryPrimitive> TryParseBool(const StringPiece& str) {
   if (Maybe<bool> maybe_result = ParseBool(str)) {
-    android::Res_value value = {};
+    const uint32_t data = maybe_result.value() ? 0xffffffffu : 0u;
-    value.dataType = android::Res_value::TYPE_INT_BOOLEAN;
+    return util::make_unique<BinaryPrimitive>(android::Res_value::TYPE_INT_BOOLEAN, data);
-
-    if (maybe_result.value()) {
-      value.data = 0xffffffffu;
-    } else {
-      value.data = 0;
-    }
-    return util::make_unique<BinaryPrimitive>(value);
   }
   return {};
 }
 
+std::unique_ptr<BinaryPrimitive> MakeBool(bool val) {
+  return util::make_unique<BinaryPrimitive>(android::Res_value::TYPE_INT_BOOLEAN,
+                                            val ? 0xffffffffu : 0u);
+}
+
 std::unique_ptr<BinaryPrimitive> TryParseInt(const StringPiece& str) {
   std::u16string str16 = util::Utf8ToUtf16(str);
   android::Res_value value;

tools/aapt2/ResourceUtils.h

@@ -147,6 +147,9 @@ std::unique_ptr<BinaryPrimitive> TryParseColor(const android::StringPiece& str);
  */
 std::unique_ptr<BinaryPrimitive> TryParseBool(const android::StringPiece& str);
 
+// Returns a boolean BinaryPrimitive.
+std::unique_ptr<BinaryPrimitive> MakeBool(bool val);
+
 /*
  * Returns a BinaryPrimitve object representing an integer if the string was
  * parsed as one.

tools/aapt2/ResourceValues.cpp

@@ -29,6 +29,11 @@
 
 namespace aapt {
 
+std::ostream& operator<<(std::ostream& out, const Value& value) {
+  value.Print(&out);
+  return out;
+}
+
 template <typename Derived>
 void BaseValue<Derived>::Accept(RawValueVisitor* visitor) {
   visitor->Visit(static_cast<Derived*>(this));
@@ -346,6 +351,15 @@ Attribute::Attribute(bool w, uint32_t t)
   weak_ = w;
 }
 
+std::ostream& operator<<(std::ostream& out, const Attribute::Symbol& s) {
+  if (s.symbol.name) {
+    out << s.symbol.name.value().entry;
+  } else {
+    out << "???";
+  }
+  return out << "=" << s.value;
+}
+
 template <typename T>
 constexpr T* add_pointer(T& val) {
   return &val;
@@ -361,31 +375,27 @@ bool Attribute::Equals(const Value* value) const {
     return false;
   }
 
-  if (type_mask != other->type_mask || min_int != other->min_int ||
+  if (type_mask != other->type_mask || min_int != other->min_int || max_int != other->max_int) {
-      max_int != other->max_int) {
     return false;
   }
 
   std::vector<const Symbol*> sorted_a;
   std::transform(symbols.begin(), symbols.end(), std::back_inserter(sorted_a),
                  add_pointer<const Symbol>);
-  std::sort(sorted_a.begin(), sorted_a.end(),
+  std::sort(sorted_a.begin(), sorted_a.end(), [](const Symbol* a, const Symbol* b) -> bool {
-            [](const Symbol* a, const Symbol* b) -> bool {
+    return a->symbol.name < b->symbol.name;
-              return a->symbol.name < b->symbol.name;
+  });
-            });
 
   std::vector<const Symbol*> sorted_b;
-  std::transform(other->symbols.begin(), other->symbols.end(),
+  std::transform(other->symbols.begin(), other->symbols.end(), std::back_inserter(sorted_b),
-                 std::back_inserter(sorted_b), add_pointer<const Symbol>);
+                 add_pointer<const Symbol>);
-  std::sort(sorted_b.begin(), sorted_b.end(),
+  std::sort(sorted_b.begin(), sorted_b.end(), [](const Symbol* a, const Symbol* b) -> bool {
-            [](const Symbol* a, const Symbol* b) -> bool {
+    return a->symbol.name < b->symbol.name;
-              return a->symbol.name < b->symbol.name;
+  });
-            });
 
   return std::equal(sorted_a.begin(), sorted_a.end(), sorted_b.begin(),
                     [](const Symbol* a, const Symbol* b) -> bool {
-                      return a->symbol.Equals(&b->symbol) &&
+                      return a->symbol.Equals(&b->symbol) && a->value == b->value;
-                             a->value == b->value;
                     });
 }
 
@@ -588,14 +598,50 @@ bool Attribute::Matches(const Item* item, DiagMessage* out_msg) const {
   return true;
 }
 
+std::ostream& operator<<(std::ostream& out, const Style::Entry& entry) {
+  if (entry.key.name) {
+    out << entry.key.name.value();
+  } else if (entry.key.id) {
+    out << entry.key.id.value();
+  } else {
+    out << "???";
+  }
+  out << " = " << entry.value;
+  return out;
+}
+
+template <typename T>
+std::vector<T*> ToPointerVec(std::vector<T>& src) {
+  std::vector<T*> dst;
+  dst.reserve(src.size());
+  for (T& in : src) {
+    dst.push_back(&in);
+  }
+  return dst;
+}
+
+template <typename T>
+std::vector<const T*> ToPointerVec(const std::vector<T>& src) {
+  std::vector<const T*> dst;
+  dst.reserve(src.size());
+  for (const T& in : src) {
+    dst.push_back(&in);
+  }
+  return dst;
+}
+
+static bool KeyNameComparator(const Style::Entry* a, const Style::Entry* b) {
+  return a->key.name < b->key.name;
+}
+
 bool Style::Equals(const Value* value) const {
   const Style* other = ValueCast<Style>(value);
   if (!other) {
     return false;
   }
+
   if (bool(parent) != bool(other->parent) ||
-      (parent && other->parent &&
+      (parent && other->parent && !parent.value().Equals(&other->parent.value()))) {
-       !parent.value().Equals(&other->parent.value()))) {
     return false;
   }
 
@@ -603,26 +649,15 @@ bool Style::Equals(const Value* value) const {
     return false;
   }
 
-  std::vector<const Entry*> sorted_a;
+  std::vector<const Entry*> sorted_a = ToPointerVec(entries);
-  std::transform(entries.begin(), entries.end(), std::back_inserter(sorted_a),
+  std::sort(sorted_a.begin(), sorted_a.end(), KeyNameComparator);
-                 add_pointer<const Entry>);
-  std::sort(sorted_a.begin(), sorted_a.end(),
-            [](const Entry* a, const Entry* b) -> bool {
-              return a->key.name < b->key.name;
-            });
 
-  std::vector<const Entry*> sorted_b;
+  std::vector<const Entry*> sorted_b = ToPointerVec(other->entries);
-  std::transform(other->entries.begin(), other->entries.end(),
+  std::sort(sorted_b.begin(), sorted_b.end(), KeyNameComparator);
-                 std::back_inserter(sorted_b), add_pointer<const Entry>);
-  std::sort(sorted_b.begin(), sorted_b.end(),
-            [](const Entry* a, const Entry* b) -> bool {
-              return a->key.name < b->key.name;
-            });
 
   return std::equal(sorted_a.begin(), sorted_a.end(), sorted_b.begin(),
                     [](const Entry* a, const Entry* b) -> bool {
-                      return a->key.Equals(&b->key) &&
+                      return a->key.Equals(&b->key) && a->value->Equals(b->value.get());
-                             a->value->Equals(b->value.get());
                     });
 }
 
@@ -633,8 +668,7 @@ Style* Style::Clone(StringPool* new_pool) const {
   style->comment_ = comment_;
   style->source_ = source_;
   for (auto& entry : entries) {
-    style->entries.push_back(
+    style->entries.push_back(Entry{entry.key, std::unique_ptr<Item>(entry.value->Clone(new_pool))});
-        Entry{entry.key, std::unique_ptr<Item>(entry.value->Clone(new_pool))});
   }
   return style;
 }
@@ -642,26 +676,73 @@ Style* Style::Clone(StringPool* new_pool) const {
 void Style::Print(std::ostream* out) const {
   *out << "(style) ";
   if (parent && parent.value().name) {
-    if (parent.value().private_reference) {
+    const Reference& parent_ref = parent.value();
+    if (parent_ref.private_reference) {
       *out << "*";
     }
-    *out << parent.value().name.value();
+    *out << parent_ref.name.value();
   }
   *out << " [" << util::Joiner(entries, ", ") << "]";
 }
 
-static ::std::ostream& operator<<(::std::ostream& out,
+Style::Entry CloneEntry(const Style::Entry& entry, StringPool* pool) {
-                                  const Style::Entry& value) {
+  Style::Entry cloned_entry{entry.key};
-  if (value.key.name) {
+  if (entry.value != nullptr) {
-    out << value.key.name.value();
+    cloned_entry.value.reset(entry.value->Clone(pool));
-  } else if (value.key.id) {
-    out << value.key.id.value();
-  } else {
-    out << "???";
   }
-  out << " = ";
+  return cloned_entry;
-  value.value->Print(&out);
+}
-  return out;
+
+void Style::MergeWith(Style* other, StringPool* pool) {
+  if (other->parent) {
+    parent = other->parent;
+  }
+
+  // We can't assume that the entries are sorted alphabetically since they're supposed to be
+  // sorted by Resource Id. Not all Resource Ids may be set though, so we can't sort and merge
+  // them keying off that.
+  //
+  // Instead, sort the entries of each Style by their name in a separate structure. Then merge
+  // those.
+
+  std::vector<Entry*> this_sorted = ToPointerVec(entries);
+  std::sort(this_sorted.begin(), this_sorted.end(), KeyNameComparator);
+
+  std::vector<Entry*> other_sorted = ToPointerVec(other->entries);
+  std::sort(other_sorted.begin(), other_sorted.end(), KeyNameComparator);
+
+  auto this_iter = this_sorted.begin();
+  const auto this_end = this_sorted.end();
+
+  auto other_iter = other_sorted.begin();
+  const auto other_end = other_sorted.end();
+
+  std::vector<Entry> merged_entries;
+  while (this_iter != this_end) {
+    if (other_iter != other_end) {
+      if ((*this_iter)->key.name < (*other_iter)->key.name) {
+        merged_entries.push_back(std::move(**this_iter));
+        ++this_iter;
+      } else {
+        // The other overrides.
+        merged_entries.push_back(CloneEntry(**other_iter, pool));
+        if ((*this_iter)->key.name == (*other_iter)->key.name) {
+          ++this_iter;
+        }
+        ++other_iter;
+      }
+    } else {
+      merged_entries.push_back(std::move(**this_iter));
+      ++this_iter;
+    }
+  }
+
+  while (other_iter != other_end) {
+    merged_entries.push_back(CloneEntry(**other_iter, pool));
+    ++other_iter;
+  }
+
+  entries = std::move(merged_entries);
 }
 
 bool Array::Equals(const Value* value) const {
@@ -758,11 +839,6 @@ void Plural::Print(std::ostream* out) const {
   }
 }
 
-static ::std::ostream& operator<<(::std::ostream& out,
-                                  const std::unique_ptr<Item>& item) {
-  return out << *item;
-}
-
 bool Styleable::Equals(const Value* value) const {
   const Styleable* other = ValueCast<Styleable>(value);
   if (!other) {
@@ -810,8 +886,7 @@ struct NameOnlyComparator {
 
 void Styleable::MergeWith(Styleable* other) {
   // Compare only names, because some References may already have their IDs
-  // assigned
+  // assigned (framework IDs that don't change).
-  // (framework IDs that don't change).
   std::set<Reference, NameOnlyComparator> references;
   references.insert(entries.begin(), entries.end());
   references.insert(other->entries.begin(), other->entries.end());

tools/aapt2/ResourceValues.h

@@ -40,7 +40,8 @@ struct RawValueVisitor;
 // type specific operations is to check the Value's type() and
 // cast it to the appropriate subclass. This isn't super clean,
 // but it is the simplest strategy.
-struct Value {
+class Value {
+ public:
   virtual ~Value() = default;
 
   // Whether this value is weak and can be overridden without warning or error. Default is false.
@@ -82,6 +83,8 @@ struct Value {
   // Human readable printout of this value.
   virtual void Print(std::ostream* out) const = 0;
 
+  friend std::ostream& operator<<(std::ostream& out, const Value& value);
+
  protected:
   Source source_;
   std::string comment_;
@@ -245,6 +248,8 @@ struct Attribute : public BaseValue<Attribute> {
   struct Symbol {
     Reference symbol;
     uint32_t value;
+
+    friend std::ostream& operator<<(std::ostream& out, const Symbol& symbol);
   };
 
   uint32_t type_mask;
@@ -266,6 +271,8 @@ struct Style : public BaseValue<Style> {
   struct Entry {
     Reference key;
     std::unique_ptr<Item> value;
+
+    friend std::ostream& operator<<(std::ostream& out, const Entry& entry);
   };
 
   Maybe<Reference> parent;
@@ -278,6 +285,10 @@ struct Style : public BaseValue<Style> {
   bool Equals(const Value* value) const override;
   Style* Clone(StringPool* new_pool) const override;
   void Print(std::ostream* out) const override;
+
+  // Merges `style` into this Style. All identical attributes of `style` take precedence, including
+  // the parent, if there is one.
+  void MergeWith(Style* style, StringPool* pool);
 };
 
 struct Array : public BaseValue<Array> {
@@ -307,20 +318,15 @@ struct Styleable : public BaseValue<Styleable> {
   void MergeWith(Styleable* styleable);
 };
 
-// Stream operator for printing Value objects.
+template <typename T>
-inline ::std::ostream& operator<<(::std::ostream& out, const Value& value) {
+typename std::enable_if<std::is_base_of<Value, T>::value, std::ostream&>::type operator<<(
-  value.Print(&out);
+    std::ostream& out, const std::unique_ptr<T>& value) {
-  return out;
+  if (value == nullptr) {
-}
+    out << "NULL";
-
-inline ::std::ostream& operator<<(::std::ostream& out,
-                                  const Attribute::Symbol& s) {
-  if (s.symbol.name) {
-    out << s.symbol.name.value().entry;
   } else {
-    out << "???";
+    value->Print(&out);
   }
-  return out << "=" << s.value;
+  return out;
 }
 
 }  // namespace aapt

tools/aapt2/ResourceValues_test.cpp

@@ -148,4 +148,36 @@ TEST(ResourceValuesTest, StyleClone) {
   EXPECT_TRUE(a->Equals(b.get()));
 }
 
+TEST(ResourceValuesTest, StyleMerges) {
+  StringPool pool_a;
+  StringPool pool_b;
+
+  std::unique_ptr<Style> a =
+      test::StyleBuilder()
+          .SetParent("android:style/Parent")
+          .AddItem("android:attr/a", util::make_unique<String>(pool_a.MakeRef("FooA")))
+          .AddItem("android:attr/b", util::make_unique<String>(pool_a.MakeRef("FooB")))
+          .Build();
+
+  std::unique_ptr<Style> b =
+      test::StyleBuilder()
+          .SetParent("android:style/OverlayParent")
+          .AddItem("android:attr/c", util::make_unique<String>(pool_b.MakeRef("OverlayFooC")))
+          .AddItem("android:attr/a", util::make_unique<String>(pool_b.MakeRef("OverlayFooA")))
+          .Build();
+
+  a->MergeWith(b.get(), &pool_a);
+
+  StringPool pool;
+  std::unique_ptr<Style> expected =
+      test::StyleBuilder()
+          .SetParent("android:style/OverlayParent")
+          .AddItem("android:attr/a", util::make_unique<String>(pool.MakeRef("OverlayFooA")))
+          .AddItem("android:attr/b", util::make_unique<String>(pool.MakeRef("FooB")))
+          .AddItem("android:attr/c", util::make_unique<String>(pool.MakeRef("OverlayFooC")))
+          .Build();
+
+  EXPECT_TRUE(a->Equals(expected.get()));
+}
+
 } // namespace aapt

tools/aapt2/link/TableMerger.cpp

@@ -179,32 +179,39 @@ static bool MergeEntry(IAaptContext* context, const Source& src,
   return true;
 }
 
-/**
+// Modified CollisionResolver which will merge Styleables and Styles. Used with overlays.
- * Modified CollisionResolver which will merge Styleables. Used with overlays.
+//
- *
+// Styleables are not actual resources, but they are treated as such during the
- * Styleables are not actual resources, but they are treated as such during the
+// compilation phase.
- * compilation phase. Styleables don't simply overlay each other, their
+//
- * definitions merge
+// Styleables and Styles don't simply overlay each other, their definitions merge
- * and accumulate. If both values are Styleables, we just merge them into the
+// and accumulate. If both values are Styleables/Styles, we just merge them into the
- * existing value.
+// existing value.
- */
+static ResourceTable::CollisionResult ResolveMergeCollision(Value* existing, Value* incoming,
-static ResourceTable::CollisionResult ResolveMergeCollision(Value* existing,
+                                                            StringPool* pool) {
-                                                            Value* incoming) {
   if (Styleable* existing_styleable = ValueCast<Styleable>(existing)) {
     if (Styleable* incoming_styleable = ValueCast<Styleable>(incoming)) {
       // Styleables get merged.
       existing_styleable->MergeWith(incoming_styleable);
       return ResourceTable::CollisionResult::kKeepOriginal;
     }
+  } else if (Style* existing_style = ValueCast<Style>(existing)) {
+    if (Style* incoming_style = ValueCast<Style>(incoming)) {
+      // Styles get merged.
+      existing_style->MergeWith(incoming_style, pool);
+      return ResourceTable::CollisionResult::kKeepOriginal;
+    }
   }
   // Delegate to the default handler.
   return ResourceTable::ResolveValueCollision(existing, incoming);
 }
 
-static ResourceTable::CollisionResult MergeConfigValue(
+static ResourceTable::CollisionResult MergeConfigValue(IAaptContext* context,
-    IAaptContext* context, const ResourceNameRef& res_name, const bool overlay,
+                                                       const ResourceNameRef& res_name,
-    ResourceConfigValue* dst_config_value,
+                                                       const bool overlay,
-    ResourceConfigValue* src_config_value) {
+                                                       ResourceConfigValue* dst_config_value,
+                                                       ResourceConfigValue* src_config_value,
+                                                       StringPool* pool) {
   using CollisionResult = ResourceTable::CollisionResult;
 
   Value* dst_value = dst_config_value->value.get();
@@ -212,10 +219,9 @@ static ResourceTable::CollisionResult MergeConfigValue(
 
   CollisionResult collision_result;
   if (overlay) {
-    collision_result = ResolveMergeCollision(dst_value, src_value);
+    collision_result = ResolveMergeCollision(dst_value, src_value, pool);
   } else {
-    collision_result =
+    collision_result = ResourceTable::ResolveValueCollision(dst_value, src_value);
-        ResourceTable::ResolveValueCollision(dst_value, src_value);
   }
 
   if (collision_result == CollisionResult::kConflict) {
@@ -224,10 +230,9 @@ static ResourceTable::CollisionResult MergeConfigValue(
     }
 
     // Error!
-    context->GetDiagnostics()->Error(
+    context->GetDiagnostics()->Error(DiagMessage(src_value->GetSource())
-        DiagMessage(src_value->GetSource())
+                                     << "resource '" << res_name << "' has a conflicting value for "
-        << "resource '" << res_name << "' has a conflicting value for "
+                                     << "configuration (" << src_config_value->config << ")");
-        << "configuration (" << src_config_value->config << ")");
     context->GetDiagnostics()->Note(DiagMessage(dst_value->GetSource())
                                     << "originally defined here");
     return CollisionResult::kConflict;
@@ -287,7 +292,7 @@ bool TableMerger::DoMerge(const Source& src, ResourceTable* src_table,
         if (dst_config_value) {
           CollisionResult collision_result =
               MergeConfigValue(context_, res_name, overlay, dst_config_value,
-                               src_config_value.get());
+                               src_config_value.get(), &master_table_->string_pool);
           if (collision_result == CollisionResult::kConflict) {
             error = true;
             continue;
@@ -295,25 +300,22 @@ bool TableMerger::DoMerge(const Source& src, ResourceTable* src_table,
             continue;
           }
         } else {
-          dst_config_value = dst_entry->FindOrCreateValue(
+          dst_config_value =
-              src_config_value->config, src_config_value->product);
+              dst_entry->FindOrCreateValue(src_config_value->config, src_config_value->product);
         }
 
         // Continue if we're taking the new resource.
 
-        if (FileReference* f =
+        if (FileReference* f = ValueCast<FileReference>(src_config_value->value.get())) {
-                ValueCast<FileReference>(src_config_value->value.get())) {
           std::unique_ptr<FileReference> new_file_ref;
           if (mangle_package) {
             new_file_ref = CloneAndMangleFile(src_package->name, *f);
           } else {
-            new_file_ref = std::unique_ptr<FileReference>(
+            new_file_ref = std::unique_ptr<FileReference>(f->Clone(&master_table_->string_pool));
-                f->Clone(&master_table_->string_pool));
           }
 
           if (callback) {
-            if (!callback(res_name, src_config_value->config,
+            if (!callback(res_name, src_config_value->config, new_file_ref.get(), f)) {
-                          new_file_ref.get(), f)) {
               error = true;
               continue;
             }
@@ -337,18 +339,15 @@ std::unique_ptr<FileReference> TableMerger::CloneAndMangleFile(
     std::string mangled_entry = NameMangler::MangleEntry(package, entry.to_string());
     std::string newPath = prefix.to_string() + mangled_entry + suffix.to_string();
     std::unique_ptr<FileReference> new_file_ref =
-        util::make_unique<FileReference>(
+        util::make_unique<FileReference>(master_table_->string_pool.MakeRef(newPath));
-            master_table_->string_pool.MakeRef(newPath));
     new_file_ref->SetComment(file_ref.GetComment());
     new_file_ref->SetSource(file_ref.GetSource());
     return new_file_ref;
   }
-  return std::unique_ptr<FileReference>(
+  return std::unique_ptr<FileReference>(file_ref.Clone(&master_table_->string_pool));
-      file_ref.Clone(&master_table_->string_pool));
 }
 
-bool TableMerger::MergeFileImpl(const ResourceFile& file_desc, io::IFile* file,
+bool TableMerger::MergeFileImpl(const ResourceFile& file_desc, io::IFile* file, bool overlay) {
-                                bool overlay) {
   ResourceTable table;
   std::string path = ResourceUtils::BuildResourceFileName(file_desc);
   std::unique_ptr<FileReference> file_ref =

tools/aapt2/link/TableMerger_test.cpp

@@ -20,6 +20,14 @@
 #include "io/FileSystem.h"
 #include "test/Test.h"
 
+using ::aapt::test::ValueEq;
+using ::testing::Contains;
+using ::testing::NotNull;
+using ::testing::UnorderedElementsAreArray;
+using ::testing::Pointee;
+using ::testing::Field;
+using ::testing::Eq;
+
 namespace aapt {
 
 struct TableMergerTest : public ::testing::Test {
@@ -62,26 +70,20 @@ TEST_F(TableMergerTest, SimpleMerge) {
   io::FileCollection collection;
 
   ASSERT_TRUE(merger.Merge({}, table_a.get()));
-  ASSERT_TRUE(
+  ASSERT_TRUE(merger.MergeAndMangle({}, "com.app.b", table_b.get(), &collection));
-      merger.MergeAndMangle({}, "com.app.b", table_b.get(), &collection));
 
   EXPECT_TRUE(merger.merged_packages().count("com.app.b") != 0);
 
   // Entries from com.app.a should not be mangled.
-  AAPT_EXPECT_TRUE(
+  EXPECT_TRUE(final_table.FindResource(test::ParseNameOrDie("com.app.a:id/foo")));
-      final_table.FindResource(test::ParseNameOrDie("com.app.a:id/foo")));
+  EXPECT_TRUE(final_table.FindResource(test::ParseNameOrDie("com.app.a:id/bar")));
-  AAPT_EXPECT_TRUE(
+  EXPECT_TRUE(final_table.FindResource(test::ParseNameOrDie("com.app.a:styleable/view")));
-      final_table.FindResource(test::ParseNameOrDie("com.app.a:id/bar")));
-  AAPT_EXPECT_TRUE(final_table.FindResource(
-      test::ParseNameOrDie("com.app.a:styleable/view")));
 
   // The unmangled name should not be present.
-  AAPT_EXPECT_FALSE(
+  EXPECT_FALSE(final_table.FindResource(test::ParseNameOrDie("com.app.b:id/foo")));
-      final_table.FindResource(test::ParseNameOrDie("com.app.b:id/foo")));
 
   // Look for the mangled name.
-  AAPT_EXPECT_TRUE(final_table.FindResource(
+  EXPECT_TRUE(final_table.FindResource(test::ParseNameOrDie("com.app.a:id/com.app.b$foo")));
-      test::ParseNameOrDie("com.app.a:id/com.app.b$foo")));
 }
 
 TEST_F(TableMergerTest, MergeFile) {
@@ -100,7 +102,7 @@ TEST_F(TableMergerTest, MergeFile) {
 
   FileReference* file = test::GetValueForConfig<FileReference>(
       &final_table, "com.app.a:layout/main", test::ParseConfigOrDie("hdpi-v4"));
-  ASSERT_NE(nullptr, file);
+  ASSERT_THAT(file, NotNull());
   EXPECT_EQ(std::string("res/layout-hdpi-v4/main.xml"), *file->path);
 }
 
@@ -137,17 +139,14 @@ TEST_F(TableMergerTest, MergeFileReferences) {
   collection.InsertFile("res/xml/file.xml");
 
   ASSERT_TRUE(merger.Merge({}, table_a.get()));
-  ASSERT_TRUE(
+  ASSERT_TRUE(merger.MergeAndMangle({}, "com.app.b", table_b.get(), &collection));
-      merger.MergeAndMangle({}, "com.app.b", table_b.get(), &collection));
 
-  FileReference* f =
+  FileReference* f = test::GetValue<FileReference>(&final_table, "com.app.a:xml/file");
-      test::GetValue<FileReference>(&final_table, "com.app.a:xml/file");
+  ASSERT_THAT(f, NotNull());
-  ASSERT_NE(f, nullptr);
   EXPECT_EQ(std::string("res/xml/file.xml"), *f->path);
 
-  f = test::GetValue<FileReference>(&final_table,
+  f = test::GetValue<FileReference>(&final_table, "com.app.a:xml/com.app.b$file");
-                                    "com.app.a:xml/com.app.b$file");
+  ASSERT_THAT(f, NotNull());
-  ASSERT_NE(f, nullptr);
   EXPECT_EQ(std::string("res/xml/com.app.b$file.xml"), *f->path);
 }
 
@@ -171,10 +170,9 @@ TEST_F(TableMergerTest, OverrideResourceWithOverlay) {
   ASSERT_TRUE(merger.Merge({}, base.get()));
   ASSERT_TRUE(merger.MergeOverlay({}, overlay.get()));
 
-  BinaryPrimitive* foo =
+  BinaryPrimitive* foo = test::GetValue<BinaryPrimitive>(&final_table, "com.app.a:bool/foo");
-      test::GetValue<BinaryPrimitive>(&final_table, "com.app.a:bool/foo");
+  ASSERT_THAT(foo,
-  ASSERT_NE(nullptr, foo);
+              Pointee(Field(&BinaryPrimitive::value, Field(&android::Res_value::data, Eq(0u)))));
-  EXPECT_EQ(0x0u, foo->value.data);
 }
 
 TEST_F(TableMergerTest, OverrideSameResourceIdsWithOverlay) {
@@ -301,7 +299,7 @@ TEST_F(TableMergerTest, FailToMergeNewResourceWithoutAutoAddOverlay) {
   ASSERT_FALSE(merger.MergeOverlay({}, table_b.get()));
 }
 
-TEST_F(TableMergerTest, OverlaidStyleablesShouldBeMerged) {
+TEST_F(TableMergerTest, OverlaidStyleablesAndStylesShouldBeMerged) {
   std::unique_ptr<ResourceTable> table_a =
       test::ResourceTableBuilder()
           .SetPackageId("com.app.a", 0x7f)
@@ -310,15 +308,27 @@ TEST_F(TableMergerTest, OverlaidStyleablesShouldBeMerged) {
                         .AddItem("com.app.a:attr/bar")
                         .AddItem("com.app.a:attr/foo", ResourceId(0x01010000))
                         .Build())
+          .AddValue("com.app.a:style/Theme",
+                    test::StyleBuilder()
+                        .SetParent("com.app.a:style/Parent")
+                        .AddItem("com.app.a:attr/bar", util::make_unique<Id>())
+                        .AddItem("com.app.a:attr/foo", ResourceUtils::MakeBool(false))
+                        .Build())
           .Build();
 
   std::unique_ptr<ResourceTable> table_b =
       test::ResourceTableBuilder()
           .SetPackageId("com.app.a", 0x7f)
-          .AddValue("com.app.a:styleable/Foo",
+          .AddValue("com.app.a:styleable/Foo", test::StyleableBuilder()
-                    test::StyleableBuilder()
+                                                   .AddItem("com.app.a:attr/bat")
-                        .AddItem("com.app.a:attr/bat")
+                                                   .AddItem("com.app.a:attr/foo")
-                        .AddItem("com.app.a:attr/foo")
+                                                   .Build())
+          .AddValue("com.app.a:style/Theme",
+                    test::StyleBuilder()
+                        .SetParent("com.app.a:style/OverlayParent")
+                        .AddItem("com.app.a:attr/bat", util::make_unique<Id>())
+                        .AddItem("com.app.a:attr/foo", ResourceId(0x01010000),
+                                 ResourceUtils::MakeBool(true))
                         .Build())
           .Build();
 
@@ -330,18 +340,29 @@ TEST_F(TableMergerTest, OverlaidStyleablesShouldBeMerged) {
   ASSERT_TRUE(merger.Merge({}, table_a.get()));
   ASSERT_TRUE(merger.MergeOverlay({}, table_b.get()));
 
-  Styleable* styleable =
+  Styleable* styleable = test::GetValue<Styleable>(&final_table, "com.app.a:styleable/Foo");
-      test::GetValue<Styleable>(&final_table, "com.app.a:styleable/Foo");
+  ASSERT_THAT(styleable, NotNull());
-  ASSERT_NE(nullptr, styleable);
 
   std::vector<Reference> expected_refs = {
       Reference(test::ParseNameOrDie("com.app.a:attr/bar")),
       Reference(test::ParseNameOrDie("com.app.a:attr/bat")),
-      Reference(test::ParseNameOrDie("com.app.a:attr/foo"),
+      Reference(test::ParseNameOrDie("com.app.a:attr/foo"), ResourceId(0x01010000)),
-                ResourceId(0x01010000)),
   };
+  EXPECT_THAT(styleable->entries, UnorderedElementsAreArray(expected_refs));
 
-  EXPECT_EQ(expected_refs, styleable->entries);
+  Style* style = test::GetValue<Style>(&final_table, "com.app.a:style/Theme");
+  ASSERT_THAT(style, NotNull());
+
+  std::vector<Reference> extracted_refs;
+  for (const auto& entry : style->entries) {
+    extracted_refs.push_back(entry.key);
+  }
+  EXPECT_THAT(extracted_refs, UnorderedElementsAreArray(expected_refs));
+
+  const auto expected = ResourceUtils::MakeBool(true);
+  EXPECT_THAT(style->entries, Contains(Field(&Style::Entry::value, Pointee(ValueEq(*expected)))));
+  EXPECT_THAT(style->parent,
+              Eq(make_value(Reference(test::ParseNameOrDie("com.app.a:style/OverlayParent")))));
 }
 
 }  // namespace aapt

tools/aapt2/test/Common.h

@@ -22,12 +22,14 @@
 #include "android-base/logging.h"
 #include "android-base/macros.h"
 #include "androidfw/StringPiece.h"
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 #include "ConfigDescription.h"
 #include "Debug.h"
 #include "ResourceTable.h"
 #include "ResourceUtils.h"
+#include "ResourceValues.h"
 #include "ValueVisitor.h"
 #include "io/File.h"
 #include "process/IResourceTableConsumer.h"
@@ -51,13 +53,11 @@ struct DummyDiagnosticsImpl : public IDiagnostics {
         return;
 
       case Level::Warn:
-        std::cerr << actual_msg.source << ": warn: " << actual_msg.message
+        std::cerr << actual_msg.source << ": warn: " << actual_msg.message << "." << std::endl;
-                  << "." << std::endl;
         break;
 
       case Level::Error:
-        std::cerr << actual_msg.source << ": error: " << actual_msg.message
+        std::cerr << actual_msg.source << ": error: " << actual_msg.message << "." << std::endl;
-                  << "." << std::endl;
         break;
     }
   }
@@ -84,11 +84,9 @@ template <typename T>
 T* GetValueForConfigAndProduct(ResourceTable* table, const android::StringPiece& res_name,
                                const ConfigDescription& config,
                                const android::StringPiece& product) {
-  Maybe<ResourceTable::SearchResult> result =
+  Maybe<ResourceTable::SearchResult> result = table->FindResource(ParseNameOrDie(res_name));
-      table->FindResource(ParseNameOrDie(res_name));
   if (result) {
-    ResourceConfigValue* config_value =
+    ResourceConfigValue* config_value = result.value().entry->FindValue(config, product);
-        result.value().entry->FindValue(config, product);
     if (config_value) {
       return ValueCast<T>(config_value->value.get());
     }
@@ -111,9 +109,13 @@ class TestFile : public io::IFile {
  public:
   explicit TestFile(const android::StringPiece& path) : source_(path) {}
 
-  std::unique_ptr<io::IData> OpenAsData() override { return {}; }
+  std::unique_ptr<io::IData> OpenAsData() override {
+    return {};
+  }
 
-  const Source& GetSource() const override { return source_; }
+  const Source& GetSource() const override {
+    return source_;
+  }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(TestFile);
@@ -121,6 +123,47 @@ class TestFile : public io::IFile {
   Source source_;
 };
 
+}  // namespace test
+
+// Workaround gtest bug (https://github.com/google/googletest/issues/443)
+// that does not select base class operator<< for derived class T.
+template <typename T>
+typename std::enable_if<std::is_base_of<Value, T>::value, std::ostream&>::type operator<<(
+    std::ostream& out, const T& value) {
+  value.Print(&out);
+  return out;
+}
+
+template std::ostream& operator<<<Item>(std::ostream&, const Item&);
+template std::ostream& operator<<<Reference>(std::ostream&, const Reference&);
+template std::ostream& operator<<<Id>(std::ostream&, const Id&);
+template std::ostream& operator<<<RawString>(std::ostream&, const RawString&);
+template std::ostream& operator<<<String>(std::ostream&, const String&);
+template std::ostream& operator<<<StyledString>(std::ostream&, const StyledString&);
+template std::ostream& operator<<<FileReference>(std::ostream&, const FileReference&);
+template std::ostream& operator<<<BinaryPrimitive>(std::ostream&, const BinaryPrimitive&);
+template std::ostream& operator<<<Attribute>(std::ostream&, const Attribute&);
+template std::ostream& operator<<<Style>(std::ostream&, const Style&);
+template std::ostream& operator<<<Array>(std::ostream&, const Array&);
+template std::ostream& operator<<<Plural>(std::ostream&, const Plural&);
+
+// Add a print method to Maybe.
+template <typename T>
+void PrintTo(const Maybe<T>& value, std::ostream* out) {
+  if (value) {
+    *out << ::testing::PrintToString(value.value());
+  } else {
+    *out << "Nothing";
+  }
+}
+
+namespace test {
+
+MATCHER_P(ValueEq, a,
+          std::string(negation ? "isn't" : "is") + " equal to " + ::testing::PrintToString(a)) {
+  return arg.Equals(&a);
+}
+
 }  // namespace test
 }  // namespace aapt
 

tools/aapt2/test/Test.h

@@ -17,6 +17,7 @@
 #ifndef AAPT_TEST_TEST_H
 #define AAPT_TEST_TEST_H
 
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
 #include "test/Builders.h"