Drive: Removes unused cache files after the initial feed fetch.

chrome/browser/chromeos/gdata/stale_cache_files_remover.cc

Index: chrome/browser/chromeos/gdata/stale_cache_files_remover.cc
diff --git a/chrome/browser/chromeos/gdata/stale_cache_files_remover.cc b/chrome/browser/chromeos/gdata/stale_cache_files_remover.cc
new file mode 100644
index 0000000000000000000000000000000000000000..7c0388d9a7fafb507399e5f5d48296317406810e
--- /dev/null
+++ b/chrome/browser/chromeos/gdata/stale_cache_files_remover.cc
@@ -0,0 +1,124 @@
+// 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 "chrome/browser/chromeos/gdata/stale_cache_files_remover.h"
+
+#include "base/bind.h"
+#include "base/message_loop.h"
+#include "chrome/browser/chromeos/gdata/gdata_cache.h"
+#include "chrome/browser/chromeos/gdata/gdata_file_system.h"
+#include "chrome/browser/chromeos/gdata/gdata.pb.h"
+#include "content/public/browser/browser_thread.h"
+
+using content::BrowserThread;
+
+namespace gdata {
+
+namespace {
+
+// Delay to scrub caches after the first feed fetch (in seconds).
+const int kDelaySeconds = 10;
+
+// Check the cache file and removes if it is unavailable or invalid (eg. md5
+// mismatch). This is called from GetCacheEntryAndRemoveCacheIfNecessary();
+void RemoveCacheIfNecessary(

[satorux1, 2012/08/14 17:19:37]

Please make it a private function of StaleCacheFilesRemover.  please see below
for why.

[yoshiki, 2012/08/16 07:39:56]

Done.

+    const std::string& resource_id,
+    gdata::GDataCache* cache,
+    const std::string& cache_md5,
+    GDataFileError error,
+    const FilePath& gdata_file_path,
+    scoped_ptr<gdata::GDataEntryProto> file_proto) {

[satorux1, 2012/08/14 17:19:37]

entry_proto

[yoshiki, 2012/08/16 07:39:56]

Done.

+  DCHECK(cache);
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  if (error != gdata::GDATA_FILE_OK ||
+      !file_proto.get() ||
+      file_proto->file_info().is_directory() ||
+      cache_md5 != file_proto->file_specific_info().file_md5()) {

[satorux1, 2012/08/14 17:19:37]

The condition looks complicated. Would be nice to separate:

// The entry is not found in the file system.
if (error != gdata::GDATA_FILE_OK) {
  cache->RemoveOnUIThread(resource_id, CacheOperationCallback());
  return;
}

// The entry is found but the MD5 does not match.
DCHECK(entry_proto.get());
if (cache_md5 != file_proto->file_specific_info().file_md5()) {
  cache->RemoveOnUIThread(resource_id, CacheOperationCallback());
  return;
}

[yoshiki, 2012/08/16 07:39:56]

Done.

+    cache->RemoveOnUIThread(resource_id, CacheOperationCallback());

[satorux1, 2012/08/14 17:19:37]

Instead of passing a null-callback, could you pass a simple callback like:

void EmitDebugLog(...) {
  DVLOG(1) << ...
}

base::Bind(&EmitDebugLog)

[yoshiki, 2012/08/16 07:39:56]

Done.

+    return;
+  }
+
+  // Otherwise, does nothing with the cache.

[satorux1, 2012/08/14 17:19:37]

don't need this.

[yoshiki, 2012/08/16 07:39:56]

Done.

+}
+
+// Gets the file entry and calls RemoveCacheIfNecessary() with the file entry.
+// This is called from StaleCacheFilesRemover::OnGetResourceIdsOfAllFiles().
+void GetEntryInfoAndRemoveCacheIfNecessary(

[satorux1, 2012/08/14 17:19:37]

Please make it a private function of StaleCacheFilesRemover. 

Rather than passing file_system_ and cache_ to this function, having this as a
private function would be cleaner.

[yoshiki, 2012/08/16 07:39:56]

Done.

+    gdata::GDataFileSystemInterface* file_system,
+    gdata::GDataCache* cache,
+    const std::string& resource_id,
+    bool success,
+    const gdata::GDataCacheEntry& cache_entry) {
+  DCHECK(file_system);
+  DCHECK(cache);
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  // Removes the chehe if GetCacheEntryOnUIThread() is failed.

[satorux1, 2012/08/14 17:19:37]

cache. If you use emacs, please use M-x ispell-comments-and-strings

[satorux1, 2012/08/14 17:19:37]

is failed -> failed

[yoshiki, 2012/08/16 07:39:56]

Done.

[yoshiki, 2012/08/16 07:39:56]

Done.

+  if (!success) {
+    cache->RemoveOnUIThread(resource_id, CacheOperationCallback());

[satorux1, 2012/08/14 17:19:37]

This probably won't work if GetCacheEntryOnUIThread() failed. Instead of this,
please add LOG(WARNING)

[yoshiki, 2012/08/16 07:39:56]

Done.

+    return;
+  }
+
+  file_system->GetEntryInfoByResourceId(resource_id,
+                                        base::Bind(&RemoveCacheIfNecessary,
+                                                   resource_id,
+                                                   cache,
+                                                   cache_entry.md5()));
+}
+
+}  // namespace
+
+StaleCacheFilesRemover::StaleCacheFilesRemover(
+    GDataFileSystemInterface* file_system,
+    GDataCache* cache)
+    : file_system_(file_system),
+      cache_(cache),
+      weak_ptr_factory_(this) {

[satorux1, 2012/08/14 17:19:37]

allow_this_in...

[yoshiki, 2012/08/16 07:39:56]

Done.

+}
+
+StaleCacheFilesRemover::~StaleCacheFilesRemover() {
+}
+
+// Sets the timer to start removing.

[satorux1, 2012/08/14 17:19:37]

please remove comment here.

[yoshiki, 2012/08/16 07:39:56]

Done.

+void StaleCacheFilesRemover::Start() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  base::MessageLoopProxy::current()->PostDelayedTask(
+      FROM_HERE,
+      base::Bind(&StaleCacheFilesRemover::RemoveSlateCacheFiles,
+                 weak_ptr_factory_.GetWeakPtr()),
+      base::TimeDelta::FromSeconds(kDelaySeconds));
+}
+
+// Gets the list of all the resource id and calls OnGetResourceIdsOfAllFiles()
+// with the list.

[satorux1, 2012/08/14 17:19:37]

please move this to .h file.

[yoshiki, 2012/08/16 07:39:56]

This method is removed

+void StaleCacheFilesRemover::RemoveSlateCacheFiles() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  const FilePath root_path = FilePath(gdata::kGDataRootDirectory);
+  cache_->GetResourceIdsOfAllFilesOnUIThread(
+      base::Bind(&StaleCacheFilesRemover::OnGetResourceIdsOfAllFiles,
+                 weak_ptr_factory_.GetWeakPtr()));
+}
+
+// Gets the cache entry of each resource id. And passes the cache entry to
+// GetEntryInfoAndRemoveCacheIfNecessary()

[satorux1, 2012/08/14 17:19:37]

please move this to .h file.

[yoshiki, 2012/08/16 07:39:56]

Done.

+void StaleCacheFilesRemover::OnGetResourceIdsOfAllFiles(
+    const std::vector<std::string>& resource_ids) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  for (size_t i = 0; i < resource_ids.size(); ++i) {
+    const std::string& resource_id = resource_ids[i];
+    cache_->GetCacheEntryOnUIThread(
+        resource_id,
+        "",  // Doesn't check MD5.

[satorux1, 2012/08/14 17:19:37]

// Don't check MD5.

[yoshiki, 2012/08/16 07:39:56]

Done.

+        base::Bind(&GetEntryInfoAndRemoveCacheIfNecessary,
+                   file_system_,
+                   cache_,
+                   resource_id));
+  }
+}
+
+}  // namespace gdata