[go: nahoru, domu]

[Code Health] Add base::Value::List::rbegin()/rend()

This change adds reverse iterators to base::Value::List to enable more
migrations from std::vector<base::Value> to base::Value::List.

Also updates one call site from using a span now that reverse iterators
are available.

Bug: 1187001
Change-Id: I220857d4bb5dd531ef818ecc5e895ea31a5965ca
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4539691
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Oksana Zhuravlova <oksamyt@chromium.org>
Reviewed-by: Wes Okuhara <wesokuhara@google.com>
Cr-Commit-Position: refs/heads/main@{#1147865}
diff --git a/base/values.cc b/base/values.cc
index 9121574..6024d315 100644
--- a/base/values.cc
+++ b/base/values.cc
@@ -909,6 +909,30 @@
                         base::to_address(storage_.cend()));
 }
 
+// TODO(crbug.com/1446739): Implement reverse iterators in
+// CheckedContiguousIterator and use them here.
+std::vector<Value>::reverse_iterator Value::List::rend() {
+  return storage_.rend();
+}
+
+// TODO(crbug.com/1446739): Implement reverse iterators in
+// CheckedContiguousIterator and use them here.
+std::vector<Value>::const_reverse_iterator Value::List::rend() const {
+  return storage_.rend();
+}
+
+// TODO(crbug.com/1446739): Implement reverse iterators in
+// CheckedContiguousIterator and use them here.
+std::vector<Value>::reverse_iterator Value::List::rbegin() {
+  return storage_.rbegin();
+}
+
+// TODO(crbug.com/1446739): Implement reverse iterators in
+// CheckedContiguousIterator and use them here.
+std::vector<Value>::const_reverse_iterator Value::List::rbegin() const {
+  return storage_.rbegin();
+}
+
 const Value& Value::List::front() const {
   CHECK(!storage_.empty());
   return storage_.front();
diff --git a/base/values.h b/base/values.h
index 930ff25..5d62e8c 100644
--- a/base/values.h
+++ b/base/values.h
@@ -140,6 +140,8 @@
 //       `front()`, `back()`, `reserve()`, `operator[]`, `clear()`, `erase()`:
 //       Identical to the STL container equivalents, with additional safety
 //       checks, e.g. `operator[]` will `CHECK()` if the index is out of range.
+// - `rbegin()` and `rend()` are also supported, but there are no safety checks
+// (see crbug.com/1446739).
 // - `Clone()`: Create a deep copy.
 // - `Append()`: Append a value to the end of the list. Accepts `Value` or any
 //       of the subtypes that `Value` can hold.
@@ -628,6 +630,15 @@
     const_iterator end() const;
     const_iterator cend() const;
 
+    // Returns a reverse iterator preceding the first value in this list. May
+    // not be dereferenced.
+    std::vector<Value>::reverse_iterator rend();
+    std::vector<Value>::const_reverse_iterator rend() const;
+
+    // Returns a reverse iterator to the last value in this list.
+    std::vector<Value>::reverse_iterator rbegin();
+    std::vector<Value>::const_reverse_iterator rbegin() const;
+
     // Returns a reference to the first value in the container. Fails with
     // `CHECK()` if the list is empty.
     const Value& front() const;
diff --git a/base/values_unittest.cc b/base/values_unittest.cc
index 0b99a937..6d1cf6f 100644
--- a/base/values_unittest.cc
+++ b/base/values_unittest.cc
@@ -534,6 +534,29 @@
   EXPECT_EQ(*iter, "Hello world!");
 }
 
+TEST(ValuesTest, ReverseIter) {
+  Value::List list;
+  const Value::List& const_list = list;
+
+  list.Append(Value(true));
+  list.Append(Value(123));
+  list.Append(Value("Hello world!"));
+
+  auto iter = list.rbegin();
+  EXPECT_TRUE(const_list.rbegin() == iter);
+  EXPECT_EQ(*iter, "Hello world!");
+
+  ++iter;
+  EXPECT_EQ(*iter, 123);
+
+  ++iter;
+  EXPECT_EQ(*iter, true);
+
+  ++iter;
+  EXPECT_TRUE(list.rend() == iter);
+  EXPECT_TRUE(const_list.rend() == iter);
+}
+
 // Test all three behaviors of EnsureDict() (Create a new dict where no
 // matchining values exist, return an existing dict, create a dict overwriting
 // a value of another type).
diff --git a/chrome/browser/ui/webui/settings/ash/privacy_hub_handler_unittest.cc b/chrome/browser/ui/webui/settings/ash/privacy_hub_handler_unittest.cc
index 44042f83..f8f5a6c 100644
--- a/chrome/browser/ui/webui/settings/ash/privacy_hub_handler_unittest.cc
+++ b/chrome/browser/ui/webui/settings/ash/privacy_hub_handler_unittest.cc
@@ -90,9 +90,7 @@
       }
 
       // Assume that the data is stored in the last valid arg.
-      auto data_span =
-          base::make_span(data->args().begin(), data->args().end());
-      for (const auto& arg : base::Reversed(data_span)) {
+      for (const auto& arg : base::Reversed(data->args())) {
         if (&arg != data->arg1())
           return arg.Clone();
       }