Adding ClearBlackList to the PreviewsBlackList and plumbing to UI

This adds functionality to clear the previews black list on the IO
thread and the UI thread. Further this exposes other black list calls on
the UI thread.

BUG=639087

Review-Url: https://codereview.chromium.org/2387823002
Cr-Commit-Position: refs/heads/master@{#422926}

chrome/browser/browsing_data/browsing_data_remover.cc

@@ -37,6 +37,8 @@
 #include "chrome/browser/permissions/permission_decision_auto_blocker.h"
 #include "chrome/browser/prerender/prerender_manager.h"
 #include "chrome/browser/prerender/prerender_manager_factory.h"
+#include "chrome/browser/previews/previews_service.h"
+#include "chrome/browser/previews/previews_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
@@ -64,6 +66,7 @@
 #include "components/power/origin_power_map.h"
 #include "components/power/origin_power_map_factory.h"
 #include "components/prefs/pref_service.h"
+#include "components/previews/core/previews_ui_service.h"
 #include "components/search_engines/template_url_service.h"
 #include "components/sessions/core/tab_restore_service.h"
 #include "components/web_cache/browser/web_cache_manager.h"
@@ -722,6 +725,14 @@ void BrowsingDataRemover::RemoveImpl(
             ->DeleteBrowsingHistory(delete_begin_, delete_end_);
       }
     }
+
+    // |previews_service| is null if |profile_| is off the record.
+    PreviewsService* previews_service =
+        PreviewsServiceFactory::GetForProfile(profile_);
+    if (previews_service && previews_service->previews_ui_service()) {
+      previews_service->previews_ui_service()->ClearBlackList(delete_begin_,
+                                                              delete_end_);
+    }
   }
 
   if ((remove_mask & REMOVE_DOWNLOADS) && may_delete_history) {

chrome/browser/previews/previews_service_factory.cc

@@ -18,7 +18,6 @@ base::LazyInstance<PreviewsServiceFactory> g_previews_factory =
 
 // static
 PreviewsService* PreviewsServiceFactory::GetForProfile(Profile* profile) {
-  DCHECK_NE(profile->GetProfileType(), Profile::INCOGNITO_PROFILE);
   return static_cast<PreviewsService*>(
       GetInstance()->GetServiceForBrowserContext(profile, true));
 }

components/previews/core/previews_black_list.cc

@@ -78,6 +78,39 @@ bool PreviewsBlackList::IsLoadedAndAllowed(const GURL& url,
   return !black_list_item || !black_list_item->IsBlackListed(clock_->Now());
 }
 
+void PreviewsBlackList::ClearBlackList(base::Time begin_time,
+                                       base::Time end_time) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK_LE(begin_time, end_time);
+  // 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_) {
+    ClearBlackListSync(begin_time, end_time);
+  } else {
+    QueuePendingTask(base::Bind(&PreviewsBlackList::ClearBlackListSync,
+                                weak_factory_.GetWeakPtr(), begin_time,
+                                end_time));
+  }
+}
+
+void PreviewsBlackList::ClearBlackListSync(base::Time begin_time,
+                                           base::Time end_time) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(loaded_);
+  DCHECK_LE(begin_time, end_time);
+  black_list_item_map_.reset(nullptr);
+  loaded_ = false;
+  // Delete relevant entries and reload the blacklist into memory.
+  if (opt_out_store_) {
+    opt_out_store_->ClearBlackList(begin_time, end_time);
+    opt_out_store_->LoadBlackList(base::Bind(
+        &PreviewsBlackList::LoadBlackListDone, weak_factory_.GetWeakPtr()));
+  } else {
+    LoadBlackListDone(base::MakeUnique<BlackListItemMap>());
+  }
+}
+
 void PreviewsBlackList::QueuePendingTask(base::Closure callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!loaded_);

components/previews/core/previews_black_list.h

@@ -59,6 +59,12 @@ class PreviewsBlackList {
   // not used to make this decision.
   bool IsLoadedAndAllowed(const GURL& url, PreviewsType type) const;
 
+  // Asynchronously deletes all entries in the in-memory black list. Informs
+  // the backing store to delete entries between |begin_time| and |end_time|,
+  // and reloads entries into memory from the backing store. If the embedder
+  // passed in a null store, resets all history in the in-memory black list.
+  void ClearBlackList(base::Time begin_time, base::Time end_time);
+
  private:
   // Synchronous version of AddPreviewNavigation method.
   void AddPreviewNavigationSync(const GURL& host_name,
@@ -69,6 +75,9 @@ class PreviewsBlackList {
   // item for |host_name|, returns null.
   PreviewsBlackListItem* GetBlackListItem(const std::string& host_name) const;
 
+  // Synchronous version of ClearBlackList method.
+  void ClearBlackListSync(base::Time begin_time, base::Time end_time);
+
   // Returns a new PreviewsBlackListItem representing |host_name|. Adds the new
   // item to the in-memory map.
   PreviewsBlackListItem* CreateBlackListItem(const std::string& host_name);

components/previews/core/previews_black_list_unittest.cc

@@ -41,9 +41,11 @@ void RunLoadCallback(LoadBlackListCallback callback,
 
 class TestPreviewsOptOutStore : public PreviewsOptOutStore {
  public:
-  TestPreviewsOptOutStore() {}
+  TestPreviewsOptOutStore() : clear_blacklist_count_(0) {}
   ~TestPreviewsOptOutStore() override {}
 
+  int clear_blacklist_count() { return clear_blacklist_count_; }
+
  private:
   // PreviewsOptOutStore implementation:
   void AddPreviewNavigation(bool opt_out,
@@ -58,6 +60,12 @@ class TestPreviewsOptOutStore : public PreviewsOptOutStore {
         FROM_HERE, base::Bind(&RunLoadCallback, callback,
                               base::Passed(&black_list_item_map)));
   }
+
+  void ClearBlackList(base::Time begin_time, base::Time end_time) override {
+    ++clear_blacklist_count_;
+  }
+
+  int clear_blacklist_count_;
 };
 
 }  // namespace
@@ -79,7 +87,9 @@ TEST_F(PreviewsBlackListTest, BlackListNoStore) {
                                            params) &&
       base::FieldTrialList::CreateFieldTrial("ClientSidePreviews", "Enabled"));
 
-  base::Clock* test_clock = new base::SimpleTestClock();
+  base::SimpleTestClock* test_clock = new base::SimpleTestClock();
+  base::Time start = test_clock->Now();
+  test_clock->Advance(base::TimeDelta::FromSeconds(1));
 
   std::unique_ptr<PreviewsBlackList> black_list(
       new PreviewsBlackList(nullptr, base::WrapUnique(test_clock)));
@@ -106,6 +116,12 @@ TEST_F(PreviewsBlackListTest, BlackListNoStore) {
   EXPECT_FALSE(black_list->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE));
   EXPECT_TRUE(black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE));
 
+  test_clock->Advance(base::TimeDelta::FromSeconds(1));
+  black_list->ClearBlackList(start, test_clock->Now());
+
+  EXPECT_TRUE(black_list->IsLoadedAndAllowed(url_a, PreviewsType::OFFLINE));
+  EXPECT_TRUE(black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE));
+
   variations::testing::ClearAllVariationParams();
 }
 
@@ -129,11 +145,14 @@ TEST_F(PreviewsBlackListTest, BlackListWithStore) {
 
   base::MessageLoop loop;
 
-  base::Clock* test_clock = new base::SimpleTestClock();
+  base::SimpleTestClock* test_clock = new base::SimpleTestClock();
+  base::Time start = test_clock->Now();
+  test_clock->Advance(base::TimeDelta::FromSeconds(1));
+
+  TestPreviewsOptOutStore* opt_out_store = new TestPreviewsOptOutStore();
 
-  std::unique_ptr<PreviewsBlackList> black_list(
-      new PreviewsBlackList(base::MakeUnique<TestPreviewsOptOutStore>(),
-                            base::WrapUnique(test_clock)));
+  std::unique_ptr<PreviewsBlackList> black_list(new PreviewsBlackList(
+      base::WrapUnique(opt_out_store), base::WrapUnique(test_clock)));
 
   EXPECT_FALSE(black_list->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE));
   EXPECT_FALSE(black_list->IsLoadedAndAllowed(url_a2, PreviewsType::OFFLINE));
@@ -167,6 +186,22 @@ TEST_F(PreviewsBlackListTest, BlackListWithStore) {
   EXPECT_FALSE(black_list->IsLoadedAndAllowed(url_a2, PreviewsType::OFFLINE));
   EXPECT_TRUE(black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE));
 
+  test_clock->Advance(base::TimeDelta::FromSeconds(1));
+  EXPECT_EQ(0, opt_out_store->clear_blacklist_count());
+  black_list->ClearBlackList(start, base::Time::Now());
+  EXPECT_EQ(1, opt_out_store->clear_blacklist_count());
+
+  EXPECT_FALSE(black_list->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE));
+  EXPECT_FALSE(black_list->IsLoadedAndAllowed(url_a2, PreviewsType::OFFLINE));
+  EXPECT_FALSE(black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE));
+
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(1, opt_out_store->clear_blacklist_count());
+
+  EXPECT_TRUE(black_list->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE));
+  EXPECT_TRUE(black_list->IsLoadedAndAllowed(url_a1, PreviewsType::OFFLINE));
+  EXPECT_TRUE(black_list->IsLoadedAndAllowed(url_b, PreviewsType::OFFLINE));
+
   variations::testing::ClearAllVariationParams();
 }
 
@@ -174,6 +209,7 @@ TEST_F(PreviewsBlackListTest, QueueBehavior) {
   // Tests the black list asynchronous queue behavior. Methods called while
   // loading are queued and should run in the order they were queued.
   const GURL url("http://www.url.com");
+  const GURL url2("http://www.url2.com");
   const int duration_in_days = 365;
   base::FieldTrialList field_trial_list(nullptr);
   std::map<std::string, std::string> params;
@@ -188,11 +224,12 @@ TEST_F(PreviewsBlackListTest, QueueBehavior) {
   std::vector<bool> test_opt_out{true, false};
 
   for (auto opt_out : test_opt_out) {
-    base::Clock* test_clock = new base::SimpleTestClock();
+    base::SimpleTestClock* test_clock = new base::SimpleTestClock();
+
+    TestPreviewsOptOutStore* opt_out_store = new TestPreviewsOptOutStore();
 
-    std::unique_ptr<PreviewsBlackList> black_list(
-        new PreviewsBlackList(base::MakeUnique<TestPreviewsOptOutStore>(),
-                              base::WrapUnique(test_clock)));
+    std::unique_ptr<PreviewsBlackList> black_list(new PreviewsBlackList(
+        base::WrapUnique(opt_out_store), base::WrapUnique(test_clock)));
 
     EXPECT_FALSE(black_list->IsLoadedAndAllowed(url, PreviewsType::OFFLINE));
     black_list->AddPreviewNavigation(url, opt_out, PreviewsType::OFFLINE);
@@ -201,6 +238,24 @@ TEST_F(PreviewsBlackListTest, QueueBehavior) {
     base::RunLoop().RunUntilIdle();
     EXPECT_EQ(!opt_out,
               black_list->IsLoadedAndAllowed(url, PreviewsType::OFFLINE));
+
+    base::Time start = test_clock->Now();
+    test_clock->Advance(base::TimeDelta::FromSeconds(1));
+    black_list->AddPreviewNavigation(url, opt_out, PreviewsType::OFFLINE);
+    black_list->AddPreviewNavigation(url, opt_out, PreviewsType::OFFLINE);
+    test_clock->Advance(base::TimeDelta::FromSeconds(1));
+    EXPECT_EQ(0, opt_out_store->clear_blacklist_count());
+    black_list->ClearBlackList(
+        start, test_clock->Now() + base::TimeDelta::FromSeconds(1));
+    EXPECT_EQ(1, opt_out_store->clear_blacklist_count());
+    black_list->AddPreviewNavigation(url2, opt_out, PreviewsType::OFFLINE);
+    black_list->AddPreviewNavigation(url2, opt_out, PreviewsType::OFFLINE);
+    base::RunLoop().RunUntilIdle();
+    EXPECT_EQ(1, opt_out_store->clear_blacklist_count());
+
+    EXPECT_TRUE(black_list->IsLoadedAndAllowed(url, PreviewsType::OFFLINE));
+    EXPECT_EQ(!opt_out,
+              black_list->IsLoadedAndAllowed(url2, PreviewsType::OFFLINE));
   }
 
   variations::testing::ClearAllVariationParams();

components/previews/core/previews_io_data.cc

@@ -14,6 +14,7 @@
 #include "components/previews/core/previews_black_list.h"
 #include "components/previews/core/previews_opt_out_store.h"
 #include "components/previews/core/previews_ui_service.h"
+#include "url/gurl.h"
 
 namespace previews {
 
@@ -50,4 +51,17 @@ void PreviewsIOData::InitializeOnIOThread(
                             weak_factory_.GetWeakPtr()));
 }
 
+void PreviewsIOData::AddPreviewNavigation(const GURL& url,
+                                          bool opt_out,
+                                          PreviewsType type) {
+  DCHECK(io_task_runner_->BelongsToCurrentThread());
+  previews_black_list_->AddPreviewNavigation(url, opt_out, type);
+}
+
+void PreviewsIOData::ClearBlackList(base::Time begin_time,
+                                    base::Time end_time) {
+  DCHECK(io_task_runner_->BelongsToCurrentThread());
+  previews_black_list_->ClearBlackList(begin_time, end_time);
+}
+
 }  // namespace previews

components/previews/core/previews_io_data.h

@@ -12,8 +12,11 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "base/single_thread_task_runner.h"
+#include "base/time/time.h"
 #include "components/previews/core/previews_opt_out_store.h"
 
+class GURL;
+
 namespace previews {
 class PreviewsBlackList;
 class PreviewsUIService;
@@ -33,6 +36,14 @@ class PreviewsIOData {
   void Initialize(base::WeakPtr<PreviewsUIService> previews_ui_service,
                   std::unique_ptr<PreviewsOptOutStore> previews_opt_out_store);
 
+  // Adds a navigation to |url| to the black list with result |opt_out|.
+  void AddPreviewNavigation(const GURL& url, bool opt_out, PreviewsType type);
+
+  // Clears the history of the black list between |begin_time| and |end_time|,
+  // both inclusive.
+  void ClearBlackList(base::Time begin_time, base::Time end_time);
+
+  // The previews black list that decides whether a navigation can use previews.
   PreviewsBlackList* black_list() const { return previews_black_list_.get(); }
 
  protected:

components/previews/core/previews_opt_out_store.h

@@ -51,6 +51,9 @@ class PreviewsOptOutStore {
   // Asynchronously loads a map of host names to PreviewsBlackListItem for that
   // host from the store. And runs |callback| once loading is finished.
   virtual void LoadBlackList(LoadBlackListCallback callback) = 0;
+
+  // Deletes all history in the store between |begin_time| and |end_time|.
+  virtual void ClearBlackList(base::Time begin_time, base::Time end_time) = 0;
 };
 
 }  // namespace previews

components/previews/core/previews_ui_service.cc

@@ -7,6 +7,7 @@
 #include "base/bind.h"
 #include "base/single_thread_task_runner.h"
 #include "components/previews/core/previews_io_data.h"
+#include "url/gurl.h"
 
 namespace previews {
 
@@ -28,4 +29,21 @@ void PreviewsUIService::SetIOData(base::WeakPtr<PreviewsIOData> io_data) {
   io_data_ = io_data;
 }
 
+void PreviewsUIService::AddPreviewNavigation(const GURL& url,
+                                             PreviewsType type,
+                                             bool opt_out) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  io_task_runner_->PostTask(
+      FROM_HERE, base::Bind(&PreviewsIOData::AddPreviewNavigation, io_data_,
+                            url, opt_out, type));
+}
+
+void PreviewsUIService::ClearBlackList(base::Time begin_time,
+                                       base::Time end_time) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  io_task_runner_->PostTask(
+      FROM_HERE, base::Bind(&PreviewsIOData::ClearBlackList, io_data_,
+                            begin_time, end_time));
+}
+
 }  // namespace previews

components/previews/core/previews_ui_service.h

@@ -14,6 +14,8 @@
 #include "base/time/time.h"
 #include "components/previews/core/previews_opt_out_store.h"
 
+class GURL;
+
 namespace base {
 class SingleThreadTaskRunner;
 }
@@ -35,6 +37,12 @@ class PreviewsUIService {
   // thread. Virtualized in testing.
   virtual void SetIOData(base::WeakPtr<PreviewsIOData> io_data);
 
+  // Adds a navigation to |url| to the black list with result |opt_out|.
+  void AddPreviewNavigation(const GURL& url, PreviewsType type, bool opt_out);
+
+  // Clears the history of the black list between |begin_time| and |end_time|.
+  void ClearBlackList(base::Time begin_time, base::Time end_time);
+
  private:
   // The IO thread portion of the inter-thread communication for previews/.
   base::WeakPtr<previews::PreviewsIOData> io_data_;