Add access count and time-since-accessed histograms to temp storage eviction.

storage/browser/quota/quota_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 "storage/browser/quota/quota_database.h"
 
 #include <vector>
 
 #include "base/auto_reset.h"
 #include "base/bind.h"
@@ skipping 339 matching lines @@
     statement.BindInt(1, static_cast<int>(type));
 
     if (!statement.Run())
       return false;
   }
 
   ScheduleCommit();
   return true;
 }
 
+bool QuotaDatabase::GetOriginInfo(const GURL& origin,
+                                  StorageType type,
+                                  QuotaDatabase::OriginInfoTableEntry* entry) {
+  if (!LazyOpen(false))
+    return false;
+
+  const char* kSql =
+      "SELECT * FROM OriginInfoTable"
+      " WHERE origin = ? AND type = ?";
+  sql::Statement statement(db_->GetCachedStatement(SQL_FROM_HERE, kSql));
+  statement.BindString(0, origin.spec());
+  statement.BindInt(1, static_cast<int>(type));
+
+  if (statement.Step()) {
+    *entry = OriginInfoTableEntry(
+        GURL(statement.ColumnString(0)),
+        static_cast<StorageType>(statement.ColumnInt(1)),
+        statement.ColumnInt(2),
+        base::Time::FromInternalValue(statement.ColumnInt64(3)),
+        base::Time::FromInternalValue(statement.ColumnInt64(4)));
+  }
+
+  return statement.Succeeded();

[michaeln, 2015/10/30 00:49:02]

Hmmm... this will return true when nothing was found.

There's precedent for methods that lookup a value in a particular row to return
false when there is no such row, see GetHostQuota() and FindOriginUsedCount().
It'd be good to keep the getters consistent about how NOTFOUND is handled. Wdyt
about adjusting the semantics of the return value to match those pre-existing
methods.

  if (!statement.Step())
    return false;

  // read the columns
  return true;


I think it'd be good to change recently added GetOriginLastEvictionTime() to
match too. (Sorry i didn't notice the difference when reviewing the CL that
added that new method.)

[calamity, 2015/11/02 06:27:44]

The return value indicating success/error is actually used by the QuotaManager
quite heavily to determine if it should disable the database. I'm not sure why
GetHostQuota is different but FindOriginUsedCount is private and used in
contexts where subsequent database failures will return false.

[michaeln, 2015/11/03 02:42:04]

The return value of the new GetOriginInfo method is not used in that way in the
CL. It appears to be used in a way that expects 'true' to mean that something
was found and then the values it returns are logged. I think consistency trumps
here and it looks like the return value indicating whether something was gotten
is prevalent for getters of specific values.

[calamity, 2015/11/04 00:42:43]

Done. Added some comments about return values.

+}
+
 bool QuotaDatabase::DeleteHostQuota(
     const std::string& host, StorageType type) {
   if (!LazyOpen(false))
     return false;
 
   const char* kSql =
       "DELETE FROM HostQuotaTable"
       " WHERE host = ? AND type = ?";
 
   sql::Statement statement(db_->GetCachedStatement(SQL_FROM_HERE, kSql));
@@ skipping 397 matching lines @@
   if (lhs.origin < rhs.origin) return true;
   if (rhs.origin < lhs.origin) return false;
   if (lhs.type < rhs.type) return true;
   if (rhs.type < lhs.type) return false;
   if (lhs.used_count < rhs.used_count) return true;
   if (rhs.used_count < lhs.used_count) return false;
   return lhs.last_access_time < rhs.last_access_time;
 }
 
 }  // namespace storage