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

Commit 5855de7e authored by Ryan Mitchell's avatar Ryan Mitchell
Browse files

Use R field directly for shared lib styleable attrs 

If a shared library exposes an attribute and a client uses the
attribute in its own styleable, the value of the lib attribute
resource id in the client styleable must be fixed with the correct
package id at runtime. Since the client will not have an
onResourcesLoaded to call, the client should directly reference the
attribute resource field in its styleable.

Bug: 147674078
Test: aapt2_tests
Change-Id: I8e64bb2d3165a7072e2604fe1730b248545978f4
parent d145f376

tools/aapt2/cmd/Link.cpp

+10 −0
Original line number Diff line number Diff line
@@ -2263,6 +2263,16 @@ int LinkCommand::Action(const std::vector<std::string>& args) { 
    return 1;

  }



  if (shared_lib_ && options_.private_symbols) {

    // If a shared library styleable in a public R.java uses a private attribute, attempting to

    // reference the private attribute within the styleable array will cause a link error because

    // the private attribute will not be emitted in the public R.java.

    context.GetDiagnostics()->Error(DiagMessage()

                                    << "--shared-lib cannot currently be used in combination with"

                                    << " --private-symbols");

    return 1;

  }



  if (options_.merge_only && !static_lib_) {

    context.GetDiagnostics()->Error(

        DiagMessage() << "the --merge-only flag can be only used when building a static library");

tools/aapt2/cmd/Link_test.cpp

+76 −1
Original line number Diff line number Diff line
@@ -14,13 +14,16 @@ 
 * limitations under the License.

 */



#include "AppInfo.h"

#include "Link.h"



#include <android-base/file.h>



#include "AppInfo.h"

#include "LoadedApk.h"

#include "test/Test.h"



using testing::Eq;

using testing::HasSubstr;

using testing::Ne;



namespace aapt {
@@ -317,4 +320,76 @@ TEST_F(LinkTest, AppInfoWithUsesSplit) { 
  ASSERT_TRUE(Link(link_args, feature2_files_dir, &diag));

}



TEST_F(LinkTest, SharedLibraryAttributeRJava) {

  StdErrDiagnostics diag;

  const std::string lib_values =

      R"(<resources>

           <attr name="foo"/>

           <public type="attr" name="foo" id="0x00010001"/>

           <declare-styleable name="LibraryStyleable">

             <attr name="foo" />

           </declare-styleable>

         </resources>)";



  const std::string client_values =

      R"(<resources>

           <attr name="bar" />

           <declare-styleable name="ClientStyleable">

             <attr name="com.example.lib:foo" />

             <attr name="bar" />

           </declare-styleable>

         </resources>)";



  // Build a library with a public attribute

  const std::string lib_res = GetTestPath("library-res");

  ASSERT_TRUE(CompileFile(GetTestPath("res/values/values.xml"), lib_values, lib_res, &diag));



  const std::string lib_apk = GetTestPath("library.apk");

  const std::string lib_java = GetTestPath("library_java");

  // clang-format off

  auto lib_manifest = ManifestBuilder(this)

      .SetPackageName("com.example.lib")

      .Build();



  auto lib_link_args = LinkCommandBuilder(this)

      .SetManifestFile(lib_manifest)

      .AddFlag("--shared-lib")

      .AddParameter("--java", lib_java)

      .AddCompiledResDir(lib_res, &diag)

      .Build(lib_apk);

  // clang-format on

  ASSERT_TRUE(Link(lib_link_args, &diag));



  const std::string lib_r_java = lib_java + "/com/example/lib/R.java";

  std::string lib_r_contents;

  ASSERT_TRUE(android::base::ReadFileToString(lib_r_java, &lib_r_contents));

  EXPECT_THAT(lib_r_contents, HasSubstr(" public static int foo=0x00010001;"));

  EXPECT_THAT(lib_r_contents, HasSubstr(" com.example.lib.R.attr.foo"));



  // Build a client that uses the library attribute in a declare-styleable

  const std::string client_res = GetTestPath("client-res");

  ASSERT_TRUE(CompileFile(GetTestPath("res/values/values.xml"), client_values, client_res, &diag));



  const std::string client_apk = GetTestPath("client.apk");

  const std::string client_java = GetTestPath("client_java");

  // clang-format off

  auto client_manifest = ManifestBuilder(this)

      .SetPackageName("com.example.client")

      .Build();



  auto client_link_args = LinkCommandBuilder(this)

      .SetManifestFile(client_manifest)

      .AddParameter("--java", client_java)

      .AddParameter("-I", lib_apk)

      .AddCompiledResDir(client_res, &diag)

      .Build(client_apk);

  // clang-format on

  ASSERT_TRUE(Link(client_link_args, &diag));



  const std::string client_r_java = client_java + "/com/example/client/R.java";

  std::string client_r_contents;

  ASSERT_TRUE(android::base::ReadFileToString(client_r_java, &client_r_contents));

  EXPECT_THAT(client_r_contents, HasSubstr(" com.example.lib.R.attr.foo, 0x7f010000"));

}



}  // namespace aapt

tools/aapt2/format/binary/TableFlattener.cpp

+0 −1
Original line number Diff line number Diff line
@@ -570,7 +570,6 @@ class PackageFlattener { 
      ResourceEntry* entry = sorted_entries->at(entryIndex);



      // Populate the config masks for this entry.



      if (entry->visibility.level == Visibility::Level::kPublic) {

        config_masks[entry->id.value()] |= util::HostToDevice32(ResTable_typeSpec::SPEC_PUBLIC);

      }

tools/aapt2/java/ClassDefinition.h

+23 −6
Original line number Diff line number Diff line
@@ -70,8 +70,8 @@ class PrimitiveMember : public ClassMember { 
    return name_;

  }



  void Print(bool final, text::Printer* printer, bool strip_api_annotations = false)

      const override {

  void Print(bool final, text::Printer* printer,

             bool strip_api_annotations = false) const override {

    using std::to_string;



    ClassMember::Print(final, printer, strip_api_annotations);
@@ -127,13 +127,13 @@ using IntMember = PrimitiveMember<uint32_t>; 
using ResourceMember = PrimitiveMember<ResourceId>;

using StringMember = PrimitiveMember<std::string>;



template <typename T>

template <typename T, typename StringConverter>

class PrimitiveArrayMember : public ClassMember {

 public:

  explicit PrimitiveArrayMember(const android::StringPiece& name) : name_(name.to_string()) {}



  void AddElement(const T& val) {

    elements_.push_back(val);

    elements_.emplace_back(val);

  }



  bool empty() const override {
@@ -158,7 +158,7 @@ class PrimitiveArrayMember : public ClassMember { 
        printer->Println();

      }



      printer->Print(to_string(*current));

      printer->Print(StringConverter::ToString(*current));

      if (std::distance(current, end) > 1) {

        printer->Print(", ");

      }
@@ -175,7 +175,24 @@ class PrimitiveArrayMember : public ClassMember { 
  std::vector<T> elements_;

};



using ResourceArrayMember = PrimitiveArrayMember<ResourceId>;

struct FieldReference {

  explicit FieldReference(std::string reference) : ref(std::move(reference)) {

  }

  std::string ref;

};



struct ResourceArrayMemberStringConverter {

  static std::string ToString(const std::variant<ResourceId, FieldReference>& ref) {

    if (auto id = std::get_if<ResourceId>(&ref)) {

      return to_string(*id);

    } else {

      return std::get<FieldReference>(ref).ref;

    }

  }

};



using ResourceArrayMember = PrimitiveArrayMember<std::variant<ResourceId, FieldReference>,

                                                 ResourceArrayMemberStringConverter>;



// Represents a method in a class.

class MethodDefinition : public ClassMember {

tools/aapt2/java/JavaClassGenerator.cpp

+36 −27
Original line number Diff line number Diff line
@@ -224,7 +224,16 @@ static bool operator<(const StyleableAttr& lhs, const StyleableAttr& rhs) { 
  return cmp_ids_dynamic_after_framework(lhs_id, rhs_id);

}



void JavaClassGenerator::ProcessStyleable(const ResourceNameRef& name, const ResourceId& id,

static FieldReference GetRFieldReference(const ResourceName& name,

                                         StringPiece fallback_package_name) {

  const std::string package_name =

      name.package.empty() ? fallback_package_name.to_string() : name.package;

  const std::string entry = JavaClassGenerator::TransformToFieldName(name.entry);

  return FieldReference(

      StringPrintf("%s.R.%s.%s", package_name.c_str(), to_string(name.type).data(), entry.c_str()));

}



bool JavaClassGenerator::ProcessStyleable(const ResourceNameRef& name, const ResourceId& id,

                                          const Styleable& styleable,

                                          const StringPiece& package_name_to_generate,

                                          ClassDefinition* out_class_def,
@@ -340,14 +349,29 @@ void JavaClassGenerator::ProcessStyleable(const ResourceNameRef& name, const Res 


  // Add the ResourceIds to the array member.

  for (size_t i = 0; i < attr_count; i++) {

    const ResourceId id = sorted_attributes[i].attr_ref->id.value_or_default(ResourceId(0));

    array_def->AddElement(id);

    const StyleableAttr& attr = sorted_attributes[i];

    std::string r_txt_contents;

    if (attr.symbol && attr.symbol.value().is_dynamic) {

      if (!attr.attr_ref->name) {

        error_ = "unable to determine R.java field name of dynamic resource";

        return false;

      }



      const FieldReference field_name =

          GetRFieldReference(attr.attr_ref->name.value(), package_name_to_generate);

      array_def->AddElement(field_name);

      r_txt_contents = field_name.ref;

    } else {

      const ResourceId attr_id = attr.attr_ref->id.value_or_default(ResourceId(0));

      array_def->AddElement(attr_id);

      r_txt_contents = to_string(attr_id);

    }



    if (r_txt_printer != nullptr) {

      if (i != 0) {

        r_txt_printer->Print(",");

      }

      r_txt_printer->Print(" ").Print(id.to_string());

      r_txt_printer->Print(" ").Print(r_txt_contents);

    }

  }
@@ -419,19 +443,7 @@ void JavaClassGenerator::ProcessStyleable(const ResourceNameRef& name, const Res 
    }

  }



  // If there is a rewrite method to generate, add the statements that rewrite package IDs

  // for this styleable.

  if (out_rewrite_method != nullptr) {

    out_rewrite_method->AppendStatement(

        StringPrintf("for (int i = 0; i < styleable.%s.length; i++) {", array_field_name.data()));

    out_rewrite_method->AppendStatement(

        StringPrintf("  if ((styleable.%s[i] & 0xff000000) == 0) {", array_field_name.data()));

    out_rewrite_method->AppendStatement(

        StringPrintf("    styleable.%s[i] = (styleable.%s[i] & 0x00ffffff) | packageIdBits;",

                     array_field_name.data(), array_field_name.data()));

    out_rewrite_method->AppendStatement("  }");

    out_rewrite_method->AppendStatement("}");

  }

  return true;

}



void JavaClassGenerator::ProcessResource(const ResourceNameRef& name, const ResourceId& id,
@@ -448,8 +460,7 @@ void JavaClassGenerator::ProcessResource(const ResourceNameRef& name, const Reso 


  const std::string field_name = TransformToFieldName(name.entry);

  if (out_class_def != nullptr) {

    std::unique_ptr<ResourceMember> resource_member =

        util::make_unique<ResourceMember>(field_name, real_id);

    auto resource_member = util::make_unique<ResourceMember>(field_name, real_id);



    // Build the comments and annotations for this entry.

    AnnotationProcessor* processor = resource_member->GetCommentBuilder();
@@ -551,12 +562,11 @@ bool JavaClassGenerator::ProcessType(const StringPiece& package_name_to_generate 


    if (resource_name.type == ResourceType::kStyleable) {

      CHECK(!entry->values.empty());



      const Styleable* styleable =

          static_cast<const Styleable*>(entry->values.front()->value.get());



      ProcessStyleable(resource_name, id, *styleable, package_name_to_generate, out_type_class_def,

                       out_rewrite_method_def, r_txt_printer);

      const auto styleable = reinterpret_cast<const Styleable*>(entry->values.front()->value.get());

      if (!ProcessStyleable(resource_name, id, *styleable, package_name_to_generate,

                            out_type_class_def, out_rewrite_method_def, r_txt_printer)) {

        return false;

      }

    } else {

      ProcessResource(resource_name, id, *entry, out_type_class_def, out_rewrite_method_def,

                      r_txt_printer);
@@ -626,8 +636,7 @@ bool JavaClassGenerator::Generate(const StringPiece& package_name_to_generate, 


      if (type->type == ResourceType::kAttr) {

        // Also include private attributes in this same class.

        const ResourceTableType* priv_type = package->FindType(ResourceType::kAttrPrivate);

        if (priv_type) {

        if (const ResourceTableType* priv_type = package->FindType(ResourceType::kAttrPrivate)) {

          if (!ProcessType(package_name_to_generate, *package, *priv_type, class_def.get(),

                           rewrite_method.get(), r_txt_printer.get())) {

            return false;