[Thumbnails DB] Add functionality to clear unused on-demand favicons.

components/history/core/browser/expire_history_backend.cc

 // Copyright (c) 2012 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/history/core/browser/expire_history_backend.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <functional>
 #include <limits>
 
 #include "base/bind.h"
 #include "base/compiler_specific.h"
+#include "base/feature_list.h"
 #include "base/files/file_enumerator.h"
 #include "base/files/file_util.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/sequenced_task_runner.h"
 #include "components/history/core/browser/history_backend_client.h"
 #include "components/history/core/browser/history_backend_notifier.h"
 #include "components/history/core/browser/history_database.h"
 #include "components/history/core/browser/thumbnail_database.h"
 
@@ skipping 82 matching lines @@
 // last expiration found at least kNumExpirePerIteration and we want to check
 // again "soon."
 const int kExpirationDelaySec = 30;
 
 // The number of minutes between checking, as with kExpirationDelaySec, but
 // when we didn't find enough things to expire last time. If there was no
 // history to expire last iteration, it's likely there is nothing next
 // iteration, so we want to wait longer before checking to avoid wasting CPU.
 const int kExpirationEmptyDelayMin = 5;
 
+// The minimum number of hours between checking for old on-demand favicons that
+// should be cleared.
+const int kClearOnDemandFaviconsIntervalHours = 24;
+
+bool IsAnyURLBookmarked(HistoryBackendClient* backend_client,
+                        const std::vector<GURL>& urls) {
+  for (const GURL& url : urls) {
+    if (backend_client->IsBookmarked(url))
+      return true;
+  }
+  return false;
+}
+
 }  // namespace
 
+namespace internal {
+
+const base::Feature kClearOldOnDemandFavicons{
+    "ClearOldOnDemandFavicons", base::FEATURE_DISABLED_BY_DEFAULT};
+
+}  // namespace internal
 
 // ExpireHistoryBackend::DeleteEffects ----------------------------------------
 
 ExpireHistoryBackend::DeleteEffects::DeleteEffects() {
 }
 
 ExpireHistoryBackend::DeleteEffects::~DeleteEffects() {
 }
 
 
@@ skipping 337 matching lines @@
 }
 
 void ExpireHistoryBackend::DoExpireIteration() {
   DCHECK(!work_queue_.empty()) << "queue has to be non-empty";
 
   const ExpiringVisitsReader* reader = work_queue_.front();
   bool more_to_expire = ExpireSomeOldHistory(
       GetCurrentExpirationTime(), reader, kNumExpirePerIteration);
 
   work_queue_.pop();
-  // If there are more items to expire, add the reader back to the queue, thus
-  // creating a new task for future iterations.
-  if (more_to_expire)
+  if (more_to_expire) {
+    // If there are more items to expire, add the reader back to the queue, thus
+    // creating a new task for future iterations.
     work_queue_.push(reader);
+  } else {
+    // Otherwise do a final clean-up - remove old favicons not bound to visits.
+    ClearOldOnDemandFavicons(GetCurrentExpirationTime());
+  }
 
   ScheduleExpire();
 }
 
+void ExpireHistoryBackend::ClearOldOnDemandFavicons(
+    base::Time expiration_threshold) {
+  if (!base::FeatureList::IsEnabled(internal::kClearOldOnDemandFavicons))
+    return;
+
+  if (expiration_threshold <

[brettw, 2017/07/13 23:50:16]

Can you put a comment on this block about how this is avoiding running
expiration very often? It kind of looks like it might be a functional part of
the expiration logic.

[mastiz, 2017/07/18 07:48:19]

Done, but please double check because I didn't fully understand what you want to
reflect there: the *how* seems quite obvious from the code, did you mean *why*?

+      last_on_demand_expiration_threshold_ +
+          base::TimeDelta::FromHours(kClearOnDemandFaviconsIntervalHours)) {
+    return;
+  }
+
+  last_on_demand_expiration_threshold_ = expiration_threshold;
+
+  std::map<favicon_base::FaviconID, IconMappingsForExpiry> icon_mappings =
+      thumb_db_->GetOldOnDemandFavicons(expiration_threshold);
+  DeleteEffects effects;
+
+  for (auto id_and_mappings_pair : icon_mappings) {
+    favicon_base::FaviconID icon_id = id_and_mappings_pair.first;
+    const IconMappingsForExpiry& mappings = id_and_mappings_pair.second;
+
+    if (backend_client_ &&
+        IsAnyURLBookmarked(backend_client_, mappings.page_urls)) {
+      continue;
+    }
+
+    thumb_db_->DeleteFavicon(icon_id);
+    thumb_db_->DeleteIconMappingsForFaviconId(icon_id);
+    effects.deleted_favicons.insert(mappings.icon_url);
+  }
+
+  BroadcastNotifications(&effects, DELETION_EXPIRED);
+}
+
 bool ExpireHistoryBackend::ExpireSomeOldHistory(
     base::Time end_time,
     const ExpiringVisitsReader* reader,
     int max_visits) {
   if (!main_db_)
     return false;
 
   // Add an extra time unit to given end time, because
   // GetAllVisitsInRange, et al. queries' end value is non-inclusive.
   base::Time effective_end_time =
@@ skipping 11 matching lines @@
   BroadcastNotifications(&deleted_effects, DELETION_EXPIRED);
 
   return more_to_expire;
 }
 
 void ExpireHistoryBackend::ParanoidExpireHistory() {
   // TODO(brettw): Bug 1067331: write this to clean up any errors.
 }
 
 }  // namespace history