drive: prefetcher compares last modified time in addition to last access time.

BUG=157511


Review URL: https://chromiumcodereview.appspot.com/11360027

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@165375 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 49 insertions, 28 deletions

diff --git a/chrome/browser/chromeos/drive/drive_prefetcher.cc b/chrome/browser/chromeos/drive/drive_prefetcher.cc
index 8deace1..4e3623b2 100644
--- a/chrome/browser/chromeos/drive/drive_prefetcher.cc
+++ b/chrome/browser/chromeos/drive/drive_prefetcher.cc
@@ -8,7 +8,6 @@
 #include "base/command_line.h"
 #include "base/location.h"
 #include "base/message_loop_proxy.h"
-#include "chrome/browser/chromeos/drive/drive.pb.h"
 #include "chrome/browser/chromeos/drive/drive_file_system_interface.h"
 #include "chrome/browser/chromeos/drive/drive_file_system_util.h"
 #include "chrome/common/chrome_switches.h"
@@ -29,6 +28,24 @@ bool IsPrefetchDisabled() {
       switches::kDisableDrivePrefetch);
 }
 
+// Returns true if |left| has lower priority than |right|.
+bool ComparePrefetchPriority(const DriveEntryProto& left,
+                             const DriveEntryProto& right) {
+  // First, compare last access time. The older entry has less priority.
+  if (left.file_info().last_accessed() != right.file_info().last_accessed())
+    return left.file_info().last_accessed() < right.file_info().last_accessed();
+
+  // When the entries have the same last access time (which happens quite often
+  // because Drive server doesn't set the field until an entry is viewed via
+  // drive.google.com), we use last modified time as the tie breaker.
+  if (left.file_info().last_modified() != right.file_info().last_modified())
+    return left.file_info().last_modified() < right.file_info().last_modified();
+
+  // Two entries have the same priority. To make this function a valid
+  // comparator for std::set, we need to differentiate them anyhow.
+  return left.resource_id() < right.resource_id();
+}
+
 }
 
 DrivePrefetcherOptions::DrivePrefetcherOptions()
@@ -38,7 +55,8 @@ DrivePrefetcherOptions::DrivePrefetcherOptions()
 
 DrivePrefetcher::DrivePrefetcher(DriveFileSystemInterface* file_system,
                                  const DrivePrefetcherOptions& options)
-    : number_of_inflight_prefetches_(0),
+    : latest_files_(&ComparePrefetchPriority),
+      number_of_inflight_prefetches_(0),
       number_of_inflight_traversals_(0),
       should_suspend_prefetch_(true),
       initial_prefetch_count_(options.initial_prefetch_count),
@@ -131,22 +149,19 @@ void DrivePrefetcher::ReconstructQueue() {
   queue_.clear();
   for (LatestFileSet::reverse_iterator it = latest_files_.rbegin();
       it != latest_files_.rend(); ++it) {
-    queue_.push_back(it->second);
+    queue_.push_back(it->resource_id());
   }
 }
 
 void DrivePrefetcher::VisitFile(const DriveEntryProto& entry) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  int64 last_access = entry.file_info().last_accessed();
-  const std::string& resource_id = entry.resource_id();
-
   // Excessively large files will not be fetched.
   if (entry.file_info().size() > prefetch_file_size_limit_)
     return;
 
   // Remember the file in the set ordered by the |last_accessed| field.
-  latest_files_.insert(std::make_pair(last_access, resource_id));
+  latest_files_.insert(entry);
   // If the set become too big, forget the oldest entry.
   if (latest_files_.size() > static_cast<size_t>(initial_prefetch_count_))
     latest_files_.erase(latest_files_.begin());
diff --git a/chrome/browser/chromeos/drive/drive_prefetcher.h b/chrome/browser/chromeos/drive/drive_prefetcher.h
index 18365e2..8fe0b7c 100644
--- a/chrome/browser/chromeos/drive/drive_prefetcher.h
+++ b/chrome/browser/chromeos/drive/drive_prefetcher.h
@@ -12,6 +12,7 @@
 #include "base/basictypes.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
+#include "chrome/browser/chromeos/drive/drive.pb.h"
 #include "chrome/browser/chromeos/drive/drive_file_system_interface.h"
 #include "chrome/browser/chromeos/drive/drive_file_system_observer.h"
 #include "chrome/browser/chromeos/drive/drive_sync_client_observer.h"
@@ -21,7 +22,6 @@ class FilePath;
 
 namespace drive {
 
-class DriveEntryProto;
 class DriveFileSystemInterface;
 
 // The parameters for DrivePrefetcher construction.
@@ -79,7 +79,9 @@ class DrivePrefetcher : public DriveFileSystemObserver,
   void OnReadDirectoryFinished();
 
   // Keeps the kNumberOfLatestFilesToKeepInCache latest files in the filesystem.
-  typedef std::set<std::pair<int64, std::string> > LatestFileSet;
+  typedef bool (*PrefetchPriorityComparator)(const DriveEntryProto&,
+                                             const DriveEntryProto&);
+  typedef std::set<DriveEntryProto, PrefetchPriorityComparator> LatestFileSet;
   LatestFileSet latest_files_;
 
   // The queue of files to fetch. Files with higher priority comes front.
diff --git a/chrome/browser/chromeos/drive/drive_prefetcher_unittest.cc b/chrome/browser/chromeos/drive/drive_prefetcher_unittest.cc
index 4e55fca..b44ebbd 100644
--- a/chrome/browser/chromeos/drive/drive_prefetcher_unittest.cc
+++ b/chrome/browser/chromeos/drive/drive_prefetcher_unittest.cc
@@ -35,7 +35,8 @@ enum TestEntryType {
 struct TestEntry {
   const FilePath::CharType* path;
   TestEntryType entry_type;
-  int64 last_access;
+  int64 last_accessed;
+  int64 last_modified;
   const char* resource_id;
   int64 file_size;
 
@@ -55,7 +56,8 @@ struct TestEntry {
           entry_type == TYPE_HOSTED_FILE);
       entry.mutable_file_info()->set_size(file_size);
     }
-    entry.mutable_file_info()->set_last_accessed(last_access);
+    entry.mutable_file_info()->set_last_accessed(last_accessed);
+    entry.mutable_file_info()->set_last_modified(last_modified);
     entry.set_resource_id(resource_id);
     return entry;
   }
@@ -84,37 +86,39 @@ ACTION_P(MockReadDirectory, test_entries) {
 }
 
 const TestEntry kEmptyDrive[] = {
-  { FILE_PATH_LITERAL("drive"), TYPE_DIRECTORY, 0, "id:drive" },
+  { FILE_PATH_LITERAL("drive"), TYPE_DIRECTORY, 0, 0, "id:drive" },
 };
 
 const TestEntry kOneFileDrive[] = {
-  { FILE_PATH_LITERAL("drive"), TYPE_DIRECTORY, 0, "id:drive" },
-  { FILE_PATH_LITERAL("drive/abc.txt"), TYPE_REGULAR_FILE, 1, "id:abc" },
+  { FILE_PATH_LITERAL("drive"), TYPE_DIRECTORY, 0, 0, "id:drive" },
+  { FILE_PATH_LITERAL("drive/abc.txt"), TYPE_REGULAR_FILE, 1, 0, "id:abc" },
 };
 
 const char* kExpectedOneFile[] = { "id:abc" };
 
 const TestEntry kComplexDrive[] = {
-  { FILE_PATH_LITERAL("drive"),              TYPE_DIRECTORY,    0, "id:root" },
-  { FILE_PATH_LITERAL("drive/a"),            TYPE_DIRECTORY,    0, "id:a" },
-  { FILE_PATH_LITERAL("drive/a/foo.txt"),    TYPE_REGULAR_FILE, 3, "id:foo1" },
-  { FILE_PATH_LITERAL("drive/a/b"),          TYPE_DIRECTORY,    8, "id:b" },
-  { FILE_PATH_LITERAL("drive/a/b/bar.jpg"),  TYPE_REGULAR_FILE, 5, "id:bar1",
-                                             999 },
-  { FILE_PATH_LITERAL("drive/a/b/new.gdoc"), TYPE_HOSTED_FILE,  7, "id:new" },
-  { FILE_PATH_LITERAL("drive/a/buz.zip"),    TYPE_REGULAR_FILE, 4, "id:buz1" },
-  { FILE_PATH_LITERAL("drive/a/old.gdoc"),   TYPE_HOSTED_FILE,  1, "id:old" },
-  { FILE_PATH_LITERAL("drive/c"),            TYPE_DIRECTORY,    0, "id:c" },
-  { FILE_PATH_LITERAL("drive/c/foo.txt"),    TYPE_REGULAR_FILE, 2, "id:foo2" },
-  { FILE_PATH_LITERAL("drive/c/buz.zip"),    TYPE_REGULAR_FILE, 1, "id:buz2" },
-  { FILE_PATH_LITERAL("drive/bar.jpg"),      TYPE_REGULAR_FILE, 6, "id:bar2" },
+  // Path                                  Type           Access Modify   ID
+  { FILE_PATH_LITERAL("drive"),            TYPE_DIRECTORY,    0, 0, "id:root" },
+  { FILE_PATH_LITERAL("drive/a"),          TYPE_DIRECTORY,    0, 0, "id:a" },
+  { FILE_PATH_LITERAL("drive/a/foo.txt"),  TYPE_REGULAR_FILE, 3, 2, "id:foo1" },
+  { FILE_PATH_LITERAL("drive/a/b"),        TYPE_DIRECTORY,    8, 0, "id:b" },
+  { FILE_PATH_LITERAL("drive/a/bar.jpg"),  TYPE_REGULAR_FILE, 5, 0, "id:bar1",
+                                           999 },
+  { FILE_PATH_LITERAL("drive/a/b/x.gdoc"), TYPE_HOSTED_FILE,  7, 0, "id:new" },
+  { FILE_PATH_LITERAL("drive/a/buz.zip"),  TYPE_REGULAR_FILE, 4, 0, "id:buz1" },
+  { FILE_PATH_LITERAL("drive/a/old.gdoc"), TYPE_HOSTED_FILE,  1, 0, "id:old" },
+  { FILE_PATH_LITERAL("drive/c"),          TYPE_DIRECTORY,    0, 0, "id:c" },
+  { FILE_PATH_LITERAL("drive/c/foo.txt"),  TYPE_REGULAR_FILE, 3, 1, "id:foo2" },
+  { FILE_PATH_LITERAL("drive/c/buz.zip"),  TYPE_REGULAR_FILE, 1, 0, "id:buz2" },
+  { FILE_PATH_LITERAL("drive/bar.jpg"),    TYPE_REGULAR_FILE, 6, 0, "id:bar2" },
 };
 
 const char* kTop3Files[] = {
   "id:bar2",  // The file with the largest timestamp
               // "bar1" is the second latest, but its file size is over limit.
   "id:buz1",  // The third latest file.
-  "id:foo1"   // 4th.
+  "id:foo1"   // 4th. Has same access time with id:foo2, so the one with the
+              // newer modified time wins.
 };
 
 const char* kAllRegularFiles[] = {