Report Precache.TimeSinceLastPrecache

components/precache/core/precache_session_table.cc

Index: components/precache/core/precache_session_table.cc
diff --git a/components/precache/core/precache_session_table.cc b/components/precache/core/precache_session_table.cc
index 8537b199025d38aaa2a064d59fc60613989f581f..1edaa7d73710dd4edfb8ad5e8dd010745363a045 100644
--- a/components/precache/core/precache_session_table.cc
+++ b/components/precache/core/precache_session_table.cc
@@ -8,6 +8,7 @@
 
 #include "base/logging.h"
 #include "base/time/time.h"
+#include "components/precache/core/proto/timestamp.pb.h"
 #include "components/precache/core/proto/unfinished_work.pb.h"
 #include "sql/connection.h"
 #include "sql/statement.h"
@@ -27,6 +28,34 @@ bool PrecacheSessionTable::Init(sql::Connection* db) {
   return CreateTableIfNonExistent();
 }
 
+void PrecacheSessionTable::SetLastPrecacheTimestamp(const base::Time& time) {

[Raj, 2016/07/05 19:46:28]

The previous precache task start time is stored in java preference.
May be we could use that instead of this table. But a jni wrapper is needed to
call java code.

Also the unittests will be a little harder.

https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chr...

[jamartin, 2016/07/06 14:14:14]

What would be the benefit? If it is not clear, I'd rather not to complicate the
code and keep our data in the same place.

+  Timestamp timestamp;
+  timestamp.set_seconds(time.ToDoubleT());

[Raj, 2016/07/05 19:46:28]

There seems to be a conversion here ?
static_cast<int64>(time.ToDoubleT())

[jamartin, 2016/07/06 14:14:14]

Good catch.

I got lucky and it was working OK before because base::Time already does
saturated casts on int64 internally. However, I've added the conversions here to
make sure we abide to the API, not the internal implementation.

+  Statement statement(db_->GetCachedStatement(
+      SQL_FROM_HERE,
+      "INSERT OR REPLACE INTO precache_session (type, value) VALUES(?,?)"));
+  statement.BindInt(0, static_cast<int>(LAST_PRECACHE_TIMESTAMP));
+  statement.BindString(1, timestamp.SerializeAsString());

[Raj, 2016/07/05 19:46:28]

nit: Just wondering what is the need for timestamp proto.
Statement provides BindInt64() and BindDouble()

https://cs.chromium.org/chromium/src/sql/statement.h?rcl=0&l=108

[jamartin, 2016/07/06 14:14:14]

I had at least the following alternatives:
 a) Create an INTEGER column and store the seconds there. This would have
required to change the table schema, both creation and alter for existing users.
 b) Store the number as a string, without a proto. This works and is even less
code than my current approach.
 c) The current approach.

The current approach has the mild benefits that it is consistent with the other
type, it is easy to extend without altering the schema, and has data validation
for free.

If you don't feel strongly about this, I'd rather keep it as it is.

+  statement.Run();
+}
+
+base::Time PrecacheSessionTable::GetLastPrecacheTimestamp() {
+  Statement statement(db_->GetCachedStatement(
+      SQL_FROM_HERE, "SELECT value from precache_session where type=?"));
+  statement.BindInt(0, static_cast<int>(LAST_PRECACHE_TIMESTAMP));
+  Timestamp timestamp;
+  if (statement.Step())
+    timestamp.ParseFromString(statement.ColumnString(0));
+  return base::Time::FromDoubleT(timestamp.seconds());
+}
+
+void PrecacheSessionTable::DeleteLastPrecacheTimestamp() {
+  Statement statement(db_->GetCachedStatement(
+      SQL_FROM_HERE, "DELETE FROM precache_session where type=?"));
+  statement.BindInt(0, static_cast<int>(LAST_PRECACHE_TIMESTAMP));
+  statement.Run();
+}
+
 // Store unfinished work.
 void PrecacheSessionTable::SaveUnfinishedWork(
     std::unique_ptr<PrecacheUnfinishedWork> unfinished_work) {