AAPT2: Allow merging of Style attributes from overlays

Previously style overlays would completely override an existing style.
To be compatible with AAPT, styles now merge with the overlay, allowing
the overlay's attributes and parent to take precedence.

Bug: 38355988
Test: make aapt2_tests
Change-Id: Id25c7240050a43e6a4a177c6e3d51e048d0cceb5

10 files changed, 346 insertions, 172 deletions

diff --git a/tools/aapt2/Android.bp b/tools/aapt2/Android.bp
index bf189492b082..b2f1f62647e6 100644
--- a/tools/aapt2/Android.bp
+++ b/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"],
 }
 
diff --git a/tools/aapt2/ResourceUtils.cpp b/tools/aapt2/ResourceUtils.cpp
index 1bb7d9beee45..818c8cec30f9 100644
--- a/tools/aapt2/ResourceUtils.cpp
+++ b/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 = {};
-    value.dataType = android::Res_value::TYPE_INT_BOOLEAN;
-
-    if (maybe_result.value()) {
-      value.data = 0xffffffffu;
-    } else {
-      value.data = 0;
-    }
-    return util::make_unique<BinaryPrimitive>(value);
+    const uint32_t data = maybe_result.value() ? 0xffffffffu : 0u;
+    return util::make_unique<BinaryPrimitive>(android::Res_value::TYPE_INT_BOOLEAN, data);
   }
   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;
diff --git a/tools/aapt2/ResourceUtils.h b/tools/aapt2/ResourceUtils.h
index 48922b72fefa..da0fc8ee314a 100644
--- a/tools/aapt2/ResourceUtils.h
+++ b/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.
diff --git a/tools/aapt2/ResourceValues.cpp b/tools/aapt2/ResourceValues.cpp
index 34bd2b4361ca..abfdec48df67 100644
--- a/tools/aapt2/ResourceValues.cpp
+++ b/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 ||
-      max_int != other->max_int) {
+  if (type_mask != other->type_mask || min_int != other->min_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(),
-            [](const Symbol* a, const Symbol* b) -> bool {
-              return a->symbol.name < b->symbol.name;
-            });
+  std::sort(sorted_a.begin(), sorted_a.end(), [](const Symbol* a, const Symbol* b) -> bool {
+    return a->symbol.name < b->symbol.name;
+  });
 
   std::vector<const Symbol*> sorted_b;
-  std::transform(other->symbols.begin(), other->symbols.end(),
-                 std::back_inserter(sorted_b), add_pointer<const Symbol>);
-  std::sort(sorted_b.begin(), sorted_b.end(),
-            [](const Symbol* a, const Symbol* b) -> bool {
-              return a->symbol.name < b->symbol.name;
-            });
+  std::transform(other->symbols.begin(), other->symbols.end(), std::back_inserter(sorted_b),
+                 add_pointer<const Symbol>);
+  std::sort(sorted_b.begin(), sorted_b.end(), [](const Symbol* a, const Symbol* b) -> bool {
+    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) &&
-                             a->value == b->value;
+                      return a->symbol.Equals(&b->symbol) && 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.value().Equals(&other->parent.value()))) {
+      (parent && other->parent && !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::transform(entries.begin(), entries.end(), std::back_inserter(sorted_a),
-                 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_a = ToPointerVec(entries);
+  std::sort(sorted_a.begin(), sorted_a.end(), KeyNameComparator);
 
-  std::vector<const Entry*> sorted_b;
-  std::transform(other->entries.begin(), other->entries.end(),
-                 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;
-            });
+  std::vector<const Entry*> sorted_b = ToPointerVec(other->entries);
+  std::sort(sorted_b.begin(), sorted_b.end(), KeyNameComparator);
 
   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) &&
-                             a->value->Equals(b->value.get());
+                      return a->key.Equals(&b->key) && 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(
-        Entry{entry.key, std::unique_ptr<Item>(entry.value->Clone(new_pool))});
+    style->entries.push_back(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,
-                                  const Style::Entry& value) {
-  if (value.key.name) {
-    out << value.key.name.value();
-  } else if (value.key.id) {
-    out << value.key.id.value();
-  } else {
-    out << "???";
+Style::Entry CloneEntry(const Style::Entry& entry, StringPool* pool) {
+  Style::Entry cloned_entry{entry.key};
+  if (entry.value != nullptr) {
+    cloned_entry.value.reset(entry.value->Clone(pool));
   }
-  out << " = ";
-  value.value->Print(&out);
-  return out;
+  return cloned_entry;
+}
+
+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
-  // (framework IDs that don't change).
+  // assigned (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());
diff --git a/tools/aapt2/ResourceValues.h b/tools/aapt2/ResourceValues.h
index 06f949f9555c..ac5795fb9774 100644
--- a/tools/aapt2/ResourceValues.h
+++ b/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.
-inline ::std::ostream& operator<<(::std::ostream& out, const Value& value) {
-  value.Print(&out);
-  return out;
-}
-
-inline ::std::ostream& operator<<(::std::ostream& out,
-                                  const Attribute::Symbol& s) {
-  if (s.symbol.name) {
-    out << s.symbol.name.value().entry;
+template <typename T>
+typename std::enable_if<std::is_base_of<Value, T>::value, std::ostream&>::type operator<<(
+    std::ostream& out, const std::unique_ptr<T>& value) {
+  if (value == nullptr) {
+    out << "NULL";
   } else {
-    out << "???";
+    value->Print(&out);
   }
-  return out << "=" << s.value;
+  return out;
 }
 
 }  // namespace aapt
diff --git a/tools/aapt2/ResourceValues_test.cpp b/tools/aapt2/ResourceValues_test.cpp
index 692258003471..e154d93b6f85 100644
--- a/tools/aapt2/ResourceValues_test.cpp
+++ b/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
diff --git a/tools/aapt2/link/TableMerger.cpp b/tools/aapt2/link/TableMerger.cpp
index 931109125263..14c23efac291 100644
--- a/tools/aapt2/link/TableMerger.cpp
+++ b/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. Used with overlays.
- *
- * Styleables are not actual resources, but they are treated as such during the
- * compilation phase. Styleables don't simply overlay each other, their
- * definitions merge
- * and accumulate. If both values are Styleables, we just merge them into the
- * existing value.
- */
-static ResourceTable::CollisionResult ResolveMergeCollision(Value* existing,
-                                                            Value* incoming) {
+// Modified CollisionResolver which will merge Styleables and Styles. Used with overlays.
+//
+// Styleables are not actual resources, but they are treated as such during the
+// compilation phase.
+//
+// Styleables and Styles don't simply overlay each other, their definitions merge
+// and accumulate. If both values are Styleables/Styles, we just merge them into the
+// existing value.
+static ResourceTable::CollisionResult ResolveMergeCollision(Value* existing, Value* incoming,
+                                                            StringPool* pool) {
   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(
-    IAaptContext* context, const ResourceNameRef& res_name, const bool overlay,
-    ResourceConfigValue* dst_config_value,
-    ResourceConfigValue* src_config_value) {
+static ResourceTable::CollisionResult MergeConfigValue(IAaptContext* context,
+                                                       const ResourceNameRef& res_name,
+                                                       const bool overlay,
+                                                       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 =
-        ResourceTable::ResolveValueCollision(dst_value, src_value);
+    collision_result = ResourceTable::ResolveValueCollision(dst_value, src_value);
   }
 
   if (collision_result == CollisionResult::kConflict) {
@@ -224,10 +230,9 @@ static ResourceTable::CollisionResult MergeConfigValue(
     }
 
     // Error!
-    context->GetDiagnostics()->Error(
-        DiagMessage(src_value->GetSource())
-        << "resource '" << res_name << "' has a conflicting value for "
-        << "configuration (" << src_config_value->config << ")");
+    context->GetDiagnostics()->Error(DiagMessage(src_value->GetSource())
+                                     << "resource '" << res_name << "' has a conflicting value for "
+                                     << "configuration (" << src_config_value->config << ")");
     context->GetDiagnostics()->Note(DiagMessage(dst_value->GetSource())
                                     << "originally defined here");
     return CollisionResult::kConflict;
@@ -243,8 +248,7 @@ bool TableMerger::DoMerge(const Source& src, ResourceTable* src_table,
   bool error = false;
 
   for (auto& src_type : src_package->types) {
-    ResourceTableType* dst_type =
-        master_package_->FindOrCreateType(src_type->type);
+    ResourceTableType* dst_type = master_package_->FindOrCreateType(src_type->type);
     if (!MergeType(context_, src, dst_type, src_type.get())) {
       error = true;
       continue;
@@ -253,8 +257,7 @@ bool TableMerger::DoMerge(const Source& src, ResourceTable* src_table,
     for (auto& src_entry : src_type->entries) {
       std::string entry_name = src_entry->name;
       if (mangle_package) {
-        entry_name =
-            NameMangler::MangleEntry(src_package->name, src_entry->name);
+        entry_name = NameMangler::MangleEntry(src_package->name, src_entry->name);
       }
 
       ResourceEntry* dst_entry;
@@ -264,16 +267,14 @@ bool TableMerger::DoMerge(const Source& src, ResourceTable* src_table,
         dst_entry = dst_type->FindEntry(entry_name);
       }
 
-      const ResourceNameRef res_name(src_package->name, src_type->type,
-                                     src_entry->name);
+      const ResourceNameRef res_name(src_package->name, src_type->type, src_entry->name);
 
       if (!dst_entry) {
-        context_->GetDiagnostics()->Error(
-            DiagMessage(src) << "resource " << res_name
-                             << " does not override an existing resource");
-        context_->GetDiagnostics()->Note(
-            DiagMessage(src) << "define an <add-resource> tag or use "
-                             << "--auto-add-overlay");
+        context_->GetDiagnostics()->Error(DiagMessage(src)
+                                          << "resource " << res_name
+                                          << " does not override an existing resource");
+        context_->GetDiagnostics()->Note(DiagMessage(src) << "define an <add-resource> tag or use "
+                                                          << "--auto-add-overlay");
         error = true;
         continue;
       }
@@ -291,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;
@@ -299,25 +300,22 @@ bool TableMerger::DoMerge(const Source& src, ResourceTable* src_table,
             continue;
           }
         } else {
-          dst_config_value = dst_entry->FindOrCreateValue(
-              src_config_value->config, src_config_value->product);
+          dst_config_value =
+              dst_entry->FindOrCreateValue(src_config_value->config, src_config_value->product);
         }
 
         // Continue if we're taking the new resource.
 
-        if (FileReference* f =
-                ValueCast<FileReference>(src_config_value->value.get())) {
+        if (FileReference* f = 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>(
-                f->Clone(&master_table_->string_pool));
+            new_file_ref = std::unique_ptr<FileReference>(f->Clone(&master_table_->string_pool));
           }
 
           if (callback) {
-            if (!callback(res_name, src_config_value->config,
-                          new_file_ref.get(), f)) {
+            if (!callback(res_name, src_config_value->config, new_file_ref.get(), f)) {
               error = true;
               continue;
             }
@@ -341,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>(
-            master_table_->string_pool.MakeRef(newPath));
+        util::make_unique<FileReference>(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>(
-      file_ref.Clone(&master_table_->string_pool));
+  return std::unique_ptr<FileReference>(file_ref.Clone(&master_table_->string_pool));
 }
 
-bool TableMerger::MergeFileImpl(const ResourceFile& file_desc, io::IFile* file,
-                                bool overlay) {
+bool TableMerger::MergeFileImpl(const ResourceFile& file_desc, io::IFile* file, bool overlay) {
   ResourceTable table;
   std::string path = ResourceUtils::BuildResourceFileName(file_desc);
   std::unique_ptr<FileReference> file_ref =
diff --git a/tools/aapt2/link/TableMerger_test.cpp b/tools/aapt2/link/TableMerger_test.cpp
index 742f5a7a570a..3b791b5cf92b 100644
--- a/tools/aapt2/link/TableMerger_test.cpp
+++ b/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(
-      merger.MergeAndMangle({}, "com.app.b", table_b.get(), &collection));
+  ASSERT_TRUE(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(
-      final_table.FindResource(test::ParseNameOrDie("com.app.a:id/foo")));
-  AAPT_EXPECT_TRUE(
-      final_table.FindResource(test::ParseNameOrDie("com.app.a:id/bar")));
-  AAPT_EXPECT_TRUE(final_table.FindResource(
-      test::ParseNameOrDie("com.app.a:styleable/view")));
+  EXPECT_TRUE(final_table.FindResource(test::ParseNameOrDie("com.app.a:id/foo")));
+  EXPECT_TRUE(final_table.FindResource(test::ParseNameOrDie("com.app.a:id/bar")));
+  EXPECT_TRUE(final_table.FindResource(test::ParseNameOrDie("com.app.a:styleable/view")));
 
   // The unmangled name should not be present.
-  AAPT_EXPECT_FALSE(
-      final_table.FindResource(test::ParseNameOrDie("com.app.b:id/foo")));
+  EXPECT_FALSE(final_table.FindResource(test::ParseNameOrDie("com.app.b:id/foo")));
 
   // Look for the mangled name.
-  AAPT_EXPECT_TRUE(final_table.FindResource(
-      test::ParseNameOrDie("com.app.a:id/com.app.b$foo")));
+  EXPECT_TRUE(final_table.FindResource(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(
-      merger.MergeAndMangle({}, "com.app.b", table_b.get(), &collection));
+  ASSERT_TRUE(merger.MergeAndMangle({}, "com.app.b", table_b.get(), &collection));
 
-  FileReference* f =
-      test::GetValue<FileReference>(&final_table, "com.app.a:xml/file");
-  ASSERT_NE(f, nullptr);
+  FileReference* f = test::GetValue<FileReference>(&final_table, "com.app.a:xml/file");
+  ASSERT_THAT(f, NotNull());
   EXPECT_EQ(std::string("res/xml/file.xml"), *f->path);
 
-  f = test::GetValue<FileReference>(&final_table,
-                                    "com.app.a:xml/com.app.b$file");
-  ASSERT_NE(f, nullptr);
+  f = test::GetValue<FileReference>(&final_table, "com.app.a:xml/com.app.b$file");
+  ASSERT_THAT(f, NotNull());
   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 =
-      test::GetValue<BinaryPrimitive>(&final_table, "com.app.a:bool/foo");
-  ASSERT_NE(nullptr, foo);
-  EXPECT_EQ(0x0u, foo->value.data);
+  BinaryPrimitive* foo = test::GetValue<BinaryPrimitive>(&final_table, "com.app.a:bool/foo");
+  ASSERT_THAT(foo,
+              Pointee(Field(&BinaryPrimitive::value, Field(&android::Res_value::data, Eq(0u)))));
 }
 
 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",
-                    test::StyleableBuilder()
-                        .AddItem("com.app.a:attr/bat")
-                        .AddItem("com.app.a:attr/foo")
+          .AddValue("com.app.a:styleable/Foo", test::StyleableBuilder()
+                                                   .AddItem("com.app.a:attr/bat")
+                                                   .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 =
-      test::GetValue<Styleable>(&final_table, "com.app.a:styleable/Foo");
-  ASSERT_NE(nullptr, styleable);
+  Styleable* styleable = test::GetValue<Styleable>(&final_table, "com.app.a:styleable/Foo");
+  ASSERT_THAT(styleable, NotNull());
 
   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"),
-                ResourceId(0x01010000)),
+      Reference(test::ParseNameOrDie("com.app.a:attr/foo"), ResourceId(0x01010000)),
   };
+  EXPECT_THAT(styleable->entries, UnorderedElementsAreArray(expected_refs));
+
+  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));
 
-  EXPECT_EQ(expected_refs, styleable->entries);
+  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
diff --git a/tools/aapt2/test/Common.h b/tools/aapt2/test/Common.h
index 248921f3afea..a937de8869a6 100644
--- a/tools/aapt2/test/Common.h
+++ b/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::endl;
+        std::cerr << actual_msg.source << ": warn: " << actual_msg.message << "." << std::endl;
         break;
 
       case Level::Error:
-        std::cerr << actual_msg.source << ": error: " << actual_msg.message
-                  << "." << std::endl;
+        std::cerr << actual_msg.source << ": error: " << actual_msg.message << "." << 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 =
-      table->FindResource(ParseNameOrDie(res_name));
+  Maybe<ResourceTable::SearchResult> result = table->FindResource(ParseNameOrDie(res_name));
   if (result) {
-    ResourceConfigValue* config_value =
-        result.value().entry->FindValue(config, product);
+    ResourceConfigValue* config_value = 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);
@@ -122,6 +124,47 @@ class TestFile : public io::IFile {
 };
 
 }  // 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
 
 #endif /* AAPT_TEST_COMMON_H */
diff --git a/tools/aapt2/test/Test.h b/tools/aapt2/test/Test.h
index ec07432fa51e..a24c01cd4137 100644
--- a/tools/aapt2/test/Test.h
+++ b/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"