InstantExtended: Adding InstantRestrictedIDCache.

chrome/common/instant_restricted_id_cache.h

+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_COMMON_INSTANT_RESTRICTED_ID_CACHE_H_
+#define CHROME_COMMON_INSTANT_RESTRICTED_ID_CACHE_H_
+
+#include <algorithm>
+#include <deque>
+#include <vector>
+
+#include "base/gtest_prod_util.h"
+#include "base/logging.h"
+#include "chrome/common/instant_types.h"
+#include "content/public/common/page_transition_types.h"
+#include "googleurl/src/gurl.h"
+
+// T needs to be copyable.

[dhollowa, 2013/03/06 02:12:58]

nit: Please add documentation.

[Shishir, 2013/03/11 21:07:24]

Done.

+template <typename T>
+class InstantRestrictedIdCache {
+ public:
+  explicit InstantRestrictedIdCache(size_t max_cache_size);
+  ~InstantRestrictedIdCache();
+
+  // Adds items to the cache, assigning restricted ids in the process.

[dhollowa, 2013/03/06 02:12:58]

nit: s/ids/IDs

[Shishir, 2013/03/11 21:07:24]

Done.

+  void AddItems(const std::vector<T>& items);
+
+  // Returns the last set of items added to the cache.

[dhollowa, 2013/03/06 02:12:58]

Please mention that this set will change upon the next call to |AddItems|.

[Shishir, 2013/03/11 21:07:24]

Done.

+  void GetCurrentItems(std::vector<std::pair<size_t, T> >* items) const;

[dhollowa, 2013/03/06 02:12:58]

size_t seems wrong, it implies a size.  int or int64 maybe?  typedef int
RestrictedID; maybe?

[Shishir, 2013/03/11 21:07:24]

Done.

+
+  // Returns true if the |restricted_id| is present in the cache and if so,
+  // returns a copy of the item.
+  bool GetItemWithRestrictedId(size_t restricted_id, T* item) const;

[dhollowa, 2013/03/06 02:12:58]

Is this only from the "current" items?

[Shishir, 2013/03/11 21:07:24]

No. It can be any id. Top level comments should make this clearer.

+
+  // Clears the cache.
+  void Reset();
+
+  FRIEND_TEST_ALL_PREFIXES(InstantRestrictedIdCacheTest, AddAndFetchTest);
+  FRIEND_TEST_ALL_PREFIXES(InstantRestrictedIdCacheTest, EdgeCasesTest);
+
+  // Adds a single item to the cache.
+  void AddItem(const T& item);

[dhollowa, 2013/03/06 02:12:58]

This should be grouped with |AddItems|.

[Shishir, 2013/03/11 21:07:24]

Function does not exist.

+
+  std::deque<T> cache_;

[dhollowa, 2013/03/06 02:12:58]

These should be private.

[Shishir, 2013/03/11 21:07:24]

Done.

+  size_t results_base_;
+  size_t last_addition_size_;  // The number of items added last.
+  const size_t max_cache_size_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(InstantRestrictedIdCache);
+};
+
+template <typename T>
+InstantRestrictedIdCache<T>::InstantRestrictedIdCache(size_t max_cache_size)
+    : results_base_(0),
+      last_addition_size_(0),
+      max_cache_size_(max_cache_size) {
+  CHECK(max_cache_size_);

[dhollowa, 2013/03/06 02:12:58]

DCHECK since this is programmer error.

[Shishir, 2013/03/11 21:07:24]

Done.

+}
+
+template <typename T>
+InstantRestrictedIdCache<T>::~InstantRestrictedIdCache() {
+}
+
+template <typename T>
+void InstantRestrictedIdCache<T>::AddItems(const std::vector<T>& items) {
+  for (typename std::vector<T>::const_iterator it = items.begin();
+       it != items.end(); ++it) {
+    AddItem(*it);
+  }
+  last_addition_size_ = items.size();
+}
+
+template <typename T>
+void InstantRestrictedIdCache<T>::GetCurrentItems(
+    std::vector<std::pair<size_t, T> >* items) const {
+  DCHECK(items);

[dhollowa, 2013/03/06 02:12:58]

no need for DCHECK since next line will crash.

[Shishir, 2013/03/11 21:07:24]

Done.

+  items->clear();
+
+  if (last_addition_size_ == 0)
+    return;
+
+  size_t start = static_cast<size_t>(
+      std::max(0, static_cast<int>(cache_.size() - last_addition_size_)));

[dhollowa, 2013/03/06 02:12:58]

Seems like last_addition_size_ should never be greater than cache_.size(), no? 
And if not, then this should be DCHECK and not mutate with std::max.

[Shishir, 2013/03/11 21:07:24]

It can be greater. While retrieving we take care of it.

+  for (size_t i = start; i < cache_.size(); ++i) {
+    size_t restricted_id = results_base_ + i;
+    items->push_back(std::make_pair(restricted_id, cache_[i]));
+  }
+}
+
+template <typename T>
+bool InstantRestrictedIdCache<T>::GetItemWithRestrictedId(size_t restricted_id,
+                                                T* item) const {

[dhollowa, 2013/03/06 02:12:58]

nit: indent

[Shishir, 2013/03/11 21:07:24]

Done.

+  DCHECK(item);
+
+  if (restricted_id < results_base_ ||
+      restricted_id >= (results_base_ + cache_.size())) {
+    return false;
+  }
+
+  *item = cache_[restricted_id - results_base_];
+  return true;
+}
+
+template <typename T>
+void InstantRestrictedIdCache<T>::Reset() {
+  cache_.clear();
+  results_base_ = 0;
+  last_addition_size_ = 0;
+}
+
+template <typename T>
+void InstantRestrictedIdCache<T>::AddItem(const T& item) {
+  if (cache_.size() == max_cache_size_) {
+    cache_.pop_front();
+    ++results_base_;
+  }
+
+  cache_.push_back(item);
+}
+
+#endif  // CHROME_COMMON_INSTANT_RESTRICTED_ID_CACHE_H_