Add a forward iterator for LruCache

This patch also changes the call sites to use the iterator.

Note there is one usage of a backward loop. Replacing it with the range-based for loop requires the reverse adapter and `rbegin`/`rend`, that aren't included in this patch.

This patch should have no behavior changes.

PiperOrigin-RevId: 609779132

src/prediction/user_history_predictor.cc

@@ -521,17 +521,16 @@ bool UserHistoryPredictor::ClearUnusedHistory() {
   WaitForSyncer();
 
   MOZC_VLOG(1) << "Clearing unused prediction";
-  const DicElement *head = dic_->Head();
-  if (head == nullptr) {
-    MOZC_VLOG(2) << "dic head is nullptr";
+  if (dic_->empty()) {
+    MOZC_VLOG(2) << "dic is empty";
     return false;
   }
 
   std::vector<uint32_t> keys;
-  for (const DicElement *elm = head; elm != nullptr; elm = elm->next) {
-    MOZC_VLOG(3) << elm->key << " " << elm->value.suggestion_freq();
-    if (elm->value.suggestion_freq() == 0) {
-      keys.push_back(elm->key);
+  for (const DicElement &elm : *dic_) {
+    MOZC_VLOG(3) << elm.key << " " << elm.value.suggestion_freq();
+    if (elm.value.suggestion_freq() == 0) {
+      keys.push_back(elm.key);
     }
   }
 
@@ -669,9 +668,8 @@ bool UserHistoryPredictor::ClearHistoryEntry(const absl::string_view key,
     // Finds a chain of history entries that produces key and value. If exists,
     // remove the link so that N-gram history prediction never generates this
     // key value pair..
-    for (DicElement *elm = dic_->MutableHead(); elm != nullptr;
-         elm = elm->next) {
-      Entry *entry = &elm->value;
+    for (DicElement &elm : *dic_) {
+      Entry *entry = &elm.value;
       if (!absl::StartsWith(key, entry->key()) ||
           !absl::StartsWith(value, entry->value())) {
         continue;
@@ -1191,8 +1189,8 @@ bool UserHistoryPredictor::ShouldPredict(RequestType request_type,
     return false;
   }
 
-  if (dic_->Head() == nullptr) {
-    MOZC_VLOG(2) << "dic head is nullptr";
+  if (dic_->empty()) {
+    MOZC_VLOG(2) << "dic is empty";
     return false;
   }
 
@@ -1244,9 +1242,11 @@ const UserHistoryPredictor::Entry *UserHistoryPredictor::LookupPrevEntry(
                                         ? history_segment.candidate(0).value
                                         : prev_entry->value();
     int trial = 0;
-    for (const DicElement *elm = dic_->Head();
-         trial++ < kMaxPrevValueTrial && elm != nullptr; elm = elm->next) {
-      const Entry *entry = &(elm->value);
+    for (const DicElement &elm : *dic_) {
+      if (++trial > kMaxPrevValueTrial) {
+        break;
+      }
+      const Entry *entry = &(elm.value);
       // entry->value() equals to the prev_value or
       // entry->value() is a SUFFIX of prev_value.
       // length of entry->value() must be >= 2, as single-length
@@ -1299,11 +1299,11 @@ void UserHistoryPredictor::GetResultsFromHistoryDictionary(
 
   const absl::Time now = Clock::GetAbslTime();
   int trial = 0;
-  for (const DicElement *elm = dic_->Head(); elm != nullptr; elm = elm->next) {
-    if (!IsValidEntryIgnoringRemovedField(elm->value)) {
+  for (const DicElement &elm : *dic_) {
+    if (!IsValidEntryIgnoringRemovedField(elm.value)) {
       continue;
     }
-    if (absl::FromUnixSeconds(elm->value.last_access_time()) + k62Days < now) {
+    if (absl::FromUnixSeconds(elm.value.last_access_time()) + k62Days < now) {
       updated_ = true;  // We found an entry to be deleted at next save.
       continue;
     }
@@ -1317,8 +1317,8 @@ void UserHistoryPredictor::GetResultsFromHistoryDictionary(
     // If a new entry is found, the entry is pushed to the results.
     // TODO(team): make KanaFuzzyLookupEntry().
     if (!LookupEntry(request_type, input_key, base_key, expanded.get(),
-                     &(elm->value), prev_entry, results) &&
-        !RomanFuzzyLookupEntry(roman_input_key, &(elm->value), results)) {
+                     &(elm.value), prev_entry, results) &&
+        !RomanFuzzyLookupEntry(roman_input_key, &(elm.value), results)) {
       continue;
     }
 
@@ -1713,7 +1713,7 @@ void UserHistoryPredictor::Finish(const ConversionRequest &request,
   // This is a fix for http://b/issue?id=2216838
   //
   // Note: We don't make such candidates for mobile.
-  if (request_type != ZERO_QUERY_SUGGESTION && dic_->Head() != nullptr &&
+  if (request_type != ZERO_QUERY_SUGGESTION && !dic_->empty() &&
       dic_->Head()->value.last_access_time() + 5 > last_access_time &&
       // Check if the current value is a punctuation.
       segments->conversion_segments_size() == 1 &&

src/prediction/user_history_predictor_test.cc

@@ -622,10 +622,12 @@ TEST_F(UserHistoryPredictorTest, UserHistoryPredictorTestSuggestion) {
     predictor->Finish(*convreq_, &segments);
 
     // All added items must be suggestion entries.
-    const UserHistoryPredictor::DicCache::Element *element;
-    for (element = predictor->dic_->Head(); element->next;
-         element = element->next) {
-      const user_history_predictor::UserHistory::Entry &entry = element->value;
+    for (const UserHistoryPredictor::DicCache::Element &element :
+         *predictor->dic_) {
+      if (!element.next) {
+        break;  // Except the last one.
+      }
+      const user_history_predictor::UserHistory::Entry &entry = element.value;
       EXPECT_TRUE(entry.has_suggestion_freq() && entry.suggestion_freq() == 1);
       EXPECT_TRUE(!entry.has_conversion_freq() && entry.conversion_freq() == 0);
     }
@@ -4168,10 +4170,9 @@ TEST_F(UserHistoryPredictorTest, 62DayOldEntriesAreDeletedAtSync) {
 
   // Verify also that on-memory data structure doesn't contain node for 中野.
   bool found_takahashi = false;
-  for (const auto *elem = predictor->dic_->Head(); elem != nullptr;
-       elem = elem->next) {
-    EXPECT_EQ(elem->value.value().find("中野"), std::string::npos);
-    if (elem->value.value().find("高橋")) {
+  for (const auto &elem : *predictor->dic_) {
+    EXPECT_EQ(elem.value.value().find("中野"), std::string::npos);
+    if (elem.value.value().find("高橋")) {
       found_takahashi = true;
     }
   }

src/session/session_handler.cc

@@ -210,9 +210,8 @@ void SessionHandler::UpdateSessions(const config::Config &config,
     key_map_manager_ = std::make_unique<keymap::KeyMapManager>(*config_);
   }
 
-  for (SessionElement *element = session_map_->MutableHead();
-       element != nullptr; element = element->next) {
-    std::unique_ptr<session::Session> &session = element->value;
+  for (SessionElement &element : *session_map_) {
+    std::unique_ptr<session::Session> &session = element.value;
     if (!session) {
       continue;
     }
@@ -609,22 +608,21 @@ bool SessionHandler::Cleanup(commands::Command *command) {
                    absl::Seconds(7200)));
 
   std::vector<SessionID> remove_ids;
-  for (const SessionElement *element = session_map_->Head(); element != nullptr;
-       element = element->next) {
-    const session::Session *session = element->value.get();
+  for (const SessionElement &element : *session_map_) {
+    const session::Session *session = element.value.get();
     if (!IsApplicationAlive(session)) {
-      MOZC_VLOG(2) << "Application is not alive. Removing: " << element->key;
-      remove_ids.push_back(element->key);
+      MOZC_VLOG(2) << "Application is not alive. Removing: " << element.key;
+      remove_ids.push_back(element.key);
     } else if (session->last_command_time() == absl::InfinitePast()) {
       // no command is executed
       if ((current_time - session->create_session_time()) >=
           create_session_timeout) {
-        remove_ids.push_back(element->key);
+        remove_ids.push_back(element.key);
       }
     } else {  // some commands are executed already
       if ((current_time - session->last_command_time()) >=
           last_command_timeout) {
-        remove_ids.push_back(element->key);
+        remove_ids.push_back(element.key);
       }
     }
   }

src/storage/lru_cache.h

@@ -30,7 +30,10 @@
 #ifndef MOZC_STORAGE_LRU_CACHE_H_
 #define MOZC_STORAGE_LRU_CACHE_H_
 
+#include <cstddef>
+#include <iterator>
 #include <memory>
+#include <type_traits>
 
 #include "absl/container/flat_hash_map.h"
 #include "base/logging.h"
@@ -44,13 +47,6 @@ namespace storage {
 template <typename Key, typename Value>
 class LruCache {
  public:
-  // Constructs a new LruCache that can hold at most max_elements
-  explicit LruCache(size_t max_elements);
-  ~LruCache() = default;
-
-  LruCache(const LruCache&) = delete;
-  LruCache& operator=(const LruCache&) = delete;
-
   // Every Element is either on the free list or the lru list.  The
   // free list is singly-linked and only uses the next pointer, while
   // the LRU list is doubly-linked and uses both next and prev.
@@ -61,6 +57,55 @@ class LruCache {
     Value value;
   };
 
+  template <bool is_const>
+  class Iterator {
+   public:
+    using iterator_category = std::forward_iterator_tag;
+    using value_type = std::conditional_t<is_const, const Element, Element>;
+    using difference_type = ptrdiff_t;
+    using pointer = value_type*;
+    using reference = value_type&;
+
+    explicit Iterator(pointer element)
+        : current_(element), next_(element ? element->next : nullptr) {}
+
+    reference operator*() const { return *current_; }
+    pointer operator->() const { return current_; }
+
+    Iterator& operator++() {
+      current_ = next_;
+      // Capture `next` for when it's changed before the next increment.
+      next_ = current_ ? current_->next : nullptr;
+      return *this;
+    }
+
+    bool operator==(const Iterator& other) const {
+      return current_ == other.current_;
+    }
+    bool operator!=(const Iterator& other) const {
+      return current_ != other.current_;
+    }
+
+   private:
+    pointer current_;
+    pointer next_;
+  };
+  using iterator = Iterator</*is_const=*/false>;
+  using const_iterator = Iterator</*is_const=*/true>;
+
+  // Constructs a new LruCache that can hold at most max_elements
+  explicit LruCache(size_t max_elements);
+  ~LruCache() = default;
+
+  LruCache(const LruCache&) = delete;
+  LruCache& operator=(const LruCache&) = delete;
+
+  // Iterators
+  iterator begin() { return iterator{lru_head_}; }
+  iterator end() { return iterator{nullptr}; }
+  const_iterator begin() const { return const_iterator{lru_head_}; }
+  const_iterator end() const { return const_iterator{nullptr}; }
+
   // Adds the specified key/value pair into the cache, putting it at the head
   // of the LRU list.
   void Insert(const Key& key, const Value& value);
@@ -96,6 +141,7 @@ class LruCache {
 
   // Returns the number of entries currently in the cache.
   size_t Size() const { return table_.size(); }
+  bool empty() const { return lru_head_ == nullptr; }
 
   bool HasKey(const Key& key) const { return table_.find(key) != table_.end(); }
 
@@ -107,6 +153,9 @@ class LruCache {
   const Element* Tail() const { return lru_tail_; }
   Element* MutableTail() const { return lru_tail_; }
 
+  // Expose the free list only for testing purposes.
+  const Element* FreeListForTesting() const { return free_list_; }
+
  private:
   // Allocates a new block containing next_block_size_ elements, updates
   // next_block_size_ appropriately, and pushes the elements in the new block
@@ -352,11 +401,8 @@ bool LruCache<Key, Value>::Erase(const Key& key) {
 template <typename Key, typename Value>
 void LruCache<Key, Value>::Clear() {
   table_.clear();
-  Element* e = lru_head_;
-  while (e != nullptr) {
-    Element* next = e->next;
-    PushFreeList(e);
-    e = next;
+  for (Element& e : *this) {
+    PushFreeList(&e);
   }
   lru_head_ = lru_tail_ = nullptr;
 }

src/storage/lru_cache_test.cc

@@ -29,6 +29,7 @@
 
 #include "storage/lru_cache.h"
 
+#include <cstddef>
 #include <vector>
 
 #include "testing/gmock.h"
@@ -40,12 +41,23 @@ namespace {
 
 using ::testing::ElementsAre;
 
+template <typename Key, typename Value>
+size_t SizeOfFreeList(const LruCache<Key, Value> &cache) {
+  size_t size = 0;
+  for (const typename LruCache<Key, Value>::Element *e =
+           cache.FreeListForTesting();
+       e; e = e->next) {
+    ++size;
+  }
+  return size;
+}
+
 template <typename Key, typename Value>
 std::vector<Key> GetOrderedKeys(const LruCache<Key, Value> &cache) {
   std::vector<Key> keys;
   keys.reserve(cache.Size());
-  for (auto *elem = cache.Head(); elem != nullptr; elem = elem->next) {
-    keys.push_back(elem->key);
+  for (const typename LruCache<Key, Value>::Element &elem : cache) {
+    keys.push_back(elem.key);
   }
   return keys;
 }
@@ -126,8 +138,10 @@ TEST(LruCacheTest, Clear) {
     cache.Insert(i, i);
   }
   EXPECT_THAT(GetOrderedKeys(cache), ElementsAre(2, 1, 0));
+  EXPECT_EQ(SizeOfFreeList(cache), 2);
   cache.Clear();
   EXPECT_EQ(cache.Size(), 0);
+  EXPECT_EQ(SizeOfFreeList(cache), 5);
 }
 
 TEST(LruCacheTest, LargeCapacity) {