Adding a previews IO-thread blacklist

components/previews/previews_black_list_item.cc

+// Copyright 2016 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.
+
+#include "components/previews/previews_black_list_item.h"
+
+#include <algorithm>
+
+#include "components/previews/previews_opt_out_store.h"
+
+namespace previews {
+
+PreviewsBlackListItem::PreviewsBlackListItem(
+    size_t stored_history_length,
+    int opt_out_black_list_threshold,
+    base::TimeDelta black_list_duration)
+    : stored_history_length_(stored_history_length),
+      opt_out_black_list_threshold_(opt_out_black_list_threshold),
+      black_list_duration_(black_list_duration),
+      total_opt_out_(0) {}
+
+PreviewsBlackListItem::~PreviewsBlackListItem() {}
+
+void PreviewsBlackListItem::AddPreviewNavigation(bool opt_out,
+                                                 base::Time entry_time) {
+  DCHECK_LE(opt_out_history_.size(), stored_history_length_);
+  if (!most_recent_opt_out_time_ ||
+      entry_time > most_recent_opt_out_time_.value()) {
+    most_recent_opt_out_time_ = entry_time;

[tbansal1, 2016/09/15 16:34:25]

I think you need to check if opt_out is true before updating
most_recent_opt_out_time_?

[RyanSturm, 2016/09/19 18:07:25]

Done.

+  }
+  total_opt_out_ += opt_out ? 1 : 0;
+
+  // Find insert postion to keep the list sorted. Typically, this will be at the
+  // end of the list, but for systems with clocks that are not non-decreasing
+  // with time, this may not be the end. Linearly search from end to begin.
+  auto iter = opt_out_history_.rbegin();
+  while (iter != opt_out_history_.rend() && iter->entry_time > entry_time) {

[tbansal1, 2016/09/15 16:34:25]

why not start from rend and go back? That should be faster, right?

[RyanSturm, 2016/09/19 18:07:25]

rend is the equivalent of begin, and rbeing is like end, reverse iterators are
weird.

+    iter++;
+  }
+  TimeOptOut time_opt_out;
+  time_opt_out.entry_time = entry_time;
+  time_opt_out.opt_out = opt_out;
+  opt_out_history_.insert(iter.base(), time_opt_out);
+
+  // Remove the oldest entry if the size exceeds the history size.
+  if (opt_out_history_.size() > stored_history_length_) {
+    total_opt_out_ -= opt_out_history_.front().opt_out ? 1 : 0;
+    opt_out_history_.pop_front();
+  }
+}
+
+bool PreviewsBlackListItem::IsBlackListed(base::Time now) const {

[tbansal1, 2016/09/15 16:34:25]

If the argument is always the current time, then why does it need to be passed
as a variable? Can |now| be anything significantly different than the current
time?

I am asking because if |now| is significantly different then current time, then
I think the checks below are not correct (and also meaningless).

[RyanSturm, 2016/09/19 18:07:25]

I want the same time to be passed to the store and the black list item, so
passing it in from there seems better. The time is still determined in the black
list as a whole, just not the black list item.

+  DCHECK_LE(opt_out_history_.size(), stored_history_length_);
+  return most_recent_opt_out_time_ &&
+         now - most_recent_opt_out_time_.value() < black_list_duration_ &&
+         total_opt_out_ >= opt_out_black_list_threshold_;
+}
+
+}  // namespace previews