Adding a previews IO-thread blacklist

components/previews/core/previews_black_list.cc

Index: components/previews/core/previews_black_list.cc
diff --git a/components/previews/core/previews_black_list.cc b/components/previews/core/previews_black_list.cc
new file mode 100644
index 0000000000000000000000000000000000000000..d050b4967aadb0ea0fd8046e07c9cb8d5260f06f
--- /dev/null
+++ b/components/previews/core/previews_black_list.cc
@@ -0,0 +1,137 @@
+// 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/core/previews_black_list.h"
+
+#include "base/bind.h"
+#include "base/memory/ptr_util.h"
+#include "base/time/clock.h"
+#include "base/time/time.h"
+#include "components/previews/core/previews_black_list_item.h"
+#include "components/previews/core/previews_experiments.h"
+#include "url/gurl.h"
+
+namespace previews {
+
+PreviewsBlackList::PreviewsBlackList(
+    std::unique_ptr<PreviewsOptOutStore> opt_out_store,
+    std::unique_ptr<base::Clock> clock)
+    : loaded_(false),
+      opt_out_store_(std::move(opt_out_store)),
+      clock_(std::move(clock)),
+      weak_factory_(this) {
+  if (opt_out_store_) {
+    opt_out_store_->LoadBlackList(base::Bind(
+        &PreviewsBlackList::LoadBlackListDone, weak_factory_.GetWeakPtr()));

[tbansal1, 2016/09/19 21:26:07]

Since opt_out_store_ is owned by |this|, is it possible to pass a raw ptr here
(and remove all weak ptr code).

[RyanSturm, 2016/09/19 22:37:45]

I don't think we should limit the OptOutStore implementation (currently coded
the OptOutStore could be deleted before this callback is called).

+  } else {
+    LoadBlackListDone(base::MakeUnique<BlackListItemMap>());
+  }
+}
+
+PreviewsBlackList::~PreviewsBlackList() {}
+
+void PreviewsBlackList::AddPreviewNavigation(const GURL& url,
+                                             bool opt_out,
+                                             PreviewsType type) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(url.has_host());
+  // If the |black_list_item_map_| has been loaded from |opt_out_store_|,
+  // synchronous operations will be accurate. Otherwise, queue the task to run
+  // asynchronously.
+  if (loaded_) {
+    AddPreviewNavigationSync(url, opt_out, type);
+  } else {
+    QueuePendingTask(base::Bind(&PreviewsBlackList::AddPreviewNavigationSync,
+                                base::Unretained(this), url, opt_out, type));
+  }
+}
+
+void PreviewsBlackList::AddPreviewNavigationSync(const GURL& url,
+                                                 bool opt_out,
+                                                 PreviewsType type) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(url.has_host());
+  DCHECK(loaded_);
+  std::string host_name = url.host();
+  base::Time now = clock_->Now();
+  GetBlackListItem(host_name, true)->AddPreviewNavigation(opt_out, now);
+  if (!opt_out_store_)
+    return;
+  opt_out_store_->AddPreviewNavigation(opt_out, host_name, type, now);

[tbansal1, 2016/09/19 21:26:08]

DCHECK_LE(black_list_item_map_->size(), params::MaxInMemoryHostsInBlackList());

[RyanSturm, 2016/09/19 22:37:45]

Done.

+}
+
+bool PreviewsBlackList::IsLoadedAndAllowed(const GURL& url, PreviewsType type) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(url.has_host());
+  std::string host_name = url.host();
+  if (!loaded_)
+    return false;
+  PreviewsBlackListItem* black_list_item = GetBlackListItem(host_name, false);
+  return !black_list_item || !black_list_item->IsBlackListed(clock_->Now());
+}
+
+void PreviewsBlackList::QueuePendingTask(QueueClosure callback) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(!callback.is_null());

[tbansal1, 2016/09/19 21:26:07]

DCHECK(!loaded_);

[RyanSturm, 2016/09/19 22:37:45]

Done.

+  pending_callbacks_.emplace(callback);
+}
+
+void PreviewsBlackList::LoadBlackListDone(
+    std::unique_ptr<BlackListItemMap> black_list_item_map) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  loaded_ = true;
+  black_list_item_map_ = std::move(black_list_item_map);
+
+  // Run all pending tasks. |loaded_| may change if ClearBlackListIs queued.

[tbansal1, 2016/09/19 21:26:07]

s/ClearBlackListIs/ClearBlackList is/

[RyanSturm, 2016/09/19 22:37:45]

Done.

+  while (pending_callbacks_.size() > 0 && loaded_) {
+    pending_callbacks_.front().Run();
+    pending_callbacks_.pop();
+  }
+}
+
+PreviewsBlackListItem* PreviewsBlackList::GetBlackListItem(
+    const std::string& host_name,
+    bool create_if_needed) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(loaded_);
+  BlackListItemMap::iterator iter = black_list_item_map_->find(host_name);
+  if (iter != black_list_item_map_->end()) {
+    return iter->second.get();
+  }
+  if (!create_if_needed)
+    return nullptr;
+  if (black_list_item_map_->size() >= params::MaxInMemoryHostsInBlackList()) {
+    EvictOldestOptOut();
+  }

[tbansal1, 2016/09/19 21:26:08]

DCHECK_LE(black_list_item_map_->size(), params::MaxInMemoryHostsInBlackList());

[RyanSturm, 2016/09/19 22:37:45]

Done.

+  // Create the item if it doesn't exist yet.
+  PreviewsBlackListItem* black_list_item = new PreviewsBlackListItem(
+      params::MaxStoredHistoryLengthForBlackList(),
+      params::BlackListOptOutThreshold(), params::BlackListDuration());
+  black_list_item_map_->operator[](host_name) =
+      base::WrapUnique(black_list_item);
+  return black_list_item;
+}
+
+void PreviewsBlackList::EvictOldestOptOut() {

[tbansal1, 2016/09/19 21:26:07]

I think the eviction should happen on the disk also (much more preferable), or
at the time of loading it must be ensured that at most
|params::MaxInMemoryHostsInBlackList()| hosts are read (less preferable).

Currently, the limit |params::MaxInMemoryHostsInBlackList()| is a soft limit,
and it is possible for program to read more than the threshold number of entries
in the memory.

[RyanSturm, 2016/09/19 22:37:45]

I'm planning to limit the SQL implementation to the same number of hosts.

+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(loaded_);
+  // TODO(ryansturm): Add UMA. crbug.com/647717
+  BlackListItemMap::iterator item_to_delete = black_list_item_map_->end();
+  base::Time oldest_opt_out = clock_->Now();
+  for (BlackListItemMap::iterator iter = black_list_item_map_->begin();
+       iter != black_list_item_map_->end(); ++iter) {
+    if (!iter->second->most_recent_opt_out_time()) {
+      // If there is no opt out time, this is a good choice to evict.
+      item_to_delete = iter;
+      break;
+    }
+    if (iter->second->most_recent_opt_out_time().value() < oldest_opt_out) {
+      oldest_opt_out = iter->second->most_recent_opt_out_time().value();
+      item_to_delete = iter;
+    }
+  }
+  black_list_item_map_->erase(item_to_delete);
+}
+
+}  // namespace previews