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

Commit c6830a7c authored by Jeremy Meyer's avatar Jeremy Meyer
Browse files

Fix persisting which resources use feature flags 

File resources get into the table a different way when they haven't been
modified by the FlaggedXmlVersioner or XmlCompatVersioner and this
addresses that path.

With this change, during compile we now find if an xml resource uses
flags and store that in the XmlResource which is then persisted to and
retrieved from the proto. This means the FlaggedXmlVersioner doesn't
have to look for them in the case that the file config or minsdk is >=
baklava.

This also moved the FeatureFlagsFilter to before we version the files.
The FeatureFlagsFilter is what strips elements behind disabled read only
flags at compile time and removed the featureFlag attribute when the
element is behind an enabled read only flag. This is so that the
FlaggedXmlVersioner doesn't have to worry if the flags it sees are
read/write or readonly. It should only ever encounter read/write flags.

Test: Automation
Bug: 377974898
Flag: android.content.res.layout_readwrite_flags
Change-Id: Ia6cbf55bc9f8d594eeb5c44c143565e93684ae2c
parent 0890b9a3

tools/aapt2/ResourcesInternal.proto

+4 −1
Original line number Diff line number Diff line
@@ -50,8 +50,11 @@ message CompiledFile { 
  // Any symbols this file auto-generates/exports (eg. @+id/foo in an XML file).

  repeated Symbol exported_symbol = 5;



  // The status of the flag the file is behind if any

  // The status of the read only flag the file is behind if any

  uint32 flag_status = 6;

  bool flag_negated = 7;

  string flag_name = 8;



  // Whether the file uses read/write feature flags

  bool uses_readwrite_feature_flags = 9;

}

tools/aapt2/cmd/Compile.cpp

+43 −0
Original line number Diff line number Diff line
@@ -407,6 +407,45 @@ static bool IsValidFile(IAaptContext* context, const std::string& input_path) { 
  return true;

}



class FindReadWriteFlagsVisitor : public xml::Visitor {

 public:

  FindReadWriteFlagsVisitor(const FeatureFlagValues& feature_flag_values)

      : feature_flag_values_(feature_flag_values) {

  }



  void Visit(xml::Element* node) override {

    if (had_flags_) {

      return;

    }

    auto* attr = node->FindAttribute(xml::kSchemaAndroid, xml::kAttrFeatureFlag);

    if (attr != nullptr) {

      std::string_view flag_name = util::TrimWhitespace(attr->value);

      if (flag_name.starts_with('!')) {

        flag_name = flag_name.substr(1);

      }

      if (auto it = feature_flag_values_.find(flag_name); it != feature_flag_values_.end()) {

        if (!it->second.read_only) {

          had_flags_ = true;

          return;

        }

      } else {

        // Flag not passed to aapt2, must evaluate at runtime

        had_flags_ = true;

        return;

      }

    }

    VisitChildren(node);

  }



  bool HadFlags() const {

    return had_flags_;

  }



 private:

  bool had_flags_ = false;

  const FeatureFlagValues& feature_flag_values_;

};



static bool CompileXml(IAaptContext* context, const CompileOptions& options,

                       const ResourcePathData& path_data, io::IFile* file, IArchiveWriter* writer,

                       const std::string& output_path) {
@@ -436,6 +475,10 @@ static bool CompileXml(IAaptContext* context, const CompileOptions& options, 
  xmlres->file.type = ResourceFile::Type::kProtoXml;

  xmlres->file.flag = ParseFlag(path_data.flag_name);



  FindReadWriteFlagsVisitor visitor(options.feature_flag_values);

  xmlres->root->Accept(&visitor);

  xmlres->file.uses_readwrite_feature_flags = visitor.HadFlags();



  if (xmlres->file.flag) {

    std::string error;

    auto flag_status = GetFlagStatus(xmlres->file.flag, options.feature_flag_values, &error);

tools/aapt2/cmd/Link.cpp

+14 −8
Original line number Diff line number Diff line
@@ -615,6 +615,8 @@ bool ResourceFileFlattener::Flatten(ResourceTable* table, IArchiveWriter* archiv 
            file_op.xml_to_flatten->file.source = file_ref->GetSource();

            file_op.xml_to_flatten->file.name =

                ResourceName(pkg->name, type->named_type, entry->name);

            file_op.xml_to_flatten->file.uses_readwrite_feature_flags =

                config_value->uses_readwrite_feature_flags;

          }



          // NOTE(adamlesinski): Explicitly construct a StringPiece here, or
@@ -647,6 +649,17 @@ bool ResourceFileFlattener::Flatten(ResourceTable* table, IArchiveWriter* archiv 
            }

          }



          FeatureFlagsFilterOptions flags_filter_options;

          // Don't fail on unrecognized flags or flags without values as these flags might be

          // defined and have a value by the time they are evaluated at runtime.

          flags_filter_options.fail_on_unrecognized_flags = false;

          flags_filter_options.flags_must_have_value = false;

          flags_filter_options.remove_disabled_elements = true;

          FeatureFlagsFilter flags_filter(options_.feature_flag_values, flags_filter_options);

          if (!flags_filter.Consume(context_, file_op.xml_to_flatten.get())) {

            return 1;

          }



          std::vector<std::unique_ptr<xml::XmlResource>> versioned_docs =

              LinkAndVersionXmlFile(table, &file_op);

          if (versioned_docs.empty()) {
@@ -673,6 +686,7 @@ bool ResourceFileFlattener::Flatten(ResourceTable* table, IArchiveWriter* archiv 


              // Update the output format of this XML file.

              file_ref->type = XmlFileTypeForOutputFormat(options_.output_format);



              bool result = table->AddResource(

                  NewResourceBuilder(file.name)

                      .SetValue(std::move(file_ref), file.config)
@@ -685,14 +699,6 @@ bool ResourceFileFlattener::Flatten(ResourceTable* table, IArchiveWriter* archiv 
              }

            }



            FeatureFlagsFilterOptions flags_filter_options;

            flags_filter_options.fail_on_unrecognized_flags = false;

            flags_filter_options.flags_must_have_value = false;

            FeatureFlagsFilter flags_filter(options_.feature_flag_values, flags_filter_options);

            if (!flags_filter.Consume(context_, doc.get())) {

              return 1;

            }



            error |= !FlattenXml(context_, *doc, dst_path, options_.keep_raw_values,

                                 false /*utf16*/, options_.output_format, archive_writer);

          }

tools/aapt2/format/proto/ProtoDeserialize.cpp

+1 −0
Original line number Diff line number Diff line
@@ -640,6 +640,7 @@ bool DeserializeCompiledFileFromPb(const pb::internal::CompiledFile& pb_file, 
  out_file->name = name_ref.ToResourceName();

  out_file->source.path = pb_file.source_path();

  out_file->type = DeserializeFileReferenceTypeFromPb(pb_file.type());

  out_file->uses_readwrite_feature_flags = pb_file.uses_readwrite_feature_flags();



  out_file->flag_status = (FlagStatus)pb_file.flag_status();

  if (!pb_file.flag_name().empty()) {

tools/aapt2/format/proto/ProtoSerialize.cpp

+1 −0
Original line number Diff line number Diff line
@@ -767,6 +767,7 @@ void SerializeCompiledFileToPb(const ResourceFile& file, pb::internal::CompiledF 
    out_file->set_flag_negated(file.flag->negated);

    out_file->set_flag_name(file.flag->name);

  }

  out_file->set_uses_readwrite_feature_flags(file.uses_readwrite_feature_flags);



  for (const SourcedResourceName& exported : file.exported_symbols) {

    pb::internal::CompiledFile_Symbol* pb_symbol = out_file->add_exported_symbol();