AAPT2: Cleanup Visitors for XML and Values

Test: make aapt2_tests
Change-Id: Ib61f64c155a380115610edeaf2d65e60258a2426
diff --git a/tools/aapt2/Debug.cpp b/tools/aapt2/Debug.cpp
index dba9cb5..1555d6126 100644
--- a/tools/aapt2/Debug.cpp
+++ b/tools/aapt2/Debug.cpp
@@ -35,9 +35,9 @@
 
 namespace {
 
-class PrintVisitor : public ValueVisitor {
+class PrintVisitor : public DescendingValueVisitor {
  public:
-  using ValueVisitor::Visit;
+  using DescendingValueVisitor::Visit;
 
   void Visit(Attribute* attr) override {
     std::cout << "(attr) type=";
diff --git a/tools/aapt2/ResourceValues.cpp b/tools/aapt2/ResourceValues.cpp
index 1cba194..e013729 100644
--- a/tools/aapt2/ResourceValues.cpp
+++ b/tools/aapt2/ResourceValues.cpp
@@ -35,15 +35,25 @@
 }
 
 template <typename Derived>
-void BaseValue<Derived>::Accept(RawValueVisitor* visitor) {
+void BaseValue<Derived>::Accept(ValueVisitor* visitor) {
   visitor->Visit(static_cast<Derived*>(this));
 }
 
 template <typename Derived>
-void BaseItem<Derived>::Accept(RawValueVisitor* visitor) {
+void BaseValue<Derived>::Accept(ConstValueVisitor* visitor) const {
+  visitor->Visit(static_cast<const Derived*>(this));
+}
+
+template <typename Derived>
+void BaseItem<Derived>::Accept(ValueVisitor* visitor) {
   visitor->Visit(static_cast<Derived*>(this));
 }
 
+template <typename Derived>
+void BaseItem<Derived>::Accept(ConstValueVisitor* visitor) const {
+  visitor->Visit(static_cast<const Derived*>(this));
+}
+
 RawString::RawString(const StringPool::Ref& ref) : value(ref) {}
 
 bool RawString::Equals(const Value* value) const {
diff --git a/tools/aapt2/ResourceValues.h b/tools/aapt2/ResourceValues.h
index 275864b..742765d 100644
--- a/tools/aapt2/ResourceValues.h
+++ b/tools/aapt2/ResourceValues.h
@@ -33,7 +33,8 @@
 
 namespace aapt {
 
-struct RawValueVisitor;
+class ValueVisitor;
+class ConstValueVisitor;
 
 // A resource value. This is an all-encompassing representation
 // of Item and Map and their subclasses. The way to do
@@ -45,36 +46,58 @@
   virtual ~Value() = default;
 
   // Whether this value is weak and can be overridden without warning or error. Default is false.
-  bool IsWeak() const { return weak_; }
+  bool IsWeak() const {
+    return weak_;
+  }
 
-  void SetWeak(bool val) { weak_ = val; }
+  void SetWeak(bool val) {
+    weak_ = val;
+  }
 
-  // Whether the value is marked as translatable.
-  // This does not persist when flattened.
+  // Whether the value is marked as translatable. This does not persist when flattened to binary.
   // It is only used during compilation phase.
-  void SetTranslatable(bool val) { translatable_ = val; }
+  void SetTranslatable(bool val) {
+    translatable_ = val;
+  }
 
   // Default true.
-  bool IsTranslatable() const { return translatable_; }
+  bool IsTranslatable() const {
+    return translatable_;
+  }
 
   // Returns the source where this value was defined.
-  const Source& GetSource() const { return source_; }
+  const Source& GetSource() const {
+    return source_;
+  }
 
-  void SetSource(const Source& source) { source_ = source; }
+  void SetSource(const Source& source) {
+    source_ = source;
+  }
 
-  void SetSource(Source&& source) { source_ = std::move(source); }
+  void SetSource(Source&& source) {
+    source_ = std::move(source);
+  }
 
   // Returns the comment that was associated with this resource.
-  const std::string& GetComment() const { return comment_; }
+  const std::string& GetComment() const {
+    return comment_;
+  }
 
-  void SetComment(const android::StringPiece& str) { comment_ = str.to_string(); }
+  void SetComment(const android::StringPiece& str) {
+    comment_ = str.to_string();
+  }
 
-  void SetComment(std::string&& str) { comment_ = std::move(str); }
+  void SetComment(std::string&& str) {
+    comment_ = std::move(str);
+  }
 
   virtual bool Equals(const Value* value) const = 0;
 
   // Calls the appropriate overload of ValueVisitor.
-  virtual void Accept(RawValueVisitor* visitor) = 0;
+  virtual void Accept(ValueVisitor* visitor) = 0;
+
+  // Calls the appropriate overload of ConstValueVisitor.
+  virtual void Accept(ConstValueVisitor* visitor) const = 0;
 
   // Clone the value. `new_pool` is the new StringPool that
   // any resources with strings should use when copying their string.
@@ -95,7 +118,8 @@
 // Inherit from this to get visitor accepting implementations for free.
 template <typename Derived>
 struct BaseValue : public Value {
-  void Accept(RawValueVisitor* visitor) override;
+  void Accept(ValueVisitor* visitor) override;
+  void Accept(ConstValueVisitor* visitor) const override;
 };
 
 // A resource item with a single value. This maps to android::ResTable_entry.
@@ -111,7 +135,8 @@
 // Inherit from this to get visitor accepting implementations for free.
 template <typename Derived>
 struct BaseItem : public Item {
-  void Accept(RawValueVisitor* visitor) override;
+  void Accept(ValueVisitor* visitor) override;
+  void Accept(ConstValueVisitor* visitor) const override;
 };
 
 // A reference to another resource. This maps to android::Res_value::TYPE_REFERENCE.
@@ -144,7 +169,10 @@
 
 // An ID resource. Has no real value, just a place holder.
 struct Id : public BaseItem<Id> {
-  Id() { weak_ = true; }
+  Id() {
+    weak_ = true;
+  }
+
   bool Equals(const Value* value) const override;
   bool Flatten(android::Res_value* out) const override;
   Id* Clone(StringPool* new_pool) const override;
diff --git a/tools/aapt2/ValueVisitor.h b/tools/aapt2/ValueVisitor.h
index eb4fa49..4e74ec3 100644
--- a/tools/aapt2/ValueVisitor.h
+++ b/tools/aapt2/ValueVisitor.h
@@ -22,12 +22,11 @@
 
 namespace aapt {
 
-/**
- * Visits a value and invokes the appropriate method based on its type. Does not
- * traverse into compound types. Use ValueVisitor for that.
- */
-struct RawValueVisitor {
-  virtual ~RawValueVisitor() = default;
+// Visits a value and invokes the appropriate method based on its type.
+// Does not traverse into compound types. Use ValueVisitor for that.
+class ValueVisitor {
+ public:
+  virtual ~ValueVisitor() = default;
 
   virtual void VisitAny(Value* value) {}
   virtual void VisitItem(Item* value) { VisitAny(value); }
@@ -46,21 +45,67 @@
   virtual void Visit(Styleable* value) { VisitAny(value); }
 };
 
+// Const version of ValueVisitor.
+class ConstValueVisitor {
+ public:
+  virtual ~ConstValueVisitor() = default;
+
+  virtual void VisitAny(const Value* value) {
+  }
+  virtual void VisitItem(const Item* value) {
+    VisitAny(value);
+  }
+  virtual void Visit(const Reference* value) {
+    VisitItem(value);
+  }
+  virtual void Visit(const RawString* value) {
+    VisitItem(value);
+  }
+  virtual void Visit(const String* value) {
+    VisitItem(value);
+  }
+  virtual void Visit(const StyledString* value) {
+    VisitItem(value);
+  }
+  virtual void Visit(const FileReference* value) {
+    VisitItem(value);
+  }
+  virtual void Visit(const Id* value) {
+    VisitItem(value);
+  }
+  virtual void Visit(const BinaryPrimitive* value) {
+    VisitItem(value);
+  }
+
+  virtual void Visit(const Attribute* value) {
+    VisitAny(value);
+  }
+  virtual void Visit(const Style* value) {
+    VisitAny(value);
+  }
+  virtual void Visit(const Array* value) {
+    VisitAny(value);
+  }
+  virtual void Visit(const Plural* value) {
+    VisitAny(value);
+  }
+  virtual void Visit(const Styleable* value) {
+    VisitAny(value);
+  }
+};
+
 // NOLINT, do not add parentheses around T.
 #define DECL_VISIT_COMPOUND_VALUE(T)                   \
   virtual void Visit(T* value) override { /* NOLINT */ \
     VisitSubValues(value);                             \
   }
 
-/**
- * Visits values, and if they are compound values, visits the components as
- * well.
- */
-struct ValueVisitor : public RawValueVisitor {
+// Visits values, and if they are compound values, descends into their components as well.
+struct DescendingValueVisitor : public ValueVisitor {
   // The compiler will think we're hiding an overload, when we actually intend
   // to call into RawValueVisitor. This will expose the visit methods in the
   // super class so the compiler knows we are trying to call them.
-  using RawValueVisitor::Visit;
+  using ValueVisitor::Visit;
 
   void VisitSubValues(Attribute* attribute) {
     for (Attribute::Symbol& symbol : attribute->symbols) {
@@ -106,37 +151,30 @@
   DECL_VISIT_COMPOUND_VALUE(Styleable);
 };
 
-/**
- * Do not use directly. Helper struct for dyn_cast.
- */
+// Do not use directly. Helper struct for dyn_cast.
 template <typename T>
-struct DynCastVisitor : public RawValueVisitor {
-  T* value = nullptr;
+struct DynCastVisitor : public ConstValueVisitor {
+  const T* value = nullptr;
 
-  void Visit(T* v) override { value = v; }
+  void Visit(const T* v) override {
+    value = v;
+  }
 };
 
-/**
- * Specialization that checks if the value is an Item.
- */
+// Specialization that checks if the value is an Item.
 template <>
-struct DynCastVisitor<Item> : public RawValueVisitor {
-  Item* value = nullptr;
+struct DynCastVisitor<Item> : public ConstValueVisitor {
+  const Item* value = nullptr;
 
-  void VisitItem(Item* item) override { value = item; }
+  void VisitItem(const Item* item) override {
+    value = item;
+  }
 };
 
+// Returns a valid pointer to T if the value is an instance of T. Returns nullptr if value is
+// nullptr of if value is not an instance of T.
 template <typename T>
 const T* ValueCast(const Value* value) {
-  return ValueCast<T>(const_cast<Value*>(value));
-}
-
-/**
- * Returns a valid pointer to T if the Value is of subtype T.
- * Otherwise, returns nullptr.
- */
-template <typename T>
-T* ValueCast(Value* value) {
   if (!value) {
     return nullptr;
   }
@@ -145,8 +183,13 @@
   return visitor.value;
 }
 
-inline void VisitAllValuesInPackage(ResourceTablePackage* pkg,
-                                    RawValueVisitor* visitor) {
+// Non-const version of ValueCast.
+template <typename T>
+T* ValueCast(Value* value) {
+  return const_cast<T*>(ValueCast<T>(static_cast<const Value*>(value)));
+}
+
+inline void VisitAllValuesInPackage(ResourceTablePackage* pkg, ValueVisitor* visitor) {
   for (auto& type : pkg->types) {
     for (auto& entry : type->entries) {
       for (auto& config_value : entry->values) {
@@ -156,8 +199,7 @@
   }
 }
 
-inline void VisitAllValuesInTable(ResourceTable* table,
-                                  RawValueVisitor* visitor) {
+inline void VisitAllValuesInTable(ResourceTable* table, ValueVisitor* visitor) {
   for (auto& pkg : table->packages) {
     VisitAllValuesInPackage(pkg.get(), visitor);
   }
diff --git a/tools/aapt2/ValueVisitor_test.cpp b/tools/aapt2/ValueVisitor_test.cpp
index eb75b10..5aea77d 100644
--- a/tools/aapt2/ValueVisitor_test.cpp
+++ b/tools/aapt2/ValueVisitor_test.cpp
@@ -24,16 +24,16 @@
 
 namespace aapt {
 
-struct SingleReferenceVisitor : public ValueVisitor {
-  using ValueVisitor::Visit;
+struct SingleReferenceVisitor : public DescendingValueVisitor {
+  using DescendingValueVisitor::Visit;
 
   Reference* visited = nullptr;
 
   void Visit(Reference* ref) override { visited = ref; }
 };
 
-struct StyleVisitor : public ValueVisitor {
-  using ValueVisitor::Visit;
+struct StyleVisitor : public DescendingValueVisitor {
+  using DescendingValueVisitor::Visit;
 
   std::list<Reference*> visited_refs;
   Style* visited_style = nullptr;
@@ -42,7 +42,7 @@
 
   void Visit(Style* style) override {
     visited_style = style;
-    ValueVisitor::Visit(style);
+    DescendingValueVisitor::Visit(style);
   }
 };
 
diff --git a/tools/aapt2/cmd/Diff.cpp b/tools/aapt2/cmd/Diff.cpp
index 1a6f348..625c47c 100644
--- a/tools/aapt2/cmd/Diff.cpp
+++ b/tools/aapt2/cmd/Diff.cpp
@@ -22,7 +22,7 @@
 #include "process/IResourceTableConsumer.h"
 #include "process/SymbolTable.h"
 
-using android::StringPiece;
+using ::android::StringPiece;
 
 namespace aapt {
 
@@ -325,9 +325,9 @@
   return diff;
 }
 
-class ZeroingReferenceVisitor : public ValueVisitor {
+class ZeroingReferenceVisitor : public DescendingValueVisitor {
  public:
-  using ValueVisitor::Visit;
+  using DescendingValueVisitor::Visit;
 
   void Visit(Reference* ref) override {
     if (ref->name && ref->id) {
diff --git a/tools/aapt2/cmd/Link.cpp b/tools/aapt2/cmd/Link.cpp
index 3a2faa9..e3ee206d 100644
--- a/tools/aapt2/cmd/Link.cpp
+++ b/tools/aapt2/cmd/Link.cpp
@@ -33,6 +33,8 @@
 #include "Locale.h"
 #include "NameMangler.h"
 #include "ResourceUtils.h"
+#include "ResourceValues.h"
+#include "ValueVisitor.h"
 #include "cmd/Util.h"
 #include "compile/IdAssigner.h"
 #include "filter/ConfigFilter.h"
diff --git a/tools/aapt2/cmd/Optimize.cpp b/tools/aapt2/cmd/Optimize.cpp
index 33a1d3a..704faee 100644
--- a/tools/aapt2/cmd/Optimize.cpp
+++ b/tools/aapt2/cmd/Optimize.cpp
@@ -18,6 +18,7 @@
 #include <vector>
 
 #include "android-base/stringprintf.h"
+
 #include "androidfw/ResourceTypes.h"
 #include "androidfw/StringPiece.h"
 
diff --git a/tools/aapt2/compile/PseudolocaleGenerator.cpp b/tools/aapt2/compile/PseudolocaleGenerator.cpp
index 871ed4f..36c24bc 100644
--- a/tools/aapt2/compile/PseudolocaleGenerator.cpp
+++ b/tools/aapt2/compile/PseudolocaleGenerator.cpp
@@ -24,8 +24,8 @@
 #include "compile/Pseudolocalizer.h"
 #include "util/Util.h"
 
-using android::StringPiece;
-using android::StringPiece16;
+using ::android::StringPiece;
+using ::android::StringPiece16;
 
 namespace aapt {
 
@@ -215,7 +215,7 @@
 
 namespace {
 
-class Visitor : public RawValueVisitor {
+class Visitor : public ValueVisitor {
  public:
   // Either value or item will be populated upon visiting the value.
   std::unique_ptr<Value> value;
diff --git a/tools/aapt2/flatten/TableFlattener.cpp b/tools/aapt2/flatten/TableFlattener.cpp
index 14b776b..097727c 100644
--- a/tools/aapt2/flatten/TableFlattener.cpp
+++ b/tools/aapt2/flatten/TableFlattener.cpp
@@ -76,9 +76,9 @@
   uint32_t entry_key;
 };
 
-class MapFlattenVisitor : public RawValueVisitor {
+class MapFlattenVisitor : public ValueVisitor {
  public:
-  using RawValueVisitor::Visit;
+  using ValueVisitor::Visit;
 
   MapFlattenVisitor(ResTable_entry_ext* out_entry, BigBuffer* buffer)
       : out_entry_(out_entry), buffer_(buffer) {}
diff --git a/tools/aapt2/link/ReferenceLinker.cpp b/tools/aapt2/link/ReferenceLinker.cpp
index 71e828b..ad7d8b6 100644
--- a/tools/aapt2/link/ReferenceLinker.cpp
+++ b/tools/aapt2/link/ReferenceLinker.cpp
@@ -42,9 +42,9 @@
 // to the reference object.
 //
 // NOTE: All of the entries in the ResourceTable must be assigned IDs.
-class ReferenceLinkerVisitor : public ValueVisitor {
+class ReferenceLinkerVisitor : public DescendingValueVisitor {
  public:
-  using ValueVisitor::Visit;
+  using DescendingValueVisitor::Visit;
 
   ReferenceLinkerVisitor(const CallSite& callsite, IAaptContext* context, SymbolTable* symbols,
                          StringPool* string_pool, xml::IPackageDeclStack* decl)
diff --git a/tools/aapt2/link/ReferenceLinker.h b/tools/aapt2/link/ReferenceLinker.h
index 3b11bee..b0b4945 100644
--- a/tools/aapt2/link/ReferenceLinker.h
+++ b/tools/aapt2/link/ReferenceLinker.h
@@ -21,7 +21,6 @@
 
 #include "Resource.h"
 #include "ResourceValues.h"
-#include "ValueVisitor.h"
 #include "link/Linkers.h"
 #include "process/IResourceTableConsumer.h"
 #include "process/SymbolTable.h"
diff --git a/tools/aapt2/link/XmlReferenceLinker.cpp b/tools/aapt2/link/XmlReferenceLinker.cpp
index 6ebb80f..8852c8e 100644
--- a/tools/aapt2/link/XmlReferenceLinker.cpp
+++ b/tools/aapt2/link/XmlReferenceLinker.cpp
@@ -21,6 +21,7 @@
 #include "Diagnostics.h"
 #include "ResourceUtils.h"
 #include "SdkConstants.h"
+#include "ValueVisitor.h"
 #include "link/ReferenceLinker.h"
 #include "process/IResourceTableConsumer.h"
 #include "process/SymbolTable.h"
@@ -34,9 +35,9 @@
 // Visits all references (including parents of styles, references in styles, arrays, etc) and
 // links their symbolic name to their Resource ID, performing mangling and package aliasing
 // as needed.
-class ReferenceVisitor : public ValueVisitor {
+class ReferenceVisitor : public DescendingValueVisitor {
  public:
-  using ValueVisitor::Visit;
+  using DescendingValueVisitor::Visit;
 
   ReferenceVisitor(const CallSite& callsite, IAaptContext* context, SymbolTable* symbols,
                    xml::IPackageDeclStack* decls)
diff --git a/tools/aapt2/proto/TableProtoDeserializer.cpp b/tools/aapt2/proto/TableProtoDeserializer.cpp
index 4a88d61..a49f6f6 100644
--- a/tools/aapt2/proto/TableProtoDeserializer.cpp
+++ b/tools/aapt2/proto/TableProtoDeserializer.cpp
@@ -28,9 +28,9 @@
 
 namespace {
 
-class ReferenceIdToNameVisitor : public ValueVisitor {
+class ReferenceIdToNameVisitor : public DescendingValueVisitor {
  public:
-  using ValueVisitor::Visit;
+  using DescendingValueVisitor::Visit;
 
   explicit ReferenceIdToNameVisitor(const std::map<ResourceId, ResourceNameRef>* mapping)
       : mapping_(mapping) {
diff --git a/tools/aapt2/proto/TableProtoSerializer.cpp b/tools/aapt2/proto/TableProtoSerializer.cpp
index 3d5407c..e15a0ad 100644
--- a/tools/aapt2/proto/TableProtoSerializer.cpp
+++ b/tools/aapt2/proto/TableProtoSerializer.cpp
@@ -14,16 +14,17 @@
  * limitations under the License.
  */
 
+#include "proto/ProtoSerialize.h"
+
+#include "android-base/logging.h"
+
 #include "Resource.h"
 #include "ResourceTable.h"
 #include "StringPool.h"
 #include "ValueVisitor.h"
 #include "proto/ProtoHelpers.h"
-#include "proto/ProtoSerialize.h"
 #include "util/BigBuffer.h"
 
-#include "android-base/logging.h"
-
 using ::google::protobuf::io::CodedInputStream;
 using ::google::protobuf::io::CodedOutputStream;
 using ::google::protobuf::io::ZeroCopyOutputStream;
@@ -32,9 +33,9 @@
 
 namespace {
 
-class PbSerializerVisitor : public RawValueVisitor {
+class PbSerializerVisitor : public ConstValueVisitor {
  public:
-  using RawValueVisitor::Visit;
+  using ConstValueVisitor::Visit;
 
   // Constructor to use when expecting to serialize any value.
   PbSerializerVisitor(StringPool* source_pool, pb::Value* out_pb_value)
@@ -46,19 +47,19 @@
       : source_pool_(sourcePool), out_pb_value_(nullptr), out_pb_item_(outPbItem) {
   }
 
-  void Visit(Reference* ref) override {
+  void Visit(const Reference* ref) override {
     SerializeReferenceToPb(*ref, pb_item()->mutable_ref());
   }
 
-  void Visit(String* str) override {
+  void Visit(const String* str) override {
     pb_item()->mutable_str()->set_value(*str->value);
   }
 
-  void Visit(RawString* str) override {
+  void Visit(const RawString* str) override {
     pb_item()->mutable_raw_str()->set_value(*str->value);
   }
 
-  void Visit(StyledString* str) override {
+  void Visit(const StyledString* str) override {
     pb::StyledString* pb_str = pb_item()->mutable_styled_str();
     pb_str->set_value(str->value->value);
 
@@ -70,15 +71,15 @@
     }
   }
 
-  void Visit(FileReference* file) override {
+  void Visit(const FileReference* file) override {
     pb_item()->mutable_file()->set_path(*file->path);
   }
 
-  void Visit(Id* /*id*/) override {
+  void Visit(const Id* /*id*/) override {
     pb_item()->mutable_id();
   }
 
-  void Visit(BinaryPrimitive* prim) override {
+  void Visit(const BinaryPrimitive* prim) override {
     android::Res_value val = {};
     prim->Flatten(&val);
 
@@ -87,17 +88,17 @@
     pb_prim->set_data(val.data);
   }
 
-  void VisitItem(Item* item) override {
+  void VisitItem(const Item* item) override {
     LOG(FATAL) << "unimplemented item";
   }
 
-  void Visit(Attribute* attr) override {
+  void Visit(const Attribute* attr) override {
     pb::Attribute* pb_attr = pb_compound_value()->mutable_attr();
     pb_attr->set_format_flags(attr->type_mask);
     pb_attr->set_min_int(attr->min_int);
     pb_attr->set_max_int(attr->max_int);
 
-    for (auto& symbol : attr->symbols) {
+    for (const auto& symbol : attr->symbols) {
       pb::Attribute_Symbol* pb_symbol = pb_attr->add_symbol();
       SerializeItemCommonToPb(symbol.symbol, pb_symbol);
       SerializeReferenceToPb(symbol.symbol, pb_symbol->mutable_name());
@@ -105,7 +106,7 @@
     }
   }
 
-  void Visit(Style* style) override {
+  void Visit(const Style* style) override {
     pb::Style* pb_style = pb_compound_value()->mutable_style();
     if (style->parent) {
       SerializeReferenceToPb(style->parent.value(), pb_style->mutable_parent());
@@ -113,7 +114,7 @@
                           pb_style->mutable_parent_source());
     }
 
-    for (Style::Entry& entry : style->entries) {
+    for (const Style::Entry& entry : style->entries) {
       pb::Style_Entry* pb_entry = pb_style->add_entry();
       SerializeReferenceToPb(entry.key, pb_entry->mutable_key());
 
@@ -124,18 +125,18 @@
     }
   }
 
-  void Visit(Styleable* styleable) override {
+  void Visit(const Styleable* styleable) override {
     pb::Styleable* pb_styleable = pb_compound_value()->mutable_styleable();
-    for (Reference& entry : styleable->entries) {
+    for (const Reference& entry : styleable->entries) {
       pb::Styleable_Entry* pb_entry = pb_styleable->add_entry();
       SerializeItemCommonToPb(entry, pb_entry);
       SerializeReferenceToPb(entry, pb_entry->mutable_attr());
     }
   }
 
-  void Visit(Array* array) override {
+  void Visit(const Array* array) override {
     pb::Array* pb_array = pb_compound_value()->mutable_array();
-    for (auto& value : array->elements) {
+    for (const auto& value : array->elements) {
       pb::Array_Element* pb_element = pb_array->add_element();
       SerializeItemCommonToPb(*value, pb_element);
       PbSerializerVisitor sub_visitor(source_pool_, pb_element->mutable_item());
@@ -143,7 +144,7 @@
     }
   }
 
-  void Visit(Plural* plural) override {
+  void Visit(const Plural* plural) override {
     pb::Plural* pb_plural = pb_compound_value()->mutable_plural();
     const size_t count = plural->values.size();
     for (size_t i = 0; i < count; i++) {
diff --git a/tools/aapt2/unflatten/BinaryResourceParser.cpp b/tools/aapt2/unflatten/BinaryResourceParser.cpp
index 892aee6..b87278b 100644
--- a/tools/aapt2/unflatten/BinaryResourceParser.cpp
+++ b/tools/aapt2/unflatten/BinaryResourceParser.cpp
@@ -44,9 +44,9 @@
 
 // Visitor that converts a reference's resource ID to a resource name, given a mapping from
 // resource ID to resource name.
-class ReferenceIdToNameVisitor : public ValueVisitor {
+class ReferenceIdToNameVisitor : public DescendingValueVisitor {
  public:
-  using ValueVisitor::Visit;
+  using DescendingValueVisitor::Visit;
 
   explicit ReferenceIdToNameVisitor(const std::map<ResourceId, ResourceName>* mapping)
       : mapping_(mapping) {
diff --git a/tools/aapt2/xml/XmlDom.cpp b/tools/aapt2/xml/XmlDom.cpp
index 32ec7bc..b3e0a92 100644
--- a/tools/aapt2/xml/XmlDom.cpp
+++ b/tools/aapt2/xml/XmlDom.cpp
@@ -461,6 +461,12 @@
   visitor->AfterVisitElement(this);
 }
 
+void Element::Accept(ConstVisitor* visitor) const {
+  visitor->BeforeVisitElement(this);
+  visitor->Visit(this);
+  visitor->AfterVisitElement(this);
+}
+
 std::unique_ptr<Node> Text::Clone(const ElementCloneFunc&) const {
   auto t = util::make_unique<Text>();
   t->comment = comment;
@@ -474,6 +480,10 @@
   visitor->Visit(this);
 }
 
+void Text::Accept(ConstVisitor* visitor) const {
+  visitor->Visit(this);
+}
+
 void PackageAwareVisitor::BeforeVisitElement(Element* el) {
   std::vector<PackageDecl> decls;
   for (const NamespaceDecl& decl : el->namespace_decls) {
diff --git a/tools/aapt2/xml/XmlDom.h b/tools/aapt2/xml/XmlDom.h
index 9a9151d..063d7b9 100644
--- a/tools/aapt2/xml/XmlDom.h
+++ b/tools/aapt2/xml/XmlDom.h
@@ -35,6 +35,7 @@
 
 class Element;
 class Visitor;
+class ConstVisitor;
 
 // Base class for all XML nodes.
 class Node {
@@ -47,6 +48,7 @@
   std::string comment;
 
   virtual void Accept(Visitor* visitor) = 0;
+  virtual void Accept(ConstVisitor* visitor) const = 0;
 
   using ElementCloneFunc = std::function<void(const Element&, Element*)>;
 
@@ -112,6 +114,7 @@
   std::unique_ptr<Node> Clone(const ElementCloneFunc& el_cloner) const override;
 
   void Accept(Visitor* visitor) override;
+  void Accept(ConstVisitor* visitor) const override;
 };
 
 // A Text (CDATA) XML node. Can not have any children.
@@ -122,6 +125,7 @@
   std::unique_ptr<Node> Clone(const ElementCloneFunc& el_cloner) const override;
 
   void Accept(Visitor* visitor) override;
+  void Accept(ConstVisitor* visitor) const override;
 };
 
 // An XML resource with a source, name, and XML tree.
@@ -180,6 +184,38 @@
   friend class Element;
 };
 
+class ConstVisitor {
+ public:
+  virtual ~ConstVisitor() = default;
+
+  virtual void Visit(const Element* el) {
+    VisitChildren(el);
+  }
+
+  virtual void Visit(const Text* text) {
+  }
+
+ protected:
+  ConstVisitor() = default;
+
+  void VisitChildren(const Element* el) {
+    for (const auto& child : el->children) {
+      child->Accept(this);
+    }
+  }
+
+  virtual void BeforeVisitElement(const Element* el) {
+  }
+
+  virtual void AfterVisitElement(const Element* el) {
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ConstVisitor);
+
+  friend class Element;
+};
+
 // An XML DOM visitor that will record the package name for a namespace prefix.
 class PackageAwareVisitor : public Visitor, public IPackageDeclStack {
  public:
@@ -207,19 +243,19 @@
 namespace internal {
 
 // Base class that overrides the default behaviour and does not descend into child nodes.
-class NodeCastBase : public Visitor {
+class NodeCastBase : public ConstVisitor {
  public:
-  void Visit(Element* el) override {
+  void Visit(const Element* el) override {
   }
-  void Visit(Text* el) override {
+  void Visit(const Text* el) override {
   }
 
  protected:
   NodeCastBase() = default;
 
-  void BeforeVisitElement(Element* el) override {
+  void BeforeVisitElement(const Element* el) override {
   }
-  void AfterVisitElement(Element* el) override {
+  void AfterVisitElement(const Element* el) override {
   }
 
  private:
@@ -233,9 +269,9 @@
 
   NodeCastImpl() = default;
 
-  T* value = nullptr;
+  const T* value = nullptr;
 
-  void Visit(T* v) override {
+  void Visit(const T* v) override {
     value = v;
   }
 
@@ -246,12 +282,17 @@
 }  // namespace internal
 
 template <typename T>
-T* NodeCast(Node* node) {
+const T* NodeCast(const Node* node) {
   internal::NodeCastImpl<T> visitor;
   node->Accept(&visitor);
   return visitor.value;
 }
 
+template <typename T>
+T* NodeCast(Node* node) {
+  return const_cast<T*>(NodeCast<T>(static_cast<const T*>(node)));
+}
+
 }  // namespace xml
 }  // namespace aapt