From 1af573718790e59c296227211dff0b1d73d589e9 Mon Sep 17 00:00:00 2001
From: Oleksandr Tara <[email protected]>
Date: Thu, 28 May 2026 03:53:09 -0700
Subject: [PATCH] Refactor zxcvbn RankedDicts to be thread-safe.

This change introduces a RefCountedRankedDicts wrapper and uses a
read-copy-update (RCU) pattern with a lock to manage the global default
RankedDicts. This ensures that background threads can safely read from a
snapshot of the dictionaries while the main thread updates them,
preventing data races. The old dictionary is asynchronously released on
a thread pool to avoid blocking.

Fixed: 513455047
Change-Id: I5a2b5dd169351b217aba945d5fc7b9236eeaf0dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7852593
Reviewed-by: Xiaoling Bao <[email protected]>
Reviewed-by: Viktor Semeniuk <[email protected]>
Commit-Queue: Oleksandr Tara <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1637595}
---

diff --git a/chrome/browser/component_updater/zxcvbn_data_component_installer_unittest.cc b/chrome/browser/component_updater/zxcvbn_data_component_installer_unittest.cc
index cbbc971..3eaa844 100644
--- a/chrome/browser/component_updater/zxcvbn_data_component_installer_unittest.cc
+++ b/chrome/browser/component_updater/zxcvbn_data_component_installer_unittest.cc
@@ -147,7 +147,9 @@
   }
 
   void VerifyRankedDicts() {
-    zxcvbn::RankedDicts& ranked_dicts = zxcvbn::default_ranked_dicts();
+    scoped_refptr<zxcvbn::RefCountedRankedDicts> dicts_ref =
+        zxcvbn::default_ranked_dicts();
+    const zxcvbn::RankedDicts& ranked_dicts = dicts_ref->Data();
     EXPECT_EQ(ranked_dicts.Find("english"), 1UL);
     EXPECT_EQ(ranked_dicts.Find("wikipedia"), 2UL);
     EXPECT_EQ(ranked_dicts.Find("female"), 1UL);
@@ -217,7 +219,8 @@
   policy().ComponentReady(version(), GetPath(), manifest().Clone());
   task_env().RunUntilIdle();
 
-  EXPECT_FALSE(zxcvbn::default_ranked_dicts().Find("english").has_value());
+  EXPECT_FALSE(
+      zxcvbn::default_ranked_dicts()->Data().Find("english").has_value());
 }
 
 // Tests that ComponentReady reads in the file contents and properly populates
diff --git a/third_party/zxcvbn-cpp/README.chromium b/third_party/zxcvbn-cpp/README.chromium
index b4cd7a9f..bb9ba07 100644
--- a/third_party/zxcvbn-cpp/README.chromium
+++ b/third_party/zxcvbn-cpp/README.chromium
@@ -70,6 +70,7 @@
   //base equivalents as the former is deprecated in C++17.
 - Add a getter for testing and replace
   StringPiece with std::string_view.
+- Refactor zxcvbn RankedDicts to be thread-safe.
 
 Ran the following commands to generate adjacency graphs:
 $ python ./data-scripts/build_keyboard_adjacency_graphs.py ./native-src/zxcvbn/adjacency_graphs.hpp
diff --git a/third_party/zxcvbn-cpp/google.patch b/third_party/zxcvbn-cpp/google.patch
index 56c2b3c4..4d3b0e8 100644
--- a/third_party/zxcvbn-cpp/google.patch
+++ b/third_party/zxcvbn-cpp/google.patch
@@ -1,6 +1,6 @@
 diff --git a/third_party/zxcvbn-cpp/native-src/zxcvbn/adjacency_graphs_js_bindings.cpp b/third_party/zxcvbn-cpp/native-src/zxcvbn/adjacency_graphs_js_bindings.cpp
 deleted file mode 100644
-index 7e1ae349a8e0f..0000000000000
+index 7e1ae34..0000000
 --- a/third_party/zxcvbn-cpp/native-src/zxcvbn/adjacency_graphs_js_bindings.cpp
 +++ /dev/null
 @@ -1,49 +0,0 @@
@@ -54,7 +54,7 @@
 -
 -#endif
 diff --git a/third_party/zxcvbn-cpp/native-src/zxcvbn/common.hpp b/third_party/zxcvbn-cpp/native-src/zxcvbn/common.hpp
-index cba756c449a63..1f66367112aa8 100644
+index cba756c..1f66367 100644
 --- a/third_party/zxcvbn-cpp/native-src/zxcvbn/common.hpp
 +++ b/third_party/zxcvbn-cpp/native-src/zxcvbn/common.hpp
 @@ -1,7 +1,6 @@
@@ -99,7 +99,7 @@
    bool l33t;
 diff --git a/third_party/zxcvbn-cpp/native-src/zxcvbn/common_js.hpp b/third_party/zxcvbn-cpp/native-src/zxcvbn/common_js.hpp
 deleted file mode 100644
-index 729cbad18709c..0000000000000
+index 729cbad..0000000
 --- a/third_party/zxcvbn-cpp/native-src/zxcvbn/common_js.hpp
 +++ /dev/null
 @@ -1,515 +0,0 @@
@@ -620,7 +620,7 @@
 -#endif
 diff --git a/third_party/zxcvbn-cpp/native-src/zxcvbn/feedback.cpp b/third_party/zxcvbn-cpp/native-src/zxcvbn/feedback.cpp
 deleted file mode 100644
-index d22c296188973..0000000000000
+index d22c296..0000000
 --- a/third_party/zxcvbn-cpp/native-src/zxcvbn/feedback.cpp
 +++ /dev/null
 @@ -1,174 +0,0 @@
@@ -800,7 +800,7 @@
 -
 diff --git a/third_party/zxcvbn-cpp/native-src/zxcvbn/feedback.hpp b/third_party/zxcvbn-cpp/native-src/zxcvbn/feedback.hpp
 deleted file mode 100644
-index 6f6b62b9da054..0000000000000
+index 6f6b62b..0000000
 --- a/third_party/zxcvbn-cpp/native-src/zxcvbn/feedback.hpp
 +++ /dev/null
 @@ -1,20 +0,0 @@
@@ -825,10 +825,10 @@
 -
 -#endif
 diff --git a/third_party/zxcvbn-cpp/native-src/zxcvbn/frequency_lists.cpp b/third_party/zxcvbn-cpp/native-src/zxcvbn/frequency_lists.cpp
-index a4d65612d8a25..67a2aff5542f3 100644
+index a4d6561..4b328f1 100644
 --- a/third_party/zxcvbn-cpp/native-src/zxcvbn/frequency_lists.cpp
 +++ b/third_party/zxcvbn-cpp/native-src/zxcvbn/frequency_lists.cpp
-@@ -1,24 +1,245 @@
+@@ -1,24 +1,266 @@
  #include <zxcvbn/frequency_lists.hpp>
  
 -#include <zxcvbn/_frequency_lists.hpp>
@@ -844,6 +844,7 @@
 +#include "base/no_destructor.h"
 +#include "base/notreached.h"
 +#include "base/task/thread_pool.h"
++#include "base/synchronization/lock.h"
 +#include "third_party/abseil-cpp/absl/types/optional.h"
 +#include "third_party/abseil-cpp/absl/types/variant.h"
  
@@ -942,7 +943,18 @@
 +// Helper function that does nothing with the RankedDicts apart from letting
 +// it destruct as it goes out of scope. This is called on the ThreadPool to
 +// allow for potentially blocking behavior of `RankedDicts` destructor.
-+void DoNothing(RankedDicts dicts) {}
++void DoNothing(scoped_refptr<RefCountedRankedDicts> dicts) {}
++
++base::Lock& GetRankedDictsLock() {
++  static base::NoDestructor<base::Lock> lock;
++  return *lock;
++}
++
++scoped_refptr<RefCountedRankedDicts>& GetRankedDictsPointer() {
++  static base::NoDestructor<scoped_refptr<RefCountedRankedDicts>> ptr(
++      base::MakeRefCounted<RefCountedRankedDicts>(RankedDicts()));
++  return *ptr;
++}
 +
 +}  // namespace
 +
@@ -973,8 +985,7 @@
 +  }
 +  std::sort(merged_dicts.begin(), merged_dicts.end(),
 +            [](MergedEntry& a, MergedEntry& b) { return a.value < b.value; });
- 
--  return build;
++
 +  if (merged_dicts.size() == 0)
 +    return;
 +
@@ -986,7 +997,8 @@
 +  // 1 byte at the end for trailing marker byte (for finding last string size)
 +  std::vector<char> vec;
 +  vec.reserve(dict_size + 1);
-+
+ 
+-  return build;
 +  // second pass: place elements in allocated array
 +  for (MergedEntry& entry : merged_dicts)
 +    RankedDictEntryRef::AppendToVector(entry, vec);
@@ -1062,32 +1074,41 @@
 +    }
 +  }
 +  return false;
-+}
-+
-+void SetRankedDictsImplementation(RankedDicts dicts) {
-+  default_ranked_dicts() = std::move(dicts);
-+}
-+
-+void SetRankedDicts(RankedDicts dicts) {
-+  // Destroying a `RankedDict` may block if it is based on a `MemoryMappedFile`.
-+  // Therefore this helper moves the task of doing it to a thread pool.
-+  base::ThreadPool::PostTask(
-+      FROM_HERE, {base::MayBlock(), base::TaskPriority::BEST_EFFORT},
-+      base::BindOnce(&DoNothing, std::move(default_ranked_dicts())));
-+  default_ranked_dicts() = std::move(dicts);
-+}
-+
-+RankedDicts& default_ranked_dicts() {
-+  static base::NoDestructor<RankedDicts> default_dicts;
-+  return *default_dicts;
  }
 +
++// Safely updates the global `RankedDicts` using a read-copy-update (RCU) pattern.
++// A lock is held briefly to safely update the global `scoped_refptr`, preventing
++// data races against reader threads. The old `RankedDicts` obj is safely unmapped
++// asynchronously if it was using a `MemoryMappedFile`.
++void SetRankedDicts(RankedDicts dicts) {
++  scoped_refptr<RefCountedRankedDicts> new_dicts =
++      base::MakeRefCounted<RefCountedRankedDicts>(std::move(dicts));
++  scoped_refptr<RefCountedRankedDicts> old_dicts;
++  {

diff --git a/chrome/browser/component_updater/zxcvbn_data_component_installer_unittest.cc b/chrome/browser/component_updater/zxcvbn_data_component_installer_unittest.cc
index cbbc971..3eaa844 100644
--- a/chrome/browser/component_updater/zxcvbn_data_component_installer_unittest.cc
+++ b/chrome/browser/component_updater/zxcvbn_data_component_installer_unittest.cc
@@ -147,7 +147,9 @@
   }
 
   void VerifyRankedDicts() {
-    zxcvbn::RankedDicts& ranked_dicts = zxcvbn::default_ranked_dicts();
+    scoped_refptr<zxcvbn::RefCountedRankedDicts> dicts_ref =
+        zxcvbn::default_ranked_dicts();
+    const zxcvbn::RankedDicts& ranked_dicts = dicts_ref->Data();
     EXPECT_EQ(ranked_dicts.Find("english"), 1UL);
     EXPECT_EQ(ranked_dicts.Find("wikipedia"), 2UL);
     EXPECT_EQ(ranked_dicts.Find("female"), 1UL);
@@ -217,7 +219,8 @@
   policy().ComponentReady(version(), GetPath(), manifest().Clone());
   task_env().RunUntilIdle();
 
-  EXPECT_FALSE(zxcvbn::default_ranked_dicts().Find("english").has_value());
+  EXPECT_FALSE(
+      zxcvbn::default_ranked_dicts()->Data().Find("english").has_value());
 }
 
 // Tests that ComponentReady reads in the file contents and properly populates
diff --git a/third_party/zxcvbn-cpp/test/matching_unittest.cc b/third_party/zxcvbn-cpp/test/matching_unittest.cc
index 27dc9b7..db8d5c7 100644
--- a/third_party/zxcvbn-cpp/test/matching_unittest.cc
+++ b/third_party/zxcvbn-cpp/test/matching_unittest.cc
@@ -281,7 +281,7 @@
     // default dictionaries
     SetRankedDicts(RankedDicts({{"wow"}}));
     std::vector<Match> matches =
-        dictionary_match("wow", default_ranked_dicts());
+        dictionary_match("wow", default_ranked_dicts()->Data());
     EXPECT_THAT(matches, ElementsAre(ExpectedDictionaryMatch{
                              .i = 0,
                              .j = 2,