Add base::RandomShuffle as a replacement of deprecated std::random_sh…

…uffle

As std::random_shuffle is deprecated in C++14, and removed in C++17,
its usage blocks the transition to C++17.

This CL adds base::RandomShuffle as the replacement, updates all users,
and adds a presubmit check to ban std::random_shuffle.

Bug: 752720
Change-Id: I99a9dd267e56c04176cf3dc1f6bb7025cdb1deb6
Reviewed-on: https://chromium-review.googlesource.com/1023495
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Jeffrey Yasskin <jyasskin@chromium.org>
Commit-Queue: Taiju Tsuiki <tzik@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556680}

PRESUBMIT.py

@@ -577,6 +577,15 @@
       False,
       (),
     ),
+    (
+      r'std::random_shuffle',
+      (
+        'std::random_shuffle is deprecated in C++14, and removed in C++17. Use',
+        'base::RandomShuffle instead.'
+      ),
+      True,
+      (),
+    ),
 )
 
 

base/rand_util.h

@@ -8,6 +8,7 @@
 #include <stddef.h>
 #include <stdint.h>
 
+#include <algorithm>
 #include <string>
 
 #include "base/base_export.h"
@@ -22,10 +23,6 @@ BASE_EXPORT uint64_t RandUint64();
 BASE_EXPORT int RandInt(int min, int max);
 
 // Returns a random number in range [0, range).  Thread-safe.
-//
-// Note that this can be used as an adapter for std::random_shuffle():
-// Given a pre-populated |std::vector<int> myvector|, shuffle it as
-//   std::random_shuffle(myvector.begin(), myvector.end(), base::RandGenerator);
 BASE_EXPORT uint64_t RandGenerator(uint64_t range);
 
 // Returns a random double in range [0, 1). Thread-safe.
@@ -53,6 +50,25 @@ BASE_EXPORT void RandBytes(void* output, size_t output_length);
 // crypto::RandBytes instead to ensure the requirement is easily discoverable.
 BASE_EXPORT std::string RandBytesAsString(size_t length);
 
+// An STL UniformRandomBitGenerator backed by RandUint64.
+// TODO(tzik): Consider replacing this with a faster implementation.
+class RandomBitGenerator {
+ public:
+  using result_type = uint64_t;
+  static constexpr result_type min() { return 0; }
+  static constexpr result_type max() { return UINT64_MAX; }
+  result_type operator()() const { return RandUint64(); }
+
+  RandomBitGenerator() = default;
+  ~RandomBitGenerator() = default;
+};
+
+// Shuffles [first, last) randomly. Thread-safe.
+template <typename Itr>
+void RandomShuffle(Itr first, Itr last) {
+  std::shuffle(first, last, RandomBitGenerator());
+}
+
 #if defined(OS_POSIX) && !defined(OS_FUCHSIA)
 BASE_EXPORT int GetUrandomFD();
 #endif

chrome/browser/android/thumbnail/scoped_ptr_expiring_cache_unittest.cc

@@ -12,6 +12,7 @@
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/memory/ref_counted.h"
+#include "base/rand_util.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -154,7 +155,7 @@ TEST_F(ScopedPtrExpiringCacheTest, Iterator) {
   for (unsigned int i = 0; i < MAX_CACHE_SIZE; i++) {
     test_keys.push_back(i);
   }
-  std::random_shuffle(test_keys.begin(), test_keys.end());
+  base::RandomShuffle(test_keys.begin(), test_keys.end());
 
   for (unsigned int i = 0; i < MAX_CACHE_SIZE; i++) {
     cache.Put(test_keys[i], MockObject::Create(test_keys[i]));

chrome/browser/extensions/api/declarative_net_request/declarative_net_request_browsertest.cc

@@ -15,6 +15,7 @@
 #include "base/macros.h"
 #include "base/memory/scoped_refptr.h"
 #include "base/path_service.h"
+#include "base/rand_util.h"
 #include "base/test/histogram_tester.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/values.h"
@@ -927,7 +928,7 @@ IN_PROC_BROWSER_TEST_P(DeclarativeNetRequestBrowserTest, RedirectPriority) {
 
   // Shuffle the rules to ensure that the order in which rules are added has no
   // effect on the test.
-  std::random_shuffle(rules.begin(), rules.end());
+  base::RandomShuffle(rules.begin(), rules.end());
   ASSERT_NO_FATAL_FAILURE(LoadExtensionWithRules(rules));
 
   for (size_t i = 0; i <= kNumPatternTypes + 1; ++i) {

components/autofill/core/browser/personal_data_manager_unittest.cc

@@ -19,6 +19,7 @@
 #include "base/guid.h"
 #include "base/i18n/time_formatting.h"
 #include "base/message_loop/message_loop.h"
+#include "base/rand_util.h"
 #include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/synchronization/waitable_event.h"
@@ -5302,7 +5303,7 @@ TEST_F(PersonalDataManagerTest, RemoveProfilesNotUsedSinceTimestamp) {
 
     // Created a shuffled master copy of the profile pointers.
     std::vector<AutofillProfile*> shuffled_profiles(all_profile_ptrs);
-    std::random_shuffle(shuffled_profiles.begin(), shuffled_profiles.end());
+    base::RandomShuffle(shuffled_profiles.begin(), shuffled_profiles.end());
 
     // Copy the shuffled profile pointer collections to use as the working set.
     std::vector<AutofillProfile*> profiles(shuffled_profiles);
@@ -5531,7 +5532,7 @@ TEST_F(PersonalDataManagerTest, RemoveExpiredCreditCardsNotUsedSinceTimestamp) {
 
     // Created a shuffled master copy of the card pointers.
     std::vector<CreditCard*> shuffled_cards(all_card_ptrs);
-    std::random_shuffle(shuffled_cards.begin(), shuffled_cards.end());
+    base::RandomShuffle(shuffled_cards.begin(), shuffled_cards.end());
 
     // Copy the shuffled card pointer collections to use as the working
     // set.

components/sync/base/ordinal_unittest.cc

@@ -7,6 +7,7 @@
 #include <cctype>
 #include <vector>
 
+#include "base/rand_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace syncer {
@@ -350,7 +351,7 @@ TEST(Ordinal, Sort) {
   sorted_ordinals.push_back(ordinal4);
 
   std::vector<LongOrdinal> ordinals = sorted_ordinals;
-  std::random_shuffle(ordinals.begin(), ordinals.end());
+  base::RandomShuffle(ordinals.begin(), ordinals.end());
   std::sort(ordinals.begin(), ordinals.end(), LongOrdinal::LessThanFn());
   EXPECT_TRUE(std::equal(ordinals.begin(), ordinals.end(),
                          sorted_ordinals.begin(), LongOrdinal::EqualsFn()));

components/url_pattern_index/url_pattern_index_unittest.cc

@@ -12,6 +12,7 @@
 #include <vector>
 
 #include "base/macros.h"
+#include "base/rand_util.h"
 #include "base/strings/string_piece.h"
 #include "components/url_pattern_index/url_pattern.h"
 #include "components/url_pattern_index/url_rule_test_support.h"
@@ -748,7 +749,7 @@ TEST_F(UrlPatternIndexTest, FindMatchHighestPriority) {
     // Create a shuffled vector of priorities from 1 to |i|.
     std::vector<uint32_t> priorities(i);
     std::iota(priorities.begin(), priorities.end(), 1);
-    std::random_shuffle(priorities.begin(), priorities.end());
+    base::RandomShuffle(priorities.begin(), priorities.end());
 
     for (size_t j = 0; j < i; j++) {
       AddSimpleUrlRule(pattern, id, priorities[j]);

net/proxy_resolution/dhcp_pac_file_fetcher_win_unittest.cc

@@ -662,9 +662,7 @@ TEST(DhcpPacFileFetcherWin, ReuseFetcher) {
   test_functions.push_back(TestShortCircuitLessPreferredAdapters);
   test_functions.push_back(TestImmediateCancel);
 
-  std::random_shuffle(test_functions.begin(),
-                      test_functions.end(),
-                      base::RandGenerator);
+  base::RandomShuffle(test_functions.begin(), test_functions.end());
   for (TestVector::const_iterator it = test_functions.begin();
        it != test_functions.end();
        ++it) {

net/third_party/http2/hpack/decoder/hpack_block_collector.cc

@@ -8,6 +8,7 @@
 #include <memory>
 
 #include "base/logging.h"
+#include "base/rand_util.h"
 #include "net/third_party/http2/tools/failure.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -91,7 +92,7 @@ void HpackBlockCollector::ExpectLiteralNameAndValue(HpackEntryType type,
 }
 
 void HpackBlockCollector::ShuffleEntries(RandomBase* rng) {
-  std::random_shuffle(entries_.begin(), entries_.end());
+  base::RandomShuffle(entries_.begin(), entries_.end());
 }
 
 void HpackBlockCollector::AppendToHpackBlockBuilder(

third_party/crashpad/README.chromium

@@ -37,3 +37,5 @@ $ git am --3way --message-id -p4 /tmp/patchdir
 Local Modifications:
  - codereview.settings has been excluded.
  - thread_log_messages.cc (using ThreadLocalStorage::Slot instead of StaticSlot)
+ - Replaced std::random_shuffle with base::RandomShuffle in
+   prune_crash_reports_test.cc

third_party/crashpad/crashpad/client/prune_crash_reports_test.cc

@@ -217,11 +217,7 @@ TEST(PruneCrashReports, PruneOrder) {
     temp.creation_time = NDaysAgo(i * 10);
     reports.push_back(temp);
   }
-  // The randomness from std::rand() is not, so use a better rand() instead.
-  const auto random_generator = [](ptrdiff_t rand_max) {
-    return base::RandInt(0, base::checked_cast<int>(rand_max) - 1);
-  };
-  std::random_shuffle(reports.begin(), reports.end(), random_generator);
+  base::RandomShuffle(reports.begin(), reports.end());
   std::vector<CrashReportDatabase::Report> pending_reports(
       reports.begin(), reports.begin() + 5);
   std::vector<CrashReportDatabase::Report> completed_reports(

tools/ipc_fuzzer/fuzzer/fuzzer_main.cc

@@ -195,8 +195,8 @@ int Mutate(base::CommandLine* cmd, Fuzzer* fuzzer) {
   }
 
   if (permute) {
-    std::random_shuffle(message_vector.begin(), message_vector.end(),
-                        RandInRange);
+    std::shuffle(message_vector.begin(), message_vector.end(),
+                 *g_mersenne_twister);
   }
 
   if (!MessageFile::Write(base::FilePath(output_file_name), message_vector))