Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 515e9b3b authored by TreeHugger Robot's avatar TreeHugger Robot Committed by Android (Google) Code Review
Browse files

Merge "DO NOT MERGE: Do not allow overlaying of attributes with conflicting formats" into qt-dev

parents 23e88129 b435101e

tools/aapt2/ResourceTable.cpp

+8 −6
Original line number Diff line number Diff line
@@ -267,7 +267,8 @@ bool ResourceEntry::HasDefaultValue() const { 
// A DECL will override a USE without error. Two DECLs must match in their format for there to be

// no error.

ResourceTable::CollisionResult ResourceTable::ResolveValueCollision(Value* existing,

                                                                    Value* incoming) {

                                                                    Value* incoming,

                                                                    bool overlay) {

  Attribute* existing_attr = ValueCast<Attribute>(existing);

  Attribute* incoming_attr = ValueCast<Attribute>(incoming);

  if (!incoming_attr) {
@@ -281,7 +282,7 @@ ResourceTable::CollisionResult ResourceTable::ResolveValueCollision(Value* exist 
    }

    // The existing and incoming values are strong, this is an error

    // if the values are not both attributes.

    return CollisionResult::kConflict;

    return overlay ? CollisionResult::kTakeNew : CollisionResult::kConflict;

  }



  if (!existing_attr) {
@@ -292,7 +293,7 @@ ResourceTable::CollisionResult ResourceTable::ResolveValueCollision(Value* exist 
    }

    // The existing value is not an attribute and it is strong,

    // so the incoming attribute value is an error.

    return CollisionResult::kConflict;

    return overlay ? CollisionResult::kTakeNew : CollisionResult::kConflict;

  }



  CHECK(incoming_attr != nullptr && existing_attr != nullptr);
@@ -323,8 +324,9 @@ ResourceTable::CollisionResult ResourceTable::ResolveValueCollision(Value* exist 
  return CollisionResult::kConflict;

}



ResourceTable::CollisionResult ResourceTable::IgnoreCollision(Value* /** existing **/,

                                                              Value* /** incoming **/) {

ResourceTable::CollisionResult ResourceTable::IgnoreCollision(Value* /* existing */,

                                                              Value* /* incoming */,

                                                              bool /* overlay */) {

  return CollisionResult::kKeepBoth;

}
@@ -440,7 +442,7 @@ bool ResourceTable::AddResourceImpl(const ResourceNameRef& name, const ResourceI 
    // Resource does not exist, add it now.

    config_value->value = std::move(value);

  } else {

    switch (conflict_resolver(config_value->value.get(), value.get())) {

    switch (conflict_resolver(config_value->value.get(), value.get(), false /* overlay */)) {

      case CollisionResult::kKeepBoth:

        // Insert the value ignoring for duplicate configurations

        entry->values.push_back(util::make_unique<ResourceConfigValue>(config, product));

tools/aapt2/ResourceTable.h

+3 −3
Original line number Diff line number Diff line
@@ -228,13 +228,13 @@ class ResourceTable { 


  enum class CollisionResult { kKeepBoth, kKeepOriginal, kConflict, kTakeNew };



  using CollisionResolverFunc = std::function<CollisionResult(Value*, Value*)>;

  using CollisionResolverFunc = std::function<CollisionResult(Value*, Value*, bool)>;



  // When a collision of resources occurs, this method decides which value to keep.

  static CollisionResult ResolveValueCollision(Value* existing, Value* incoming);

  static CollisionResult ResolveValueCollision(Value* existing, Value* incoming, bool overlay);



  // When a collision of resources occurs, this method keeps both values

  static CollisionResult IgnoreCollision(Value* existing, Value* incoming);

  static CollisionResult IgnoreCollision(Value* existing, Value* incoming, bool overlay);



  bool AddResource(const ResourceNameRef& name, const android::ConfigDescription& config,

                   const android::StringPiece& product, std::unique_ptr<Value> value,

tools/aapt2/ResourceValues.cpp

+4 −0
Original line number Diff line number Diff line
@@ -574,6 +574,10 @@ bool Attribute::Equals(const Value* value) const { 
}



bool Attribute::IsCompatibleWith(const Attribute& attr) const {

  if (Equals(&attr)) {

    return true;

  }



  // If the high bits are set on any of these attribute type masks, then they are incompatible.

  // We don't check that flags and enums are identical.

  if ((type_mask & ~android::ResTable_map::TYPE_ANY) != 0 ||

tools/aapt2/link/TableMerger.cpp

+3 −7
Original line number Diff line number Diff line
@@ -188,7 +188,7 @@ static ResourceTable::CollisionResult ResolveMergeCollision(Value* existing, Val 
    }

  }

  // Delegate to the default handler.

  return ResourceTable::ResolveValueCollision(existing, incoming);

  return ResourceTable::ResolveValueCollision(existing, incoming, true /* overlay */);

}



static ResourceTable::CollisionResult MergeConfigValue(IAaptContext* context,
@@ -206,15 +206,11 @@ static ResourceTable::CollisionResult MergeConfigValue(IAaptContext* context, 
  if (overlay) {

    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,

                                                            false /* overlay */);

  }



  if (collision_result == CollisionResult::kConflict) {

    if (overlay) {

      return CollisionResult::kTakeNew;

    }



    // Error!

    context->GetDiagnostics()->Error(DiagMessage(src_value->GetSource())

                                     << "resource '" << res_name << "' has a conflicting value for "

                                     << "configuration (" << src_config_value->config << ")");

tools/aapt2/link/TableMerger_test.cpp

+104 −0
Original line number Diff line number Diff line
@@ -352,6 +352,110 @@ TEST_F(TableMergerTest, MergeAddResourceFromOverlayWithAutoAddOverlay) { 
  ASSERT_TRUE(merger.Merge({}, table_b.get(), false /*overlay*/));

}



TEST_F(TableMergerTest, OverrideAttributeSameFormatsWithOverlay) {

  std::unique_ptr<ResourceTable> base =

      test::ResourceTableBuilder()

          .SetPackageId("", 0x7f)

          .AddValue("attr/foo", test::AttributeBuilder()

              .SetTypeMask(android::ResTable_map::TYPE_STRING)

              .SetWeak(false)

              .Build())

          .Build();



  std::unique_ptr<ResourceTable> overlay =

      test::ResourceTableBuilder()

          .SetPackageId("", 0x7f)

          .AddValue("attr/foo", test::AttributeBuilder()

              .SetTypeMask(android::ResTable_map::TYPE_STRING)

              .SetWeak(false)

              .Build())

          .Build();



  ResourceTable final_table;

  TableMergerOptions options;

  options.auto_add_overlay = false;

  TableMerger merger(context_.get(), &final_table, options);



  ASSERT_TRUE(merger.Merge({}, base.get(), false /*overlay*/));

  ASSERT_TRUE(merger.Merge({}, overlay.get(), true /*overlay*/));

}



TEST_F(TableMergerTest, FailToOverrideConflictingAttributeFormatsWithOverlay) {

  std::unique_ptr<ResourceTable> base =

      test::ResourceTableBuilder()

          .SetPackageId("", 0x7f)

          .AddValue("attr/foo", test::AttributeBuilder()

              .SetTypeMask(android::ResTable_map::TYPE_ANY)

              .SetWeak(false)

              .Build())

          .Build();



  std::unique_ptr<ResourceTable> overlay =

      test::ResourceTableBuilder()

          .SetPackageId("", 0x7f)

          .AddValue("attr/foo", test::AttributeBuilder()

              .SetTypeMask(android::ResTable_map::TYPE_STRING)

              .SetWeak(false)

              .Build())

          .Build();



  ResourceTable final_table;

  TableMergerOptions options;

  options.auto_add_overlay = false;

  TableMerger merger(context_.get(), &final_table, options);



  ASSERT_TRUE(merger.Merge({}, base.get(), false /*overlay*/));

  ASSERT_FALSE(merger.Merge({}, overlay.get(), true /*overlay*/));

}



TEST_F(TableMergerTest, FailToOverrideConflictingFlagsAndEnumsWithOverlay) {

  std::unique_ptr<ResourceTable> base =

      test::ResourceTableBuilder()

          .SetPackageId("", 0x7f)

          .AddValue("attr/foo", test::AttributeBuilder()

              .SetTypeMask(android::ResTable_map::TYPE_FLAGS)

              .Build())

          .Build();



  std::unique_ptr<ResourceTable> overlay =

      test::ResourceTableBuilder()

          .SetPackageId("", 0x7f)

          .AddValue("attr/foo", test::AttributeBuilder()

              .SetTypeMask(android::ResTable_map::TYPE_FLAGS)

              .SetWeak(false)

              .Build())

          .Build();



  ResourceTable final_table;

  TableMergerOptions options;

  options.auto_add_overlay = false;

  TableMerger merger(context_.get(), &final_table, options);



  ASSERT_TRUE(merger.Merge({}, base.get(), false /*overlay*/));

  ASSERT_FALSE(merger.Merge({}, overlay.get(), true /*overlay*/));



  base = test::ResourceTableBuilder()

      .SetPackageId("", 0x7f)

      .AddValue("attr/foo", test::AttributeBuilder()

          .SetTypeMask(android::ResTable_map::TYPE_ENUM)

          .Build())

        .Build();



  overlay = test::ResourceTableBuilder()

      .SetPackageId("", 0x7f)

      .AddValue("attr/foo", test::AttributeBuilder()

          .SetTypeMask(android::ResTable_map::TYPE_ENUM)

          .SetWeak(false)

          .Build())

      .Build();



  ResourceTable final_table2;

  TableMerger merger2(context_.get(), &final_table2, options);



  ASSERT_TRUE(merger2.Merge({}, base.get(), false /*overlay*/));

  ASSERT_FALSE(merger2.Merge({}, overlay.get(), true /*overlay*/));

}



TEST_F(TableMergerTest, FailToMergeNewResourceWithoutAutoAddOverlay) {

  std::unique_ptr<ResourceTable> table_a =

      test::ResourceTableBuilder().SetPackageId("", 0x7f).Build();