GVariantRef improvements

A few cleanups and quality-of-life improvements for GVariantRef:

 * Remove iteration functionality from gvariant::Type. It was not
   entirely obvious what iterating should mean for a type string.
   Explicitly getting a string_view to iterate makes it clear that one
   is iterating characters.
 * Update various usages of pointers to instead use std::string_views or
   base::spans in gvariant::Type.
 * Add GVariantFrom() helper function to infer the GVariant type string
   from the passed C++ types. E.g., GVariantFrom("foo") returns a
   GVariantRef<"s">.
 * Add BoxedRef() and FilledMaybeRef() helper functions to more easily
   wrap a reference to an existing value.
 * Introduce ObjectPathCStr and TypeSignatureCStr, which reference an
   unowned string that is known to be a valid object path or type
   signature, respectively. They can reference a constant string (which
   will be verified at compile time to be valid) or an owned ObjectPath
   or TypeSignature object, and are thus useful as function parameters.

Bug: 378932951
Change-Id: Idf625de0caf5b711c477da12d34a0d2a0a2306b8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6334519
Commit-Queue: Erik Jensen <rkjnsn@chromium.org>
Reviewed-by: Lambros Lambrou <lambroslambrou@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1431759}

remoting/host/linux/gvariant_ref.cc

@@ -2,15 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifdef UNSAFE_BUFFERS_BUILD
-// TODO(crbug.com/40285824): Remove this and convert code to safer constructs.
-#pragma allow_unsafe_buffers
-#endif
-
 #include "remoting/host/linux/gvariant_ref.h"
 
 #include <glib.h>
+#include <glibconfig.h>
 
+#include <algorithm>
 #include <cstddef>
 #include <cstdint>
 #include <cstring>
@@ -20,10 +17,10 @@
 #include <utility>
 
 #include "base/check.h"
-#include "base/containers/span.h"
-#include "base/logging.h"
+#include "base/compiler_specific.h"
 #include "base/strings/strcat.h"
 #include "base/types/expected.h"
+#include "remoting/host/linux/gvariant_type.h"
 
 namespace remoting::gvariant {
 
@@ -117,8 +114,8 @@ GVariantBase::~GVariantBase() {
   }
 }
 
-bool GVariantBase::operator==(const GVariantBase& other) const {
-  return g_variant_equal(raw(), other.raw());
+bool operator==(const GVariantBase& lhs, const GVariantBase& rhs) {
+  return g_variant_equal(lhs.raw(), rhs.raw());
 }
 
 // GVariantRef implementation
@@ -151,39 +148,87 @@ GVariantRef<C> GVariantRef<C>::RefSink(GVariant* variant)
 template GVariantRef<Type("*")> GVariantRef<Type("*")>::RefSink(
     GVariant* variant);
 
+// Wrapper implementations
+
+ObjectPathCStr::ObjectPathCStr(const ObjectPath& path LIFETIME_BOUND)
+    : path_(path.c_str()) {}
+
 // static
-base::expected<ObjectPath, std::string> ObjectPath::TryFrom(std::string path) {
-  if (g_variant_is_object_path(path.c_str())) {
-    return base::ok(ObjectPath(std::move(path)));
+base::expected<ObjectPathCStr, std::string> ObjectPathCStr::TryFrom(
+    const char* path LIFETIME_BOUND) {
+  if (g_variant_is_object_path(path)) {
+    return base::ok(ObjectPathCStr(path, Checked()));
   } else {
     return base::unexpected(
         base::StrCat({"String is not a valid object path: ", path}));
   }
 }
 
-// Wrapper implementations
+ObjectPathCStr::ObjectPathCStr(const char* path LIFETIME_BOUND, Checked)
+    : path_(path) {}
+
+ObjectPath::ObjectPath() : path_("/") {}
+
+ObjectPath::ObjectPath(ObjectPathCStr path) : path_(path.c_str()) {}
+
+// static
+base::expected<ObjectPath, std::string> ObjectPath::TryFrom(std::string path) {
+  return ObjectPathCStr::TryFrom(path.c_str()).transform([&](ObjectPathCStr) {
+    return ObjectPath(std::move(path));
+  });
+}
 
 const std::string& ObjectPath::value() const {
   return path_;
 }
 
+const char* ObjectPath::c_str() const {
+  return path_.c_str();
+}
+
 ObjectPath::ObjectPath(std::string path) : path_(path) {}
 
+TypeSignatureCStr::TypeSignatureCStr(
+    const TypeSignature& signature LIFETIME_BOUND)
+    : signature_(signature.c_str()) {}
+
 // static
-base::expected<TypeSignature, std::string> TypeSignature::TryFrom(
-    std::string signature) {
-  if (g_variant_is_signature(signature.c_str())) {
-    return base::ok(TypeSignature(std::move(signature)));
+base::expected<TypeSignatureCStr, std::string> TypeSignatureCStr::TryFrom(
+    const char* signature LIFETIME_BOUND) {
+  if (g_variant_is_signature(signature)) {
+    return base::ok(TypeSignatureCStr(signature, Checked()));
   } else {
     return base::unexpected(
         base::StrCat({"String is not a valid type signature: ", signature}));
   }
 }
 
+TypeSignatureCStr::TypeSignatureCStr(const char* signature LIFETIME_BOUND,
+                                     Checked)
+    : signature_(signature) {}
+
+TypeSignature::TypeSignature() = default;
+
+TypeSignature::TypeSignature(TypeSignatureCStr signature)
+    : signature_(signature.c_str()) {}
+
+// static
+base::expected<TypeSignature, std::string> TypeSignature::TryFrom(
+    std::string signature) {
+  return TypeSignatureCStr::TryFrom(signature.c_str())
+      .transform([&](TypeSignatureCStr) {
+        return TypeSignature(std::move(signature));
+      });
+}
+
 const std::string& TypeSignature::value() const {
   return signature_;
 }
 
+const char* TypeSignature::c_str() const {
+  return signature_.c_str();
+}
+
 TypeSignature::TypeSignature(std::string path) : signature_(path) {}
 
 // Mapping implementations
@@ -278,16 +323,36 @@ double Mapping<double>::Into(const GVariantRef<kType>& variant) {
   return g_variant_get_double(variant.raw());
 }
 
+// Creates a GVariantRef of a string-like type ("s", "o", or "g") from a
+// string_view that has already been verified to be of the correct form for the
+// type.
 template <Type C>
-static GVariantRef<C> CreateStringVariant(std::string_view value) {
+  requires(C == Type("s") || C == Type("o") || C == Type("g"))
+static GVariantRef<C> CreateStringVariantUnchecked(std::string_view value) {
+  // g_variant_new_string() can't be used directly because it requires a null-
+  // terminated string, which |value| might not be. To avoid making two copies
+  // of |value| (one to add a null byte and another to construct the GVariant),
+  // we instead create a new backing buffer ourselves for the GVariant to use.
+
+  // The serialized form of a string GVariant is just the string contents
+  // followed by a null byte.
   char* data = static_cast<char*>(g_malloc(value.size() + 1));
   std::copy(value.begin(), value.end(), data);
-  data[value.size()] = '\0';
+  // SAFETY: We allocate |data| earlier in this function to have a size of
+  // value.size() + 1.
+  UNSAFE_BUFFERS(data[value.size()] = '\0');
+
+  // GVariant holds the buffer as a reference-counted GBytes object. The GBytes
+  // object takes ownership of |data|, and will call g_free on it when the last
+  // reference is dropped.
   GBytes* bytes = g_bytes_new_take(data, value.size() + 1);
+
+  // g_variant_new_from_bytes() creates a new GVariant using |bytes| as its
+  // backing buffer without making a copy.
   GVariantRef<C> variant = GVariantRef<C>::TakeUnchecked(
       g_variant_new_from_bytes(C.gvariant_type(), bytes, true));
 
-  // g_variant_new_from_bytes takes its own ref to bytes.
+  // g_variant_new_from_bytes() takes its own ref to |bytes|.
   g_bytes_unref(bytes);
 
   return variant;
@@ -327,7 +392,7 @@ auto Mapping<std::string_view>::TryFrom(std::string_view value)
     return base::unexpected("String is not valid UTF-8");
   }
 
-  return base::ok(CreateStringVariant<kType>(value));
+  return base::ok(CreateStringVariantUnchecked<kType>(value));
 }
 
 // static
@@ -352,9 +417,15 @@ decltype(std::ignore) Mapping<decltype(std::ignore)>::Into(
   return std::ignore;
 }
 
+// static
+auto Mapping<ObjectPathCStr>::From(const ObjectPathCStr& value)
+    -> GVariantRef<kType> {
+  return CreateStringVariantUnchecked<kType>(value.c_str());
+}
+
 // static
 auto Mapping<ObjectPath>::From(const ObjectPath& value) -> GVariantRef<kType> {
-  return CreateStringVariant<kType>(value.value());
+  return CreateStringVariantUnchecked<kType>(value.value());
 }
 
 // static
@@ -366,10 +437,16 @@ ObjectPath Mapping<ObjectPath>::Into(const GVariantRef<kType>& variant) {
   return ObjectPath(std::string(value, length));
 }
 
+// static
+auto Mapping<TypeSignatureCStr>::From(const TypeSignatureCStr& value)
+    -> GVariantRef<kType> {
+  return CreateStringVariantUnchecked<kType>(value.c_str());
+}
+
 // static
 auto Mapping<TypeSignature>::From(const TypeSignature& value)
     -> GVariantRef<kType> {
-  return CreateStringVariant<kType>(value.value());
+  return CreateStringVariantUnchecked<kType>(value.value());
 }
 
 // static

remoting/host/linux/gvariant_ref.h

@@ -7,6 +7,7 @@
 
 #include <glib-object.h>
 #include <glib.h>
+#include <glibconfig.h>
 
 #include <algorithm>
 #include <array>
@@ -14,9 +15,8 @@
 #include <concepts>
 #include <cstddef>
 #include <cstdint>
-#include <functional>
+#include <cstring>
 #include <iterator>
-#include <locale>
 #include <map>
 #include <optional>
 #include <ranges>
@@ -30,7 +30,7 @@
 
 #include "base/check.h"
 #include "base/check_op.h"
-#include "base/containers/span.h"
+#include "base/compiler_specific.h"
 #include "base/memory/raw_ptr.h"
 #include "base/strings/strcat.h"
 #include "base/strings/string_util.h"
@@ -242,8 +242,25 @@ namespace gvariant {
 //     must be used to check if the array is, in fact, empty.
 // gvariant::ObjectPath (type code "o") - Wrapper around a std::string that
 //     contains a DBus object path.
+// gvariant::ObjectPathCStr ([Try]From() only, type code "o") - An owned object
+//     path that can be used as a function parameter (somewhat analogous to a
+//     string_view) or to hold a compile-time-verified object path constant.
 // gvariant::TypeSignature (type code "g") - Wrapper around a std::string
 //     that contains a DBus type signature.
+// gvariant::TypeSignatureCStr ([Try]From() only, type code "g") - An owned type
+//     signature that can be used as a function parameter (somewhat analogous to
+//     a string_view) or to hold a compile-time-verified type signature
+//     constant.
+//
+// Convenience functions:
+//
+// GVariantFrom(value) - Like GVariantRef<C>::From(), but infers C from value's
+//     type.
+// gvariant::BoxedRef(value) - Returns a gvariant::Boxed that holds a const
+//     reference to value. (Whereas Boxed{value} would hold a copy of value.)
+// gvariant::FilledMaybeRef(value) - Returns a gvariant::FilledMaybe that holds
+//     a const reference to value. (Whereas FilledMaybe{value} would hold a copy
+//     of value.)
 template <Type C = Type("*")>
 class GVariantRef;
 
@@ -313,6 +330,8 @@ class GVariantBase {
   // be definite.
   Type<> GetType() const;
 
+  friend bool operator==(const GVariantBase& lhs, const GVariantBase& rhs);
+
  protected:
   // clang-format off
   static constexpr struct {} kTake;
@@ -329,12 +348,12 @@ class GVariantBase {
   GVariantBase& operator=(GVariantBase&& other);
   ~GVariantBase();
 
-  bool operator==(const GVariantBase& other) const;
-
  private:
   raw_ptr<GVariant> variant_;
 };
 
+bool operator==(const GVariantBase& lhs, const GVariantBase& rhs);
+
 template <Type C>
 class GVariantRef : public GVariantBase {
  public:
@@ -499,14 +518,17 @@ class GVariantRef : public GVariantBase {
   static GVariantRef RefUnchecked(GVariant* variant);
   static GVariantRef RefSinkUnchecked(GVariant* variant);
 
-  template <Type D>
-  bool operator==(const GVariantRef<D>& other) const
-    requires(C.HasCommonTypeWith(D));
-
  private:
   using GVariantBase::GVariantBase;
 };
 
+// Constructs a new GVariantRef from the provided value, inferring the type
+// string.
+template <typename T>
+static GVariantRef<Mapping<T>::kType> GVariantFrom(const T& value) {
+  return GVariantRef<Mapping<T>::kType>::From(value);
+}
+
 // Wrapper types and special types
 
 // Can be used as a nested type in a call to [Try]Into() as a placeholder for a
@@ -519,47 +541,181 @@ struct Ignored {};
 template <typename T>
 struct Boxed {
   T value;
-
-  bool operator==(const Boxed& other) const = default;
 };
 
+template <typename T, typename U>
+bool operator==(const Boxed<T>& lhs, const Boxed<U>& rhs)
+  requires requires(T t, U u) { t == u; }
+{
+  return lhs.value == rhs.value;
+}
+
+// Returns a gvariant::Boxed that holds a const reference to value. (Whereas
+// Boxed{value} would hold a copy of value.) Useful to avoid making an extra
+// copy when constructing a GVariantRef.
+template <typename T>
+Boxed<const T&> BoxedRef(const T& value LIFETIME_BOUND) {
+  return {value};
+}
+
 // Wrapper for a value that should appear inside a maybe, but will always be
 // present.
 template <typename T>
 struct FilledMaybe {
   T value;
-
-  bool operator==(const FilledMaybe& other) const = default;
 };
 
+template <typename T, typename U>
+bool operator==(const FilledMaybe<T>& lhs, const FilledMaybe<U>& rhs)
+  requires requires(T t, U u) { t == u; }
+{
+  return lhs.value == rhs.value;
+}
+
+// Returns a gvariant::FilledMaybe that holds a const reference to value.
+// (Whereas FilledMaybe{value} would hold a copy of value.) Useful to avoid
+// making an extra copy when constructing a GVariantRef.
+template <typename T>
+FilledMaybe<const T&> FilledMaybeRef(const T& value LIFETIME_BOUND) {
+  return {value};
+}
+
 // Represents an empty array of the given type.
 template <Type C>
 struct EmptyArrayOf {};
 
-// Represents a D-Bus object path.
-class ObjectPath {
- public:
-  static base::expected<ObjectPath, std::string> TryFrom(std::string path);
-  const std::string& value() const;
+class ObjectPath;
 
-  bool operator==(const ObjectPath& other) const = default;
+// Holds an unowned pointer to a null-terminated string known to be a valid
+// D-Bus object path.
+class ObjectPathCStr {
+ public:
+  // Constructs from a compile-time constant. The passed string is checked at
+  // compile time to be a valid object path.
+  // Allows implicit construction so string constants can easily be passed to
+  // ObjectPathCStr parameters.
+  // NOLINTNEXTLINE(google-explicit-constructor)
+  consteval ObjectPathCStr(const char* path);
+  // Constructs from an ObjectPath object. The resulting ObjectPathCStr is only
+  // valid as long as the ObjectPath to which it refers.
+  // Allows explicit construction so ObjectPaths can easily be passed to
+  // ObjectPathCStr parameters.
+  // NOLINTNEXTLINE(google-explicit-constructor)
+  ObjectPathCStr(const ObjectPath& path LIFETIME_BOUND);
+
+  // Attempts to construct from an existing C string. Returns an error string
+  // if |path| is not a valid object path.
+  static base::expected<ObjectPathCStr, std::string> TryFrom(
+      const char* path LIFETIME_BOUND);
+
+  // Gets the object path C string.
+  constexpr const char* c_str() const LIFETIME_BOUND;
+
+  friend constexpr bool operator==(const ObjectPathCStr& lhs,
+                                   const ObjectPathCStr& rhs);
 
  private:
-  explicit ObjectPath(std::string);
+  struct Checked {};
+  // Construct from already-checked path. Extra parameter to avoid conflicting
+  // with consteval constructor.
+  ObjectPathCStr(const char* path LIFETIME_BOUND, Checked);
+
+  const char* path_;
+};
+
+constexpr bool operator==(const ObjectPathCStr& lhs, const ObjectPathCStr& rhs);
+
+// Represents an owned D-Bus object path.
+class ObjectPath {
+ public:
+  // Constructs an instance of the root path "/"
+  ObjectPath();
+
+  // Constructs an owned copy of |path|.
+  explicit ObjectPath(ObjectPathCStr path);
+
+  // Attempts to construct from an existing std::string. Returns an error string
+  // if |path| is not a valid object path.
+  static base::expected<ObjectPath, std::string> TryFrom(std::string path);
+
+  // Gets the object path.
+  const std::string& value() const LIFETIME_BOUND;
+
+  // Shorthand for .value().c_str()
+  const char* c_str() const LIFETIME_BOUND;
+
+ private:
+  // Construct from already-checked path.
+  explicit ObjectPath(std::string path);
   std::string path_;
   friend struct Mapping<ObjectPath>;
 };
 
-// Represents a D-Bus type signature.
-class TypeSignature {
- public:
-  static base::expected<TypeSignature, std::string> TryFrom(
-      std::string signature);
-  const std::string& value() const;
+class TypeSignature;
 
-  bool operator==(const TypeSignature& other) const = default;
+// Holds an unowned pointer to a null-terminated string known to be a valid
+// D-Bus type signature.
+class TypeSignatureCStr {
+ public:
+  // Constructs from a compile-time constant. The passed string is checked at
+  // compile time to be a valid type signature.
+  // Allows implicit construction so string constants can easily be passed to
+  // TypeSignatureCStr parameters.
+  // NOLINTNEXTLINE(google-explicit-constructor)
+  consteval TypeSignatureCStr(const char* signature);
+
+  // Constructs from a TypeSignature object. The resulting TypeSignatureCStr is
+  // only valid as long as the TypeSignature to which it refers.
+  // Allows implicit construction so TypeSignatures can easily be passed to
+  // TypeSignatureCStr parameters.
+  // NOLINTNEXTLINE(google-explicit-constructor)
+  TypeSignatureCStr(const TypeSignature& signature LIFETIME_BOUND);
+
+  // Attempts to construct from an existing C string. Returns an error string
+  // if |signature| is not a valid type signature.
+  static base::expected<TypeSignatureCStr, std::string> TryFrom(
+      const char* signature LIFETIME_BOUND);
+
+  // Gets the type signature C string.
+  constexpr const char* c_str() const LIFETIME_BOUND;
+
+  friend constexpr bool operator==(const TypeSignatureCStr& lhs,
+                                   const TypeSignatureCStr& rhs);
 
  private:
+  struct Checked {};
+  // Construct from already-checked path. Extra parameter to avoid conflicting
+  // with consteval constructor.
+  TypeSignatureCStr(const char* signature LIFETIME_BOUND, Checked);
+
+  const char* signature_;
+};
+
+constexpr bool operator==(const TypeSignatureCStr& lhs,
+                          const TypeSignatureCStr& rhs);
+
+// Represents an owned D-Bus type signature.
+class TypeSignature {
+ public:
+  // Constructs an empty type signature.
+  TypeSignature();
+
+  // Constructs an owned copy of |signature|.
+  explicit TypeSignature(TypeSignatureCStr signature);
+
+  // Attempts to construct from an existing std::string. Returns an error string
+  // if |signature| is not a valid type signature.
+  static base::expected<TypeSignature, std::string> TryFrom(
+      std::string signature);
+
+  // Gets the type signature.
+  const std::string& value() const LIFETIME_BOUND;
+
+  // Shorthand for .value().c_str()
+  const char* c_str() const LIFETIME_BOUND;
+
+ private:
+  // Construct from already-checked signature.
   explicit TypeSignature(std::string);
   std::string signature_;
   friend struct Mapping<TypeSignature>;
@@ -883,12 +1039,87 @@ GVariantRef<C> GVariantRef<C>::RefSinkUnchecked(GVariant* variant) {
   return GVariantRef<C>(kRefSink, variant);
 }
 
-template <Type C>
-template <Type D>
-bool GVariantRef<C>::operator==(const GVariantRef<D>& other) const
-  requires(C.HasCommonTypeWith(D))
-{
-  return GVariantBase::operator==(other);
+// Wrapper implementation
+
+consteval ObjectPathCStr::ObjectPathCStr(const char* path) {
+  // SAFETY: Since this constructor is consteval, it can only execute at compile
+  // time. Thus, a read past the end triggered by a missing null terminator will
+  // result in a compile-time error, with no risk at runtime.
+
+  // TODO: bug 400761089 - Remove UNSAFE_BUFFERS annotations when Clang no
+  // longer flags consteval code.
+
+  // CHECKs cannot actually print messages at compile time, but a failed check
+  // will trigger a compiler error pointing to the failed check, allowing the
+  // message to be seen in the source code.
+  CHECK_EQ(*path, '/') << "Path must start with a '/'";
+  const char* prev_char = path;
+  const char* current_char = UNSAFE_BUFFERS(path + 1);
+  while (*current_char != '\0') {
+    CHECK((*current_char >= 'A' && *current_char <= 'Z') ||
+          (*current_char >= 'a' && *current_char <= 'z') ||
+          (*current_char >= '0' && *current_char <= '9') ||
+          *current_char == '_' || *current_char == '/')
+        << "Path contains invalid character";
+    CHECK(*prev_char != '/' || *current_char != '/')
+        << "Two '/' characters may not appear in a row";
+    UNSAFE_BUFFERS(++prev_char);
+    UNSAFE_BUFFERS(++current_char;)
+  }
+  CHECK(*prev_char != '/' || prev_char == path)
+      << "Path may not end in '/' unless the whole path is only a single '/' "
+         "referring to the root path";
+  path_ = path;
+}
+
+constexpr const char* ObjectPathCStr::c_str() const {
+  return path_;
+}
+
+constexpr bool operator==(const ObjectPathCStr& lhs,
+                          const ObjectPathCStr& rhs) {
+  if (std::is_constant_evaluated()) {
+    return std::string_view(lhs.c_str()) == std::string_view(rhs.c_str());
+  } else {
+    return std::strcmp(lhs.c_str(), rhs.c_str()) == 0;
+  }
+}
+
+consteval TypeSignatureCStr::TypeSignatureCStr(const char* signature) {
+  // SAFETY: Since this constructor is consteval, it can only execute at compile
+  // time. Thus, a read past the end triggered by a missing null terminator will
+  // result in a compile-time error, with no risk at runtime.
+
+  // TODO: bug 400761089 - Remove UNSAFE_BUFFERS annotations when Clang no
+  // longer flags consteval code.
+
+  // CHECKs cannot actually print messages at compile time, but a failed check
+  // will trigger a compiler error pointing to the failed check, allowing the
+  // message to be seen in the source code.
+  CHECK(Type("(", signature, ")").IsValid()) << "Not a valid signature";
+  CHECK(Type("(", signature, ")").IsDefinite()) << "Signature must be definite";
+  char prev_char = '\0';
+  for (const char* current_char = signature; *current_char != '\0';
+       UNSAFE_BUFFERS(++current_char)) {
+    CHECK(*current_char != 'm') << "Maybe type not valid in D-Bus signature";
+    CHECK(*current_char != '{' || prev_char == 'a')
+        << "Dict entry not part of a dictionary.";
+    prev_char = *current_char;
+  }
+  signature_ = signature;
+}
+
+constexpr const char* TypeSignatureCStr::c_str() const {
+  return signature_;
+}
+
+constexpr bool operator==(const TypeSignatureCStr& lhs,
+                          const TypeSignatureCStr& rhs) {
+  if (std::is_constant_evaluated()) {
+    return std::string_view(lhs.c_str()) == std::string_view(rhs.c_str());
+  } else {
+    return std::strcmp(lhs.c_str(), rhs.c_str()) == 0;
+  }
 }
 
 // Iterator implementation
@@ -1777,6 +2008,12 @@ struct Mapping<EmptyArrayOf<C>> {
   }
 };
 
+template <>
+struct Mapping<ObjectPathCStr> {
+  static constexpr Type kType{"o"};
+  static GVariantRef<kType> From(const ObjectPathCStr& value);
+};
+
 template <>
 struct Mapping<ObjectPath> {
   static constexpr Type kType{"o"};
@@ -1784,6 +2021,12 @@ struct Mapping<ObjectPath> {
   static ObjectPath Into(const GVariantRef<kType>& variant);
 };
 
+template <>
+struct Mapping<TypeSignatureCStr> {
+  static constexpr Type kType{"g"};
+  static GVariantRef<kType> From(const TypeSignatureCStr& value);
+};
+
 template <>
 struct Mapping<TypeSignature> {
   static constexpr Type kType{"g"};
@@ -1793,6 +2036,7 @@ struct Mapping<TypeSignature> {
 
 }  // namespace gvariant
 
+using gvariant::GVariantFrom;
 using gvariant::GVariantRef;
 
 }  // namespace remoting

remoting/host/linux/gvariant_ref_unittest.cc

@@ -23,6 +23,8 @@
 #include <vector>
 
 #include "base/check.h"
+#include "base/types/expected.h"
+#include "remoting/host/linux/gvariant_type.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace remoting::gvariant {
@@ -109,9 +111,9 @@ TEST(GVariantRefTest, Strings) {
   EXPECT_EQ(base::ok(GVariantParse<"s">("'Hello sailor!'")),
             GVariantRef<>::TryFrom(std::string("Hello sailor!")));
   EXPECT_EQ(GVariantParse<"s">("'Ahoy sailor!'"),
-            GVariantRef<>::From(std::string_view("Ahoy sailor!")));
+            GVariantFrom(std::string_view("Ahoy sailor!")));
   EXPECT_EQ(GVariantParse<"s">("'So long sailor!'"),
-            GVariantRef<>::From("So long sailor!"));
+            GVariantFrom("So long sailor!"));
 
   EXPECT_EQ(
       base::ok(std::string("Welcome back!")),
@@ -130,17 +132,17 @@ TEST(GVariantRefTest, Strings) {
 
 TEST(GVariantRefTest, Optional) {
   EXPECT_EQ(GVariantParse<"mi">("just 16"),
-            GVariantRef<>::From(std::optional(std::int32_t{16})));
+            GVariantRef<"*">::From(std::optional(std::int32_t{16})));
   EXPECT_EQ(GVariantParse<"md">("nothing"),
             GVariantRef<"md">::From(std::optional<double>()));
   EXPECT_EQ(GVariantParse<"ms">("just 'banana'"),
-            GVariantRef<>::From(std::optional("banana")));
+            GVariantFrom(std::optional("banana")));
   EXPECT_EQ(GVariantParse<"ms">("nothing"),
             GVariantRef<"ms">::From(std::optional<std::string_view>()));
   // Indefinite type is okay if the option contains a concrete value.
-  EXPECT_EQ(base::ok(GVariantParse<"mn">("just 539")),
-            GVariantRef<>::TryFrom(
-                std::optional(GVariantRef<>::From(std::int16_t{539}))));
+  EXPECT_EQ(
+      base::ok(GVariantParse<"mn">("just 539")),
+      GVariantRef<>::TryFrom(std::optional(GVariantFrom(std::int16_t{539}))));
 
   EXPECT_EQ(
       std::optional(std::string("apple")),
@@ -175,10 +177,9 @@ TEST(GVariantRefTest, Optional) {
 
 TEST(GVariantRefTest, Vector) {
   EXPECT_EQ(GVariantParse<"as">("[]"),
-            GVariantRef<>::From(std::vector<const char*>()));
-  EXPECT_EQ(
-      GVariantParse<"an">("[5, -2, 45, -367, 97, -8]"),
-      GVariantRef<>::From(std::vector<std::int16_t>{5, -2, 45, -367, 97, -8}));
+            GVariantFrom(std::vector<const char*>()));
+  EXPECT_EQ(GVariantParse<"an">("[5, -2, 45, -367, 97, -8]"),
+            GVariantFrom(std::vector<std::int16_t>{5, -2, 45, -367, 97, -8}));
 
   EXPECT_EQ(base::ok(std::vector<double>()),
             (GVariantParse<"ad", "*">("[]").TryInto<std::vector<double>>()));
@@ -204,11 +205,11 @@ TEST(GVariantRefTest, Vector) {
 
 TEST(GVariantRefTest, Map) {
   EXPECT_EQ(GVariantParse<"a{us}">("{}"),
-            GVariantRef<>::From(std::map<std::uint32_t, const char*>()));
+            GVariantRef<"*">::From(std::map<std::uint32_t, const char*>()));
   EXPECT_EQ(
       GVariantParse<"a{sb}">(
           "{'a': true, 'b': false, 'c': false, 'd': false, 'e': true}"),
-      GVariantRef<>::From(std::map<std::string_view, bool>{
+      GVariantFrom(std::map<std::string_view, bool>{
           {"a", true}, {"b", false}, {"c", false}, {"d", false}, {"e", true}}));
   EXPECT_EQ(GVariantParse<"a{ib}">(
                 "{1: true, 2: false, 3: false, 4: false, 5: true}"),
@@ -245,9 +246,8 @@ TEST(GVariantRefTest, Map) {
 }
 
 TEST(GVariantRefTest, Pair) {
-  EXPECT_EQ(
-      GVariantParse<"{us}">("{31, 'xyzzy'}"),
-      GVariantRef<>::From(std::pair<std::uint32_t, const char*>(31, "xyzzy")));
+  EXPECT_EQ(GVariantParse<"{us}">("{31, 'xyzzy'}"),
+            GVariantFrom(std::pair<std::uint32_t, const char*>(31, "xyzzy")));
   EXPECT_EQ(GVariantParse<"{xt}">("{-64, 64}"),
             GVariantRef<"{xt}">::From(
                 std::pair<std::int64_t, std::uint64_t>(-64, 64)));
@@ -265,11 +265,9 @@ TEST(GVariantRefTest, Pair) {
 }
 
 TEST(GVariantRefTest, Range) {
-  EXPECT_EQ(GVariantParse<"ab">("[]"),
-            GVariantRef<>::From(std::array<bool, 0>()));
-  EXPECT_EQ(
-      GVariantParse<"as">("['1', '2', '3', '4', '5']"),
-      GVariantRef<>::From(std::set<std::string>{"5", "4", "3", "2", "1"}));
+  EXPECT_EQ(GVariantParse<"ab">("[]"), GVariantFrom(std::array<bool, 0>()));
+  EXPECT_EQ(GVariantParse<"as">("['1', '2', '3', '4', '5']"),
+            GVariantFrom(std::set<std::string>{"5", "4", "3", "2", "1"}));
 
   // An indefinite type can't be used with From(), but can be used with
   // TryFrom().
@@ -278,10 +276,10 @@ TEST(GVariantRefTest, Range) {
 }
 
 TEST(GVariantRefTest, Tuple) {
-  EXPECT_EQ(GVariantParse<"()">("()"), GVariantRef<>::From(std::tuple()));
+  EXPECT_EQ(GVariantParse<"()">("()"), GVariantRef<"*">::From(std::tuple()));
   EXPECT_EQ(GVariantParse<"(ybmds)">("(63, true, 3.0, 'Hello!')"),
-            GVariantRef<>::From(std::tuple(
-                std::uint8_t{63}, true, std::optional(double{3.0}), "Hello!")));
+            GVariantFrom(std::tuple(std::uint8_t{63}, true,
+                                    std::optional(double{3.0}), "Hello!")));
 
   EXPECT_EQ(base::ok(std::tuple()),
             (GVariantParse<"()", "*">("()").TryInto<std::tuple<>>()));
@@ -312,11 +310,11 @@ TEST(GVariantRefTest, Variant) {
                              GVariantRef<"?">>;
 
   EXPECT_EQ(GVariantParse<"s">("'asparagus'"),
-            GVariantRef<>::From(Strings(std::in_place_index<0>, "asparagus")));
+            GVariantFrom(Strings(std::in_place_index<0>, "asparagus")));
   EXPECT_EQ(GVariantParse<"s">("'broccoli'"),
-            GVariantRef<>::From(Strings(std::in_place_index<1>, "broccoli")));
+            GVariantFrom(Strings(std::in_place_index<1>, "broccoli")));
   EXPECT_EQ(GVariantParse<"s">("'carrot'"),
-            GVariantRef<>::From(Strings(std::in_place_index<2>, "carrot")));
+            GVariantFrom(Strings(std::in_place_index<2>, "carrot")));
 
   EXPECT_EQ(GVariantParse<"ad">("[2.0, 3.5]"),
             GVariantRef<"a?">::From(
@@ -329,12 +327,11 @@ TEST(GVariantRefTest, Variant) {
                 BasicArrays(std::in_place_index<2>, std::vector{true, false})));
 
   EXPECT_EQ(GVariantParse<"y">("6"),
-            GVariantRef<>::From(Mixed(std::in_place_index<0>, 6)));
+            GVariantFrom(Mixed(std::in_place_index<0>, 6)));
   EXPECT_EQ(GVariantParse<"d">("8.25"),
-            GVariantRef<>::From(Mixed(std::in_place_index<1>, 8.25)));
-  EXPECT_EQ(
-      GVariantParse<"ms">("just 'delicata squash'"),
-      GVariantRef<>::From(Mixed(std::in_place_index<2>, "delicata squash")));
+            GVariantFrom(Mixed(std::in_place_index<1>, 8.25)));
+  EXPECT_EQ(GVariantParse<"ms">("just 'delicata squash'"),
+            GVariantFrom(Mixed(std::in_place_index<2>, "delicata squash")));
 
   EXPECT_EQ(Strings(std::in_place_index<1>, "eggplant"),
             GVariantParse<"s">("'eggplant'").Into<Strings>());
@@ -348,7 +345,7 @@ TEST(GVariantRefTest, Variant) {
             GVariantParse<"d">("9.75").Into<GVariantRef<"?">>().Into<Mixed>());
   EXPECT_EQ(
       Mixed(std::in_place_index<3>, GVariantRef<"?">::From(std::int32_t{93})),
-      GVariantParse<"i">("93").Into<GVariantRef<"?">>().Into<Mixed>());
+      GVariantParse<"i">("93").Into<Mixed>());
 
   // Doesn't match type string.
   EXPECT_FALSE((GVariantParse<"i", "*">("16").TryInto<Strings>().has_value()));
@@ -372,9 +369,10 @@ TEST(GVariantRefTest, Variant) {
 }
 
 TEST(GVariantRefTest, NestedGVariantRef) {
-  EXPECT_EQ(base::ok(GVariantParse<"(sb)">("('X-387', true)")),
-            GVariantRef<"(s?)">::TryFrom(std::tuple(
-                GVariantRef<>::From("X-387"), GVariantRef<>::From(true))));
+  EXPECT_EQ(
+      base::ok(GVariantParse<"(sb)">("('X-387', true)")),
+      GVariantRef<"(s?)">::TryFrom(std::tuple(GVariantRef<"*">::From("X-387"),
+                                              GVariantRef<"*">::From(true))));
   EXPECT_EQ(GVariantParse<"i">("6"),
             GVariantRef<"?">::From(GVariantRef<"i">::From(std::int32_t{6})));
 
@@ -417,12 +415,15 @@ TEST(GVariantRefTest, Ignored) {
 }
 
 TEST(GVariantRefTest, Boxed) {
-  EXPECT_EQ(GVariantParse<"v">("<'beep'>"), GVariantRef<>::From(Boxed{"beep"}));
+  EXPECT_EQ(GVariantParse<"v">("<'beep'>"), GVariantFrom(Boxed{"beep"}));
+  std::vector<std::string> value = {"chirp", "whistle"};
+  EXPECT_EQ(GVariantParse<"v">("<['chirp', 'whistle']>"),
+            GVariantFrom(BoxedRef(value)));
 
   EXPECT_EQ((Boxed<std::variant<std::uint16_t, std::string>>{"boop"}),
             (GVariantParse<"v">("<'boop'>")
                  .TryInto<Boxed<std::variant<std::uint16_t, std::string>>>()));
-  EXPECT_EQ(Boxed{GVariantRef<>::From(std::int64_t{-204})},
+  EXPECT_EQ(Boxed{GVariantFrom(std::int64_t{-204})},
             GVariantParse<"v">("<int64 -204>").Into<Boxed<GVariantRef<>>>());
 
   EXPECT_FALSE((GVariantParse<"i", "*">("-27")
@@ -438,12 +439,15 @@ TEST(GVariantRefTest, Boxed) {
 
 TEST(GVariantRefTest, FilledMaybe) {
   EXPECT_EQ(GVariantParse<"ms">("just 'alpaca'"),
-            GVariantRef<>::From(FilledMaybe{"alpaca"}));
+            GVariantFrom(FilledMaybe{"alpaca"}));
   // Since the value is always guaranteed to be present, indefinite types (whose
   // actual type is only known at runtime) can be used with From.
-  EXPECT_EQ(GVariantParse<"mx">("-362"),
-            GVariantRef<>::From(
-                FilledMaybe{GVariantRef<>::From(std::int64_t{-362})}));
+  EXPECT_EQ(
+      GVariantParse<"mx">("-362"),
+      GVariantFrom(FilledMaybe{GVariantRef<"*">::From(std::int64_t{-362})}));
+  std::vector<std::uint16_t> value = {1, 2, 3};
+  EXPECT_EQ(GVariantParse<"maq">("just [1, 2, 3]"),
+            GVariantFrom(FilledMaybeRef(value)));
 
   EXPECT_EQ(
       base::ok(FilledMaybe{true}),
@@ -462,7 +466,7 @@ TEST(GVariantRefTest, FilledMaybe) {
 
 TEST(GVariantRefTest, EmptyArrayOf) {
   EXPECT_EQ(GVariantParse<"ai">("[]"),
-            GVariantRef<>::From(EmptyArrayOf<"i">()));
+            GVariantRef<"ai">::From(EmptyArrayOf<"i">()));
 
   EXPECT_TRUE((
       GVariantParse<"ai", "*">("[]").TryInto<EmptyArrayOf<"i">>().has_value()));
@@ -480,12 +484,16 @@ TEST(GVariantRefTest, ObjectPath) {
   auto path = ObjectPath::TryFrom("/valid/path");
   ASSERT_TRUE(path.has_value());
 
+  EXPECT_EQ(GVariantParse<"o">("'/valid/path'"), GVariantFrom(path.value()));
   EXPECT_EQ(GVariantParse<"o">("'/valid/path'"),
-            GVariantRef<>::From(path.value()));
+            GVariantFrom(ObjectPathCStr(path.value())));
 
   EXPECT_EQ(path.value(),
             GVariantParse<"o">("'/valid/path'").Into<ObjectPath>());
 
+  EXPECT_EQ(GVariantParse<"o">("'/path/constant'"),
+            GVariantFrom(ObjectPathCStr("/path/constant")));
+
   EXPECT_FALSE((GVariantParse<"s", "*">("'/valid/path'")
                     .TryInto<ObjectPath>()
                     .has_value()));
@@ -497,12 +505,16 @@ TEST(GVariantRefTest, TypeSignature) {
   auto signature = TypeSignature::TryFrom("goodsig");
   ASSERT_TRUE(signature.has_value());
 
+  EXPECT_EQ(GVariantParse<"g">("'goodsig'"), GVariantFrom(signature.value()));
   EXPECT_EQ(GVariantParse<"g">("'goodsig'"),
-            GVariantRef<>::From(signature.value()));
+            GVariantFrom(TypeSignatureCStr(signature.value())));
 
   EXPECT_EQ(signature.value(),
             GVariantParse<"g">("'goodsig'").Into<TypeSignature>());
 
+  EXPECT_EQ(GVariantParse<"g">("'gonstsig'"),
+            GVariantFrom(TypeSignatureCStr("gonstsig")));
+
   EXPECT_FALSE((GVariantParse<"s", "*">("'goodsig'")
                     .TryInto<TypeSignature>()
                     .has_value()));
@@ -514,9 +526,8 @@ TEST(GVariantRefTest, FromReferences) {
   std::vector<std::uint8_t> data{1, 2, 3, 4, 5};
   Boxed<std::vector<std::uint8_t>&> boxed_data{data};
 
-  EXPECT_EQ(
-      GVariantParse<"(isv)">("(57, 'check', <[byte 1, 2, 3, 4, 5]>)"),
-      GVariantRef<>::From(std::forward_as_tuple(int32, string, boxed_data)));
+  EXPECT_EQ(GVariantParse<"(isv)">("(57, 'check', <[byte 1, 2, 3, 4, 5]>)"),
+            GVariantFrom(std::forward_as_tuple(int32, string, boxed_data)));
 }
 
 TEST(GVariantRefTest, Destructure) {
@@ -541,7 +552,7 @@ TEST(GVariantRefTest, Destructure) {
   EXPECT_EQ(6, f);
   EXPECT_EQ(7u, g);
   EXPECT_EQ(8u, h);
-  EXPECT_EQ(GVariantRef<>::From(std::optional<std::uint8_t>(9)), i);
+  EXPECT_EQ(GVariantFrom(std::optional<std::uint8_t>(9)), i);
 }
 
 TEST(GVariantRefTest, TryDestructure) {
@@ -628,15 +639,15 @@ TEST(GVariantRefTest, Lookup) {
   auto vardict = GVariantParse<"a{sv}">("{'a': <true>, 'b': <int32 5>}");
   auto dict2 = GVariantParse<"a{ib}">("{1: false, 3: true, 5: true}");
 
-  EXPECT_EQ(GVariantRef<>::From(Boxed{true}), vardict.LookUp("a"));
-  EXPECT_EQ(GVariantRef<>::From(Boxed<std::int32_t>{5}), vardict.LookUp("b"));
+  EXPECT_EQ(GVariantFrom(Boxed{true}), vardict.LookUp("a"));
+  EXPECT_EQ(GVariantFrom(Boxed<std::int32_t>{5}), vardict.LookUp("b"));
   EXPECT_EQ(std::nullopt, vardict.LookUp("c"));
 
-  EXPECT_EQ(GVariantRef<>::From(false), dict2.LookUp(std::int32_t{1}));
+  EXPECT_EQ(GVariantFrom(false), dict2.LookUp(std::int32_t{1}));
   EXPECT_EQ(std::nullopt, dict2.LookUp(std::int32_t{2}));
-  EXPECT_EQ(GVariantRef<>::From(true), dict2.LookUp(std::int32_t{3}));
+  EXPECT_EQ(GVariantFrom(true), dict2.LookUp(std::int32_t{3}));
   EXPECT_EQ(std::nullopt, dict2.LookUp(std::int32_t{4}));
-  EXPECT_EQ(GVariantRef<>::From(true), dict2.LookUp(std::int32_t{5}));
+  EXPECT_EQ(GVariantFrom(true), dict2.LookUp(std::int32_t{5}));
 }
 
 }  // namespace remoting::gvariant

remoting/host/linux/gvariant_type.cc

@@ -8,7 +8,6 @@
 
 #include <cstddef>
 #include <string>
-#include <string_view>
 #include <utility>
 
 namespace remoting::gvariant {

remoting/host/linux/gvariant_type.h

@@ -4,14 +4,8 @@
 #ifndef REMOTING_HOST_LINUX_GVARIANT_TYPE_H_
 #define REMOTING_HOST_LINUX_GVARIANT_TYPE_H_
 
-#ifdef UNSAFE_BUFFERS_BUILD
-// TODO(crbug.com/40285824): Remove this and convert code to safer constructs.
-#pragma allow_unsafe_buffers
-#endif
-
 #include <glib.h>
 
-#include <algorithm>
 #include <array>
 #include <concepts>
 #include <cstddef>
@@ -27,6 +21,7 @@
 #include <utility>
 #include <vector>
 
+#include "base/check.h"
 #include "base/containers/span.h"
 #include "base/strings/strcat.h"
 
@@ -58,11 +53,6 @@ class TypeBase {
   explicit operator const GVariantType*() const;
   explicit constexpr operator Type<>() const;
 
-  // Iteration
-  constexpr const char* begin() const;
-  constexpr const char* end() const;
-  constexpr std::size_t size() const;
-
   // Comparison
   constexpr bool operator==(const TypeBase& other) const;
   constexpr bool IsSubtypeOf(const TypeBase& supertype) const;
@@ -130,13 +120,15 @@ class TypeBase {
 
   // Helpers
 
-  // string must be a view within a null-terminated string.
+  // Verifies that |string| is a valid type string representing a single,
+  // complete type.
   static constexpr bool Validate(std::string_view string);
   static constexpr bool IsBasicType(char code);
-  // Given the pointer to the start of a type in a type string, returns a
-  // pointer to the position immediately following it. E.g., given "a{sv}i",
-  // returns a pointer to "i".
-  static constexpr const char* SkipType(const char* start);
+  // Given a view within a type string, returns the length of the first single
+  // complete type. E.g. given "a{sv}i)", returns 5 (the length of "a{sv}"). If
+  // |view| does not begin with a valid complete type, the returned length is
+  // unspecified, but will be no greater than view.length().
+  static constexpr size_t TypeLength(std::string_view view);
 
   // Converts a dynamic Type<> to a fixed Type<N>. get_dynamic must provide a
   // constexpr operator() that returns the Type<>. It will be called exactly
@@ -169,11 +161,14 @@ class Type final : public TypeBase {
   // GVariantRef<gvariant::Type("s")>.
   // NOLINTNEXTLINE(google-explicit-constructor)
   constexpr Type(const char (&string_lit)[N + 1])
-      : Type(string_lit, std::integral_constant<std::size_t, N>()) {}
+      : Type(base::span(string_lit).template first<N>()) {
+    CHECK(string_lit[N] == '\0')
+        << "This constructor expects a null-terminated string";
+  }
 
   // Construct from possibly non-null-terminated string of the proper length.
-  constexpr Type(const char* string, std::integral_constant<std::size_t, N>) {
-    std::copy(string, string + N, type_string_.begin());
+  explicit constexpr Type(base::span<const char, N> string) {
+    base::span(type_string_).template first<N>().copy_from(string);
     type_string_[N] = '\0';
   }
 
@@ -188,15 +183,15 @@ class Type final : public TypeBase {
     requires(N == (... + decltype(gvariant::Type(
                              std::forward<Pieces>(pieces)))::fixed_size))
   {
-    auto position = type_string_.begin();
+    base::span<char> remaining = type_string_;
     // "Loop" through the passed values by invoking a lambda for each argument.
     (
         [&]<std::size_t M>(const Type<M>& type) {
-          std::copy(type.begin(), type.end(), position);
-          position += type.size();
+          remaining.take_first(type.string_view().size())
+              .copy_from(type.string_view());
         }(gvariant::Type(std::forward<Pieces>(pieces))),
         ...);
-    *position = '\0';
+    remaining.front() = '\0';
   }
 
   // Copyable
@@ -293,8 +288,8 @@ constexpr const char* TypeBase::c_string() const {
 }
 
 constexpr std::string_view TypeBase::string_view() const {
-  auto contents = this->contents();
-  return std::string_view(contents.first, contents.second);
+  auto [string, length] = this->contents();
+  return std::string_view(string, length);
 }
 
 constexpr TypeBase::operator const char*() const {
@@ -309,19 +304,6 @@ constexpr TypeBase::operator Type<>() const {
   return Type<>(string_view());
 }
 
-constexpr const char* TypeBase::begin() const {
-  return contents().first;
-}
-
-constexpr const char* TypeBase::end() const {
-  auto contents = this->contents();
-  return contents.first + contents.second;
-}
-
-constexpr std::size_t TypeBase::size() const {
-  return contents().second;
-}
-
 constexpr bool TypeBase::operator==(const TypeBase& other) const {
   return string_view() == other.string_view();
 }
@@ -331,23 +313,25 @@ constexpr bool TypeBase::IsSubtypeOf(const TypeBase& supertype) const {
     return false;
   }
 
-  const char* sub_iter = c_string();
-  const char* super_iter = supertype.c_string();
+  std::string_view sub_view = string_view();
+  std::string_view super_view = supertype.string_view();
 
-  while (*super_iter != '\0') {
-    if (*super_iter == *sub_iter ||
-        (*super_iter == '?' && IsBasicType(*sub_iter))) {
-      ++sub_iter, ++super_iter;
-    } else if (*sub_iter == ')' || *sub_iter == '}' || *sub_iter == '\0') {
+  while (!sub_view.empty() && !super_view.empty()) {
+    if (super_view.front() == sub_view.front() ||
+        (super_view.front() == '?' && IsBasicType(sub_view.front()))) {
+      sub_view.remove_prefix(1);
+      super_view.remove_prefix(1);
+    } else if (sub_view.front() == ')' || sub_view.front() == '}') {
       return false;
-    } else if (*super_iter == '*' || (*super_iter == 'r' && *sub_iter == '(')) {
-      sub_iter = SkipType(sub_iter);
-      ++super_iter;
+    } else if (super_view.front() == '*' ||
+               (super_view.front() == 'r' && sub_view.front() == '(')) {
+      sub_view.remove_prefix(TypeLength(sub_view));
+      super_view.remove_prefix(1);
     } else {
       return false;
     }
   }
-  return *sub_iter == '\0';
+  return sub_view.empty() && super_view.empty();
 }
 
 constexpr bool TypeBase::HasCommonTypeWith(const TypeBase& other) const {
@@ -356,27 +340,31 @@ constexpr bool TypeBase::HasCommonTypeWith(const TypeBase& other) const {
   }
 
   // This is like IsSubtypeOf, but symmetrical.
-  const char* iter1 = c_string();
-  const char* iter2 = other.c_string();
+  std::string_view view1 = string_view();
+  std::string_view view2 = other.string_view();
 
-  while (*iter1 != '\0' && *iter2 != '\0') {
-    if (*iter1 == *iter2 || (*iter1 == '?' && IsBasicType(*iter2)) ||
-        (*iter2 == '?' && IsBasicType(*iter1))) {
-      ++iter1, ++iter2;
-    } else if (*iter1 == ')' || *iter1 == '}' || *iter2 == ')' ||
-               *iter2 == '}') {
+  while (!view1.empty() && !view2.empty()) {
+    if (view1.front() == view2.front() ||
+        (view1.front() == '?' && IsBasicType(view2.front())) ||
+        (view2.front() == '?' && IsBasicType(view1.front()))) {
+      view1.remove_prefix(1);
+      view2.remove_prefix(1);
+    } else if (view1.front() == ')' || view1.front() == '}' ||
+               view2.front() == ')' || view2.front() == '}') {
       return false;
-    } else if (*iter1 == '*' || (*iter1 == 'r' && *iter2 == '(')) {
-      iter2 = SkipType(iter2);
-      ++iter1;
-    } else if (*iter2 == '*' || (*iter2 == 'r' && *iter1 == '(')) {
-      iter1 = SkipType(iter1);
-      ++iter2;
+    } else if (view1.front() == '*' ||
+               (view1.front() == 'r' && view2.front() == '(')) {
+      view1.remove_prefix(1);
+      view2.remove_prefix(TypeLength(view2));
+    } else if (view2.front() == '*' ||
+               (view2.front() == 'r' && view1.front() == '(')) {
+      view1.remove_prefix(TypeLength(view1));
+      view2.remove_prefix(1);
     } else {
       return false;
     }
   }
-  return *iter1 == *iter2;
+  return view1.empty() && view2.empty();
 }
 
 constexpr bool TypeBase::IsValid() const {
@@ -387,7 +375,7 @@ constexpr bool TypeBase::IsDefinite() const {
   if (!IsValid()) {
     return false;
   }
-  for (char c : *this) {
+  for (char c : this->string_view()) {
     // These characters indicate, respectively, any type, any basic type, and
     // any tuple, and the presence of any of them makes a type indefinite.
     if (c == '*' || c == '?' || c == 'r') {
@@ -398,19 +386,19 @@ constexpr bool TypeBase::IsDefinite() const {
 }
 
 constexpr bool TypeBase::IsBasic() const {
-  if (size() != 1) {
+  if (string_view().size() != 1) {
     return false;
   }
 
-  return IsBasicType(*begin());
+  return IsBasicType(string_view().front());
 }
 
 constexpr bool TypeBase::IsStringType() const {
-  if (size() != 1) {
+  if (string_view().size() != 1) {
     return false;
   }
 
-  switch (*begin()) {
+  switch (string_view().front()) {
     case 's':
     case 'o':
     case 'g':
@@ -425,7 +413,7 @@ constexpr bool TypeBase::IsContainer() const {
     return false;
   }
 
-  switch (*begin()) {
+  switch (string_view().front()) {
     case 'v':
     case 'a':
     case 'm':
@@ -443,7 +431,7 @@ constexpr bool TypeBase::IsFixedSizeContainer() const {
     return false;
   }
 
-  switch (*begin()) {
+  switch (string_view().front()) {
     case 'v':
     case '(':
     case '{':
@@ -458,16 +446,17 @@ constexpr std::optional<std::vector<Type<>>> TypeBase::Unpack() const {
     return std::nullopt;
   }
 
+  std::string_view view = string_view();
   std::vector<Type<>> result;
 
-  if (string_view() == "v") {
+  if (view == "v") {
     result.emplace_back("*");
   } else {
-    const char* position = begin() + 1;  // Skip opening '(' or '{'
-    while (*position != ')' && *position != '}') {
-      const char* next = SkipType(position);
-      result.emplace_back(std::string_view(position, next));
-      position = next;
+    view.remove_prefix(1);  // Skip opening '(' or '{'
+    while (view.front() != ')' && view.front() != '}') {
+      std::size_t type_length = TypeLength(view);
+      result.emplace_back(view.substr(0, type_length));
+      view.remove_prefix(type_length);
     }
   }
   return result;
@@ -490,13 +479,15 @@ constexpr Type<> TypeBase::CommonSuperTypeWith(const TypeBase& other) const {
     return Type<>(*this);
   } else if (IsBasic() && other.IsBasic()) {
     return Type<>("?");
-  } else if ((*begin() == 'r' && *other.begin() == '(') ||
-             (*begin() == '(' && *other.begin() == 'r')) {
+  } else if ((string_view().front() == 'r' &&
+              other.string_view().front() == '(') ||
+             (string_view().front() == '(' &&
+              other.string_view().front() == 'r')) {
     return Type<>("r");
-  } else if (*begin() == *other.begin()) {
+  } else if (string_view().front() == other.string_view().front()) {
     // Containers of the same type. (Only way first char can be equal but not
     // the rest.)
-    const char container_char = *begin();
+    const char container_char = string_view().front();
     if (container_char == 'a' || container_char == 'm') {
       return Type<>(std::string_view(&container_char, 1),
                     ContainedType().value().CommonSuperTypeWith(
@@ -507,17 +498,20 @@ constexpr Type<> TypeBase::CommonSuperTypeWith(const TypeBase& other) const {
     std::vector<Type<>> other_types = other.Unpack().value();
 
     if (my_types.size() != other_types.size()) {
+      // Dict entries always have two entries, so they must be tuples.
       return Type<>("r");
     }
 
     std::string super_types;
-    super_types.reserve(size() - 2);
+    // A supertype is no longer than either subtype.
+    super_types.reserve(string_view().size());
+    super_types += container_char;
     for (std::size_t i = 0; i < my_types.size(); ++i) {
       super_types +=
           my_types[i].CommonSuperTypeWith(other_types[i]).string_view();
     }
-    return Type<>(std::string_view(&container_char, 1), super_types,
-                  container_char == '(' ? ")" : "}");
+    super_types += container_char == '(' ? ")" : "}";
+    return Type<>(super_types);
   } else {
     return Type<>("*");
   }
@@ -556,8 +550,8 @@ constexpr std::optional<Type<>> TypeBase::ContainedType() const {
   }
   if (*this == Type("v") || *this == Type("r")) {
     return Type<>("*");
-  } else if (*begin() == 'a' || *begin() == 'm') {
-    return Type<>(begin() + 1);
+  } else if (string_view().front() == 'a' || string_view().front() == 'm') {
+    return Type<>(string_view().substr(1));
   } else {
     // Tuple or dict entry
     std::vector<Type<>> inner_types = Unpack().value();
@@ -612,9 +606,8 @@ constexpr bool TypeBase::Validate(std::string_view string) {
       }
       std::string_view remaining = string.substr(1, string.size() - 2);
       while (remaining.size() != 0) {
-        std::size_t type_length = SkipType(remaining.data()) - remaining.data();
-        if (type_length > remaining.size() ||
-            !Validate(remaining.substr(0, type_length))) {
+        std::size_t type_length = TypeLength(remaining);
+        if (!Validate(remaining.substr(0, type_length))) {
           return false;
         }
         remaining.remove_prefix(type_length);
@@ -651,28 +644,23 @@ constexpr bool TypeBase::IsBasicType(char code) {
 }
 
 // static
-constexpr const char* TypeBase::SkipType(const char* start) {
-  // SkipType is used by Validate, and thus shouldn't assume start points to a
-  // valid type string. In the event of an invalid type string, SkipType needn't
-  // do anything particularly sensible, but it should ensure that (1) it never
-  // skips past the terminating null byte and (2) if start isn't already
-  // pointing to a null byte, it skips at least one character (to avoid infinite
-  // loops).
-  while (*start == 'a' || *start == 'm') {
-    ++start;
+constexpr size_t TypeBase::TypeLength(std::string_view view) {
+  std::size_t initial_length = view.length();
+  while (!view.empty() && (view.front() == 'a' || view.front() == 'm')) {
+    view.remove_prefix(1);
   }
   std::size_t depth = 0;
   do {
-    if (*start == '\0') {
+    if (view.empty()) {
       break;
-    } else if (*start == '(' || *start == '{') {
+    } else if (view.front() == '(' || view.front() == '{') {
       ++depth;
-    } else if (*start == ')' || *start == '}') {
+    } else if (view.front() == ')' || view.front() == '}') {
       --depth;
     }
-    ++start;
+    view.remove_prefix(1);
   } while (depth != 0);
-  return start;
+  return initial_length - view.length();
 }
 
 // static
@@ -700,13 +688,15 @@ consteval /* Type<N> */ auto TypeBase::ToFixed(Callable get_dynamic) {
 
   constexpr IntermediateResult intermediate_result = [](Type<> type) {
     IntermediateResult intermediate_result{};
-    intermediate_result.size = type.size();
-    std::copy(type.begin(), type.end(), intermediate_result.data.begin());
+    intermediate_result.size = type.string_view().size();
+    base::span(intermediate_result.data)
+        .first(type.string_view().size())
+        .copy_from(type.string_view());
     return intermediate_result;
   }(get_dynamic());
 
-  return Type(intermediate_result.data.data(),
-              std::integral_constant<std::size_t, intermediate_result.size>());
+  return Type(base::span(intermediate_result.data)
+                  .template first<intermediate_result.size>());
 #endif
 }
 
@@ -714,8 +704,8 @@ consteval /* Type<N> */ auto TypeBase::ToFixed(Callable get_dynamic) {
 template <typename Callable>
 consteval /* std::tuple<Type<N>...> */ auto TypeBase::ToFixedTuple(
     Callable get_dynamic_vector) {
-  // Like with ToFixed(), this function copies the result into an almost-sure
-  // to-be-big-enough fixed-size arrays that are allowed to be stored in a
+  // Like with ToFixed(), this function copies the result into almost-sure-to-
+  // be-big-enough fixed-size arrays that are allowed to be stored in a
   // constexpr variable to avoid calling get_dynamic_vector() multiple times.
 
   struct IntermediateResult {
@@ -729,25 +719,20 @@ consteval /* std::tuple<Type<N>...> */ auto TypeBase::ToFixedTuple(
       [](std::vector<Type<>> types) {
         IntermediateResult intermediate_result{};
         intermediate_result.count = types.size();
-        std::size_t offset = 0;
+        base::span<char> data_span = intermediate_result.data;
         for (std::size_t i = 0; i < types.size(); ++i) {
-          intermediate_result.sizes[i] = types[i].size();
-          std::copy(types[i].begin(), types[i].end(),
-                    intermediate_result.data.begin() + offset);
-          offset += types[i].size();
+          intermediate_result.sizes[i] = types[i].string_view().size();
+          data_span.take_first(types[i].string_view().size())
+              .copy_from(types[i].string_view());
         }
         return intermediate_result;
       }(get_dynamic_vector());
 
   return [&]<std::size_t... Is>(std::index_sequence<Is...>) {
-    std::size_t offset = 0;
+    base::span<const char> data_span = intermediate_result.data;
     // Uses brace initialization to guarantee in-order evaluation of arguments.
-    return std::tuple{[&]() {
-      const char* data = intermediate_result.data.data() + offset;
-      constexpr std::size_t size = intermediate_result.sizes[Is];
-      offset += size;
-      return Type(data, std::integral_constant<std::size_t, size>());
-    }()...};
+    return std::tuple{
+        Type(data_span.take_first<intermediate_result.sizes[Is]>())...};
   }(std::make_index_sequence<intermediate_result.count>());
 }
 

remoting/host/linux/gvariant_type_unittest.cc

@@ -4,10 +4,6 @@
 
 #include "remoting/host/linux/gvariant_type.h"
 
-#include <iterator>
-#include <vector>
-
-#include "remoting/host/linux/gvariant_ref.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace remoting::gvariant {