Add LocalFileChangeTracker database to record non-synced dirty files

webkit/fileapi/syncable/local_file_change_tracker.cc

Index: webkit/fileapi/syncable/local_file_change_tracker.cc
diff --git a/webkit/fileapi/syncable/local_file_change_tracker.cc b/webkit/fileapi/syncable/local_file_change_tracker.cc
index 81e61a094b956a2dabd3a8c07f9b5ba2d0f31939..7ed60e09d550cf40c2956def852ff79bf51febca 100644
--- a/webkit/fileapi/syncable/local_file_change_tracker.cc
+++ b/webkit/fileapi/syncable/local_file_change_tracker.cc
@@ -4,20 +4,77 @@
 
 #include "webkit/fileapi/syncable/local_file_change_tracker.h"
 
+#include "base/location.h"
+#include "base/logging.h"
+#include "base/metrics/histogram.h"
 #include "base/sequenced_task_runner.h"
+#include "third_party/leveldatabase/src/include/leveldb/db.h"
 #include "webkit/fileapi/syncable/local_file_sync_status.h"
+#include "webkit/fileapi/file_system_util.h"
+
+namespace {
+
+const FilePath::CharType kDatabaseName[] =
+    FILE_PATH_LITERAL("LocalFileChangeTracker");
+const int64 kMinimumReportIntervalHours = 1;
+const char kInitStatusHistogramLabel[] =
+    "FileSystem.LocalFileChangeTracker.InitDatabase";
+const char kMark[] = "d";
+
+}
 
 namespace fileapi {
 
+// A database class that stores local file changes in a local database. This
+// object must be destructed on file_task_runner.
+class LocalFileChangeTracker::TrackerDB {
+ public:
+  TrackerDB(const FilePath& profile_path);
+
+  void MarkDirty(const FileSystemURL& url);
+  void ClearDirty(const FileSystemURL& url);
+
+ private:
+  enum InitStatus {
+    INIT_STATUS_OK = 0,
+    INIT_STATUS_CORRUPTION,
+    INIT_STATUS_IO_ERROR,
+    INIT_STATUS_UNKNOWN_ERROR,
+    INIT_STATUS_MAX,
+  };
+
+  enum RecoveryOption {
+    REPAIR_ON_CORRUPTION,
+    FAIL_ON_CORRUPTION,
+  };
+
+  bool Init(RecoveryOption recovery_option);
+  bool Repair(const std::string& db_path);
+  void HandleError(const tracked_objects::Location& from_here,
+                   const leveldb::Status& status);
+  void ReportInitStatus(const leveldb::Status& status);
+
+  const FilePath profile_path_;
+  scoped_ptr<leveldb::DB> db_;
+  base::Time last_reported_time_;
+  bool db_disabled_;
+
+  DISALLOW_COPY_AND_ASSIGN(TrackerDB);
+};
+
+// LocalFileChangeTracker ------------------------------------------------------
+
 LocalFileChangeTracker::LocalFileChangeTracker(
     LocalFileSyncStatus* sync_status,
+    const FilePath& profile_path,
     base::SequencedTaskRunner* file_task_runner)
     : sync_status_(sync_status),
-      file_task_runner_(file_task_runner) {}
+      file_task_runner_(file_task_runner),
+      tracker_db_(new TrackerDB(profile_path)) {}
 
 LocalFileChangeTracker::~LocalFileChangeTracker() {
-  // file_task_runner_->PostTask(FROM_HERE, base::Bind(&DropDatabase));
-  // TODO(kinuko): implement.
+  if (tracker_db_.get())
+    file_task_runner_->DeleteSoon(FROM_HERE, tracker_db_.release());
 }
 
 void LocalFileChangeTracker::OnStartUpdate(const FileSystemURL& url) {
@@ -107,11 +164,129 @@ void LocalFileChangeTracker::RecordChange(
 }
 
 void LocalFileChangeTracker::MarkDirtyOnDatabase(const FileSystemURL& url) {
-  // TODO(kinuko): implement.
+  tracker_db_->MarkDirty(url);
 }
 
 void LocalFileChangeTracker::ClearDirtyOnDatabase(const FileSystemURL& url) {
-  // TODO(kinuko): implement.
+  tracker_db_->ClearDirty(url);
+}
+
+// TrackerDB -------------------------------------------------------------------
+
+LocalFileChangeTracker::TrackerDB::TrackerDB(const FilePath& profile_path)
+  : profile_path_(profile_path),
+    db_disabled_(false) {}
+
+bool LocalFileChangeTracker::TrackerDB::Init(RecoveryOption recovery_option) {
+  if (db_.get())
+    return true;
+
+  std::string path = FilePathToString(profile_path_.Append(kDatabaseName));
+  leveldb::Options options;
+  options.create_if_missing = true;
+  leveldb::DB* db;
+  leveldb::Status status = leveldb::DB::Open(options, path, &db);
+  ReportInitStatus(status);
+  if (status.ok()) {
+    db_.reset(db);
+    return true;
+  }
+
+  HandleError(FROM_HERE, status);
+  if (!status.IsCorruption())
+    return false;
+
+  switch (recovery_option) {
+    case FAIL_ON_CORRUPTION:
+      return false;
+    case REPAIR_ON_CORRUPTION:
+      if (Repair(path))

[tzik, 2012/09/20 05:24:50]

can be just return Repair(path);

[nhiroki, 2012/09/20 09:30:33]

Done.

+        return true;
+      return false;
+  }
+  NOTREACHED();
+  return false;
+}
+
+bool LocalFileChangeTracker::TrackerDB::Repair(const std::string& db_path) {
+  DCHECK(!db_.get());
+  LOG(WARNING) << "Attempting to repair TrackerDB.";
+
+  if (leveldb::RepairDB(db_path, leveldb::Options()).ok() ||

[tzik, 2012/09/20 05:24:50]

We might lose some database entry on this method.
Can we perform some consistency check with DB to FileSystem?

Or, could you just leave some TODO comment to do it later.

[nhiroki, 2012/09/20 09:30:33]

For now, added TODO comments.

+      Init(FAIL_ON_CORRUPTION)) {
+    LOG(WARNING) << "Repairing TrackerDB completed.";
+    return true;
+  }
+
+  LOG(WARNING) << "Failed to repair TrackerDB.";
+  return false;
+}
+
+void LocalFileChangeTracker::TrackerDB::HandleError(
+    const tracked_objects::Location& from_here,
+    const leveldb::Status& status) {
+  LOG(ERROR) << "LocalFileChangeTracker::TrackerDB failed at: "
+             << from_here.ToString() << " with error: " << status.ToString();

[kinuko, 2012/09/20 05:09:11]

(Not directly related to this cl but would be nice to have these common methods
somewhere else...)

[nhiroki, 2012/09/20 09:30:33]

Added TODO comments.

+}
+
+void LocalFileChangeTracker::TrackerDB::ReportInitStatus(
+    const leveldb::Status& status) {
+  base::Time now = base::Time::Now();
+  base::TimeDelta minimum_interval =
+      base::TimeDelta::FromHours(kMinimumReportIntervalHours);
+  if (last_reported_time_ + minimum_interval >= now)
+    return;
+  last_reported_time_ = now;
+
+  if (status.ok()) {
+    UMA_HISTOGRAM_ENUMERATION(kInitStatusHistogramLabel,
+                              INIT_STATUS_OK, INIT_STATUS_MAX);
+  } else if (status.IsCorruption()) {
+    UMA_HISTOGRAM_ENUMERATION(kInitStatusHistogramLabel,
+                              INIT_STATUS_CORRUPTION, INIT_STATUS_OK);
+  } else if (status.IsIOError()) {
+    UMA_HISTOGRAM_ENUMERATION(kInitStatusHistogramLabel,
+                              INIT_STATUS_IO_ERROR, INIT_STATUS_MAX);
+  } else {
+    UMA_HISTOGRAM_ENUMERATION(kInitStatusHistogramLabel,
+                              INIT_STATUS_UNKNOWN_ERROR, INIT_STATUS_MAX);

[kinuko, 2012/09/20 05:09:11]

Adding UMA is great, but can we not add these in the initial commits?

[nhiroki, 2012/09/20 09:30:33]

Okay, removed it.

+  }
+}
+
+void LocalFileChangeTracker::TrackerDB::MarkDirty(const FileSystemURL& url) {

[kinuko, 2012/09/20 05:09:11]

Can we return success/fail here?

[nhiroki, 2012/09/20 09:30:33]

Done.

+  if (db_disabled_)
+    return;
+
+  if (!Init(REPAIR_ON_CORRUPTION)) {
+    db_disabled_ = true;
+    return;
+  }
+
+  leveldb::Status status = db_->Put(leveldb::WriteOptions(), url.spec(), kMark);

[kinuko, 2012/09/20 05:09:11]

I'm afraid Current spec() impl does not maintain all information FileSystemURL
has.  We'd probably need a safe serialization / deserialization method.

[nhiroki, 2012/09/20 09:30:33]

Added serialize/deserialize methods.

+  if (!status.ok()) {
+    HandleError(FROM_HERE, status);
+    db_disabled_ = true;
+    return;
+  }
+  return;
+}
+
+void LocalFileChangeTracker::TrackerDB::ClearDirty(const FileSystemURL& url) {

[kinuko, 2012/09/20 05:09:11]

Can we return success/fail here?

[nhiroki, 2012/09/20 09:30:33]

Done.

+  if (db_disabled_)
+    return;
+
+  if (!Init(REPAIR_ON_CORRUPTION)) {
+    db_disabled_ = true;
+    return;
+  }
+
+  leveldb::Status status = db_->Delete(leveldb::WriteOptions(), url.spec());
+  if (!status.ok() && !status.IsNotFound()) {
+    HandleError(FROM_HERE, status);
+    db_disabled_ = true;
+    return;
+  }
+  return;
 }
 
 }  // namespace fileapi