[base] Remove const iterator member functions from span

This change implements the resolution of LWG issue 3320 by removing the
members cbegin(), cend(), crbegin() and crend() from base::span. This
was done because std::cbegin(std::span<T>()) and std::span<T>().cbegin()
returned different results if T was non-const.

Note that the nested const_iterator type is not yet removed from
base::span, as this breaks using it in gMock matchers like ElementsAre.
This will be addressed once gMock no longer requires the presence of
this nested type.

References:
* https://wg21.link/LWG3320
* cplusplus/draft@6faa9c

TBR=dcheng

Bug: 828324
Change-Id: I21f8ffaa0c7183d6e63a087b73f271b677d13f3a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2086231
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Chris Palmer <palmer@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#747216}

base/containers/span.h

@@ -227,9 +227,10 @@ class span : public internal::ExtentStorage<Extent> {
   using pointer = T*;
   using reference = T&;
   using iterator = CheckedContiguousIterator<T>;
-  using const_iterator = CheckedContiguousConstIterator<T>;
+  // TODO(https://crbug.com/828324): Drop the const_iterator typedef once gMock
+  // supports containers without this nested type.
+  using const_iterator = iterator;
   using reverse_iterator = std::reverse_iterator<iterator>;
-  using const_reverse_iterator = std::reverse_iterator<const_iterator>;
   static constexpr size_t extent = Extent;
 
   // [span.cons], span constructors, copy, assignment, and destructor
@@ -395,27 +396,19 @@ class span : public internal::ExtentStorage<Extent> {
   constexpr iterator begin() const noexcept {
     return iterator(data_, data_ + size());
   }
+
   constexpr iterator end() const noexcept {
     return iterator(data_, data_ + size(), data_ + size());
   }
 
-  constexpr const_iterator cbegin() const noexcept { return begin(); }
-  constexpr const_iterator cend() const noexcept { return end(); }
-
   constexpr reverse_iterator rbegin() const noexcept {
     return reverse_iterator(end());
   }
+
   constexpr reverse_iterator rend() const noexcept {
     return reverse_iterator(begin());
   }
 
-  constexpr const_reverse_iterator crbegin() const noexcept {
-    return const_reverse_iterator(cend());
-  }
-  constexpr const_reverse_iterator crend() const noexcept {
-    return const_reverse_iterator(cbegin());
-  }
-
  private:
   T* data_;
 };

base/containers/span_unittest.cc

@@ -7,6 +7,7 @@
 #include <stdint.h>
 
 #include <algorithm>
+#include <iterator>
 #include <memory>
 #include <string>
 #include <type_traits>
@@ -965,7 +966,7 @@ TEST(SpanTest, ReverseIterator) {
   EXPECT_TRUE(std::equal(std::rbegin(kArray), std::rend(kArray), span.rbegin(),
                          span.rend()));
   EXPECT_TRUE(std::equal(std::crbegin(kArray), std::crend(kArray),
-                         span.crbegin(), span.crend()));
+                         std::crbegin(span), std::crend(span)));
 }
 
 TEST(SpanTest, AsBytes) {
@@ -1259,11 +1260,6 @@ TEST(SpanTest, IteratorIsRangeMoveSafe) {
         CheckedContiguousIterator<const int>(
             span.data() + dest_start_index,
             span.data() + dest_start_index + kNumElements)));
-    EXPECT_FALSE(CheckedContiguousConstIterator<const int>::IsRangeMoveSafe(
-        span.cbegin(), span.cend(),
-        CheckedContiguousConstIterator<const int>(
-            span.data() + dest_start_index,
-            span.data() + dest_start_index + kNumElements)));
   }
 
   // Non-overlapping ranges.
@@ -1273,28 +1269,17 @@ TEST(SpanTest, IteratorIsRangeMoveSafe) {
         CheckedContiguousIterator<const int>(
             span.data() + dest_start_index,
             span.data() + dest_start_index + kNumElements)));
-    EXPECT_TRUE(CheckedContiguousConstIterator<const int>::IsRangeMoveSafe(
-        span.cbegin(), span.cend(),
-        CheckedContiguousConstIterator<const int>(
-            span.data() + dest_start_index,
-            span.data() + dest_start_index + kNumElements)));
   }
 
   // IsRangeMoveSafe is true if the length to be moved is 0.
   EXPECT_TRUE(CheckedContiguousIterator<const int>::IsRangeMoveSafe(
       span.begin(), span.begin(),
       CheckedContiguousIterator<const int>(span.data(), span.data())));
-  EXPECT_TRUE(CheckedContiguousConstIterator<const int>::IsRangeMoveSafe(
-      span.cbegin(), span.cbegin(),
-      CheckedContiguousConstIterator<const int>(span.data(), span.data())));
 
   // IsRangeMoveSafe is false if end < begin.
   EXPECT_FALSE(CheckedContiguousIterator<const int>::IsRangeMoveSafe(
       span.end(), span.begin(),
       CheckedContiguousIterator<const int>(span.data(), span.data())));
-  EXPECT_FALSE(CheckedContiguousConstIterator<const int>::IsRangeMoveSafe(
-      span.cend(), span.cbegin(),
-      CheckedContiguousConstIterator<const int>(span.data(), span.data())));
 }
 
 TEST(SpanTest, Sort) {
@@ -1336,12 +1321,12 @@ TEST(SpanTest, SpanExtentConversions) {
 
 TEST(SpanTest, IteratorConversions) {
   static_assert(std::is_convertible<span<int>::iterator,
-                                    span<int>::const_iterator>::value,
-                "Error: iterator should be convertible to const_iterator");
+                                    span<const int>::iterator>::value,
+                "Error: iterator should be convertible to const iterator");
 
-  static_assert(!std::is_convertible<span<int>::const_iterator,
+  static_assert(!std::is_convertible<span<const int>::iterator,
                                      span<int>::iterator>::value,
-                "Error: const_iterator should not be convertible to iterator");
+                "Error: const iterator should not be convertible to iterator");
 }
 
 }  // namespace base

components/cbor/reader_unittest.cc

@@ -19,7 +19,7 @@ namespace cbor {
 namespace {
 
 std::vector<uint8_t> WithExtraneousData(base::span<const uint8_t> original) {
-  std::vector<uint8_t> ret(original.cbegin(), original.cend());
+  std::vector<uint8_t> ret(original.begin(), original.end());
   // Add a valid one byte long CBOR data item, namely, an unsigned integer
   // with value "1".
   ret.push_back(0x01);

device/fido/opaque_public_key.cc

@@ -8,9 +8,8 @@ namespace device {
 
 OpaquePublicKey::OpaquePublicKey(
     base::span<const uint8_t> cose_encoded_public_key)
-    : PublicKey(),
-      cose_encoding_(std::vector<uint8_t>(cose_encoded_public_key.cbegin(),
-                                          cose_encoded_public_key.cend())) {}
+    : cose_encoding_(cose_encoded_public_key.begin(),
+                     cose_encoded_public_key.end()) {}
 
 OpaquePublicKey::~OpaquePublicKey() = default;
 

gpu/ipc/client/image_decode_accelerator_proxy.cc

@@ -195,8 +195,7 @@ SyncToken ImageDecodeAcceleratorProxy::ScheduleImageDecode(
             ChannelIdFromCommandBufferId(raster_decoder_command_buffer_id));
 
   GpuChannelMsg_ScheduleImageDecode_Params params;
-  params.encoded_data =
-      std::vector<uint8_t>(encoded_data.cbegin(), encoded_data.cend());
+  params.encoded_data.assign(encoded_data.begin(), encoded_data.end());
   params.output_size = output_size;
   params.raster_decoder_route_id =
       RouteIdFromCommandBufferId(raster_decoder_command_buffer_id);

gpu/ipc/client/image_decode_accelerator_proxy_unittest.cc

@@ -111,8 +111,7 @@ TEST_F(ImageDecodeAcceleratorProxyTest, ScheduleImageDecodeSendsMessage) {
   const gfx::ColorSpace color_space = gfx::ColorSpace::CreateSRGB();
 
   GpuChannelMsg_ScheduleImageDecode_Params expected_params;
-  expected_params.encoded_data =
-      std::vector<uint8_t>(encoded_data.cbegin(), encoded_data.cend());
+  expected_params.encoded_data.assign(encoded_data.begin(), encoded_data.end());
   expected_params.output_size = kOutputSize;
   expected_params.raster_decoder_route_id = kRasterCmdBufferRouteId;
   expected_params.transfer_cache_entry_id = 1u;

media/gpu/vaapi/vaapi_image_decode_accelerator_worker_unittest.cc

@@ -65,11 +65,10 @@ constexpr uint8_t kLossyWebPFileHeader[] = {
 };
 // clang-format on
 
-constexpr base::span<const uint8_t> kJpegEncodedData(kJpegPFileHeader, 3u);
+constexpr base::span<const uint8_t, 3u> kJpegEncodedData = kJpegPFileHeader;
 
-constexpr base::span<const uint8_t> kLossyWebPEncodedData(
-    kLossyWebPFileHeader,
-    kWebPFileAndVp8ChunkHeaderSizeInBytes);
+constexpr base::span<const uint8_t, kWebPFileAndVp8ChunkHeaderSizeInBytes>
+    kLossyWebPEncodedData = kLossyWebPFileHeader;
 
 class MockNativePixmapDmaBuf : public gfx::NativePixmapDmaBuf {
  public:
@@ -185,10 +184,10 @@ ACTION_P2(ExportAsNativePixmapDmaBufSuccessfully,
 }
 
 TEST_F(VaapiImageDecodeAcceleratorWorkerTest, ImageDecodeSucceeds) {
-  std::vector<uint8_t> jpeg_encoded_data(kJpegEncodedData.cbegin(),
-                                         kJpegEncodedData.cend());
-  std::vector<uint8_t> webp_encoded_data(kLossyWebPEncodedData.cbegin(),
-                                         kLossyWebPEncodedData.cend());
+  std::vector<uint8_t> jpeg_encoded_data(kJpegEncodedData.begin(),
+                                         kJpegEncodedData.end());
+  std::vector<uint8_t> webp_encoded_data(kLossyWebPEncodedData.begin(),
+                                         kLossyWebPEncodedData.end());
   {
     InSequence sequence;
     MockVaapiImageDecoder* jpeg_decoder = GetJpegDecoder();
@@ -235,10 +234,10 @@ TEST_F(VaapiImageDecodeAcceleratorWorkerTest, ImageDecodeSucceeds) {
 }
 
 TEST_F(VaapiImageDecodeAcceleratorWorkerTest, ImageDecodeFails) {
-  std::vector<uint8_t> jpeg_encoded_data(kJpegEncodedData.cbegin(),
-                                         kJpegEncodedData.cend());
-  std::vector<uint8_t> webp_encoded_data(kLossyWebPEncodedData.cbegin(),
-                                         kLossyWebPEncodedData.cend());
+  std::vector<uint8_t> jpeg_encoded_data(kJpegEncodedData.begin(),
+                                         kJpegEncodedData.end());
+  std::vector<uint8_t> webp_encoded_data(kLossyWebPEncodedData.begin(),
+                                         kLossyWebPEncodedData.end());
   {
     InSequence sequence;
     MockVaapiImageDecoder* jpeg_decoder = GetJpegDecoder();