Precache tracking database

components/precache/core/precache_database.cc

+// 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.
+
+#include "components/precache/core/precache_database.h"
+
+#include <map>
+
+#include "base/metrics/histogram.h"
+#include "base/time/time.h"
+#include "components/precache/core/precache_statistics_table.h"
+#include "components/precache/core/precache_url_table.h"
+#include "sql/connection.h"
+#include "url/gurl.h"
+
+namespace precache {
+
+namespace {

[bengr, 2013/10/23 19:03:36]

Again, any reason your anonymous namespace is declared inside precache?

[sclittle, 2013/10/24 22:11:38]

Moved anon namespace out of precache namespace.

+
+// The number of days old that an entry in the precache URL table can be before
+// it is considered "old" and is removed from the table.
+const int64 kPrecacheHistoryExpiryPeriodDays = 60;
+
+int GetPercentageForUMA(int64 numerator, int64 denomenator) {

[bengr, 2013/10/23 19:03:36]

Does a function like this really not exist in chromium? I would rename as
GetInt64Percentage.

[sclittle, 2013/10/24 22:11:38]

On second thought, it doesn't really make sense to report a percentage at all if
the denomenator is zero, so I've removed this function and inlined the
calculation.

+  if (denomenator == 0) {
+    return 0;
+  }
+  return numerator * 100.0 / denomenator;
+}
+
+void RecordSingleDayPrecacheUMA(
+    const PrecacheStatisticsTable::PrecacheStatistics& stats) {
+  UMA_HISTOGRAM_COUNTS("Precache.DailyPrecachedKB",

[bengr, 2013/10/23 19:03:36]

You also need to describe these UMA in histograms.xml

[sclittle, 2013/10/24 22:11:38]

Done.

+                       stats.precached_bytes / 1000);

[bengr, 2013/10/23 19:03:36]

A KB is 1024 B. Why not report bytes instead? Also, double change the ranges for
UMA_HISTOGRAM_COUNTS

[sclittle, 2013/10/24 22:11:38]

Changed to report bytes with max_range at 1 billion.

+  UMA_HISTOGRAM_COUNTS("Precache.DailyDownloadedKB",
+                       stats.downloaded_bytes / 1000);
+  UMA_HISTOGRAM_COUNTS("Precache.DailyDownloadedKB.Cellular",
+                       stats.downloaded_bytes_cellular / 1000);
+  UMA_HISTOGRAM_COUNTS("Precache.DailySavedKB", stats.saved_bytes / 1000);
+  UMA_HISTOGRAM_COUNTS("Precache.DailySavedKB.Cellular",
+                       stats.saved_bytes_cellular / 1000);
+
+  UMA_HISTOGRAM_PERCENTAGE(
+      "Precache.DailySavingsPercentage",
+      GetPercentageForUMA(stats.saved_bytes,
+                          stats.saved_bytes + stats.downloaded_bytes));
+  UMA_HISTOGRAM_PERCENTAGE(
+      "Precache.DailySavingsPercentage.Cellular",
+      GetPercentageForUMA(
+          stats.saved_bytes_cellular,
+          stats.saved_bytes_cellular + stats.downloaded_bytes_cellular));
+}
+
+}  // namespace
+
+PrecacheDatabase::PrecacheDatabase()
+    : precache_url_table_(new PrecacheURLTable()),
+      precache_statistics_table_(new PrecacheStatisticsTable()) {
+  DetachFromThread();
+}
+
+PrecacheDatabase::~PrecacheDatabase() {
+  // Since the PrecacheDatabase is refcounted, it will only be deleted if there
+  // are no references remaining to it, meaning that it is not in use. Thus, it
+  // is save to delete it, regardless of what thread we are on.

[bengr, 2013/10/23 19:03:36]

is safe

[sclittle, 2013/10/24 22:11:38]

Done.

+  DetachFromThread();
+}
+
+void PrecacheDatabase::Init(sql::Connection* db) {
+  DCHECK(CalledOnValidThread());
+
+  DCHECK(!db_);  // Init must only be called once.
+  DCHECK(db);    // The database connection must be non-NULL.
+
+  db_.reset(db);
+  precache_url_table_->Init(db_.get());
+  precache_statistics_table_->Init(db_.get());
+}
+
+void PrecacheDatabase::FlushOldStats(base::Time flush_end_date) {
+  DCHECK(CalledOnValidThread());
+  DCHECK(db_);
+
+  // Delete old precache history that has expired.
+  precache_url_table_->DeleteAllPrecachedBetween(
+      base::Time::FromInternalValue(0),
+      flush_end_date -
+          base::TimeDelta::FromDays(kPrecacheHistoryExpiryPeriodDays));
+
+  std::map<base::Time, PrecacheStatisticsTable::PrecacheStatistics> stats_map;
+  precache_statistics_table_->GetAllStatsBetween(
+      base::Time::FromInternalValue(0), flush_end_date, &stats_map);
+
+  for (std::map<base::Time,
+                PrecacheStatisticsTable::PrecacheStatistics>::const_iterator
+           it = stats_map.begin();
+       it != stats_map.end(); ++it) {
+    RecordSingleDayPrecacheUMA(it->second);

[bengr, 2013/10/23 19:03:36]

I don't think we should record UMA for days where nothing was
fetched/prefetched.

[sclittle, 2013/10/24 22:11:38]

There won't be any rows in the statistics table for days when nothing was
fetched or precached. Added comment.

+  }
+
+  precache_statistics_table_->DeleteAllStatsBetween(
+      base::Time::FromInternalValue(0), flush_end_date);
+}
+
+void PrecacheDatabase::RecordURLFetched(GURL url, base::Time fetch_time,
+                                        int64 size, bool was_cached,
+                                        bool is_precaching, bool is_cellular) {
+  DCHECK(CalledOnValidThread());
+  DCHECK(db_);
+
+  if (is_precaching) {
+    if (was_cached && !precache_url_table_->HasURL(url)) {
+      // If a precache fetch came from the cache, and the user browsed the
+      // resource recently, then the precache fetch did nothing, so ignore it.
+      return;
+    }
+
+    if (!was_cached) {
+      PrecacheStatisticsTable::PrecacheStatistics stats;
+      stats.precached_bytes = size;
+      precache_statistics_table_->IncreaseStatsForFetch(fetch_time, stats);
+    }
+
+    precache_url_table_->AddURL(url, fetch_time);
+    return;
+  }
+
+  if (!was_cached) {
+    PrecacheStatisticsTable::PrecacheStatistics stats;
+    stats.downloaded_bytes = size;
+    if (is_cellular) {
+      stats.downloaded_bytes_cellular = size;
+    }
+    precache_statistics_table_->IncreaseStatsForFetch(fetch_time, stats);
+
+    // Since the fetch was over the network, delete any record of it having been
+    // prefetched from the URL table. If it had been precached, the resource
+    // must have expired.
+    precache_url_table_->DeleteURL(url);
+    return;
+  }
+
+  if (was_cached && precache_url_table_->HasURL(url)) {
+    PrecacheStatisticsTable::PrecacheStatistics stats;
+    stats.saved_bytes = size;
+    if (is_cellular) {
+      stats.saved_bytes_cellular = size;
+    }
+    precache_statistics_table_->IncreaseStatsForFetch(fetch_time, stats);
+
+    precache_url_table_->DeleteURL(url);
+  }
+}
+
+}  // namespace precache