Fixed accessed time for files on Google Drive. Now it is equal to last viewed time.
Before, we were setting a file's access time field to the last modified time. As a result access time and modification time was always equal. Now, on old api (gdata_wapi_parser.cc) we get the date from the 'lastView' field, while on new api (drive_api_parser.cc) from the 'lastViewedByMeDate' field.
BUG=148434
TEST=unit_tests
Review URL: https://chromiumcodereview.appspot.com/10990018
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@159195 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/chromeos/gdata/document_entry_conversion.cc b/chrome/browser/chromeos/gdata/document_entry_conversion.cc
index 242dfaa..68cab06 100644
--- a/chrome/browser/chromeos/gdata/document_entry_conversion.cc
+++ b/chrome/browser/chromeos/gdata/document_entry_conversion.cc
@@ -44,10 +44,10 @@
PlatformFileInfoProto* file_info = entry_proto.mutable_file_info();
- // TODO(satorux): The last modified and the last accessed time shouldn't
- // be treated as the same thing: crbug.com/148434
file_info->set_last_modified(doc.updated_time().ToInternalValue());
- file_info->set_last_accessed(doc.updated_time().ToInternalValue());
+ // If the file has never been viewed (last_viewed_time().is_null() == true),
+ // then we will set the last_accessed field in the protocol buffer to 0.
+ file_info->set_last_accessed(doc.last_viewed_time().ToInternalValue());
file_info->set_creation_time(doc.published_time().ToInternalValue());
if (doc.is_file() || doc.is_hosted_document()) {
diff --git a/chrome/browser/chromeos/gdata/document_entry_conversion_unittest.cc b/chrome/browser/chromeos/gdata/document_entry_conversion_unittest.cc
index e172668..4454656 100644
--- a/chrome/browser/chromeos/gdata/document_entry_conversion_unittest.cc
+++ b/chrome/browser/chromeos/gdata/document_entry_conversion_unittest.cc
@@ -51,8 +51,8 @@
const base::Time expected_time = base::Time::FromUTCExploded(exploded);
EXPECT_EQ(expected_time.ToInternalValue(),
entry_proto.file_info().last_modified());
- EXPECT_EQ(expected_time.ToInternalValue(),
- entry_proto.file_info().last_accessed());
+ // Last accessed value equal to 0 means that the file has never been viewed.
+ EXPECT_EQ(0, entry_proto.file_info().last_accessed());
EXPECT_EQ(expected_time.ToInternalValue(),
entry_proto.file_info().creation_time());
@@ -121,9 +121,21 @@
const base::Time expected_creation_time =
base::Time::FromUTCExploded(exploded);
+ // 2011-12-13T02:12:18.527Z
+ exploded.year = 2011;
+ exploded.month = 12;
+ exploded.day_of_month = 13;
+ exploded.day_of_week = 2; // Tuesday
+ exploded.hour = 2;
+ exploded.minute = 12;
+ exploded.second = 18;
+ exploded.millisecond = 527;
+ const base::Time expected_last_accessed_time =
+ base::Time::FromUTCExploded(exploded);
+
EXPECT_EQ(expected_last_modified_time.ToInternalValue(),
entry_proto.file_info().last_modified());
- EXPECT_EQ(expected_last_modified_time.ToInternalValue(),
+ EXPECT_EQ(expected_last_accessed_time.ToInternalValue(),
entry_proto.file_info().last_accessed());
EXPECT_EQ(expected_creation_time.ToInternalValue(),
entry_proto.file_info().creation_time());
@@ -191,9 +203,21 @@
const base::Time expected_creation_time =
base::Time::FromUTCExploded(exploded);
+ // 2011-11-02T04:37:38.469Z
+ exploded.year = 2011;
+ exploded.month = 11;
+ exploded.day_of_month = 2;
+ exploded.day_of_week = 2; // Tuesday
+ exploded.hour = 4;
+ exploded.minute = 37;
+ exploded.second = 38;
+ exploded.millisecond = 469;
+ const base::Time expected_last_accessed_time =
+ base::Time::FromUTCExploded(exploded);
+
EXPECT_EQ(expected_last_modified_time.ToInternalValue(),
entry_proto.file_info().last_modified());
- EXPECT_EQ(expected_last_modified_time.ToInternalValue(),
+ EXPECT_EQ(expected_last_accessed_time.ToInternalValue(),
entry_proto.file_info().last_accessed());
EXPECT_EQ(expected_creation_time.ToInternalValue(),
entry_proto.file_info().creation_time());
@@ -252,9 +276,21 @@
const base::Time expected_creation_time =
base::Time::FromUTCExploded(exploded);
+ // 2012-04-10T22:50:55.797Z
+ exploded.year = 2012;
+ exploded.month = 04;
+ exploded.day_of_month = 10;
+ exploded.day_of_week = 2; // Tuesday
+ exploded.hour = 22;
+ exploded.minute = 50;
+ exploded.second = 55;
+ exploded.millisecond = 797;
+ const base::Time expected_last_accessed_time =
+ base::Time::FromUTCExploded(exploded);
+
EXPECT_EQ(expected_last_modified_time.ToInternalValue(),
entry_proto.file_info().last_modified());
- EXPECT_EQ(expected_last_modified_time.ToInternalValue(),
+ EXPECT_EQ(expected_last_accessed_time.ToInternalValue(),
entry_proto.file_info().last_accessed());
EXPECT_EQ(expected_creation_time.ToInternalValue(),
entry_proto.file_info().creation_time());
diff --git a/chrome/browser/chromeos/gdata/drive_files.cc b/chrome/browser/chromeos/gdata/drive_files.cc
index c1838834..6e933f4e 100644
--- a/chrome/browser/chromeos/gdata/drive_files.cc
+++ b/chrome/browser/chromeos/gdata/drive_files.cc
@@ -45,10 +45,11 @@
// SetBaseNameFromTitle() must be called after |title_| is set.
SetBaseNameFromTitle();
- // TODO(satorux): The last modified and the last accessed time shouldn't
- // be treated as the same thing: crbug.com/148434
file_info_.last_modified = doc.updated_time();
- file_info_.last_accessed = doc.updated_time();
+ // If doc.last_viewed_time().is_null() then, we will pass 0 to the
+ // protocol buffer. Moreover, this value may be unreliable.
+ // See: crbug.com/152628.
+ file_info_.last_accessed = doc.last_viewed_time();
file_info_.creation_time = doc.published_time();
resource_id_ = doc.resource_id();
diff --git a/chrome/browser/google_apis/drive_api_parser.cc b/chrome/browser/google_apis/drive_api_parser.cc
index 050437d..f6f64b7 100644
--- a/chrome/browser/google_apis/drive_api_parser.cc
+++ b/chrome/browser/google_apis/drive_api_parser.cc
@@ -89,6 +89,7 @@
const char kMimeType[] = "mimeType";
const char kCreatedDate[] = "createdDate";
const char kModifiedByMeDate[] = "modifiedByMeDate";
+const char kLastViewedByMeDate[] = "lastViewedByMeDate";
const char kDownloadUrl[] = "downloadUrl";
const char kFileExtension[] = "fileExtension";
const char kMd5Checksum[] = "md5Checksum";
@@ -416,6 +417,10 @@
kModifiedByMeDate,
&FileResource::modified_by_me_date_,
&gdata::util::GetTimeFromString);
+ converter->RegisterCustomField<base::Time>(
+ kLastViewedByMeDate,
+ &FileResource::last_viewed_by_me_date_,
+ &gdata::util::GetTimeFromString);
converter->RegisterCustomField<GURL>(kDownloadUrl,
&FileResource::download_url_,
GetGURLFromString);
diff --git a/chrome/browser/google_apis/drive_api_parser.h b/chrome/browser/google_apis/drive_api_parser.h
index 02f7b55..41482dd0 100644
--- a/chrome/browser/google_apis/drive_api_parser.h
+++ b/chrome/browser/google_apis/drive_api_parser.h
@@ -388,6 +388,11 @@
// Returns modification time by the user.
const base::Time& modified_by_me_date() const { return modified_by_me_date_; }
+ // Returns last access time by the user.
+ const base::Time& last_viewed_by_me_date() const {
+ return last_viewed_by_me_date_;
+ }
+
// Returns the short-lived download URL for the file. This field exists
// only when the file content is stored in Drive.
const GURL& download_url() const { return download_url_; }
@@ -436,6 +441,7 @@
FileLabels labels_;
base::Time created_date_;
base::Time modified_by_me_date_;
+ base::Time last_viewed_by_me_date_;
GURL download_url_;
std::string file_extension_;
std::string md5_checksum_;
diff --git a/chrome/browser/google_apis/gdata_wapi_parser.cc b/chrome/browser/google_apis/gdata_wapi_parser.cc
index c52edfc..5888af2e 100644
--- a/chrome/browser/google_apis/gdata_wapi_parser.cc
+++ b/chrome/browser/google_apis/gdata_wapi_parser.cc
@@ -51,6 +51,7 @@
const char kFilenameNode[] = "filename";
const char kIDNode[] = "id";
const char kLastModifiedByNode[] = "lastModifiedBy";
+const char kLastViewedNode[] = "lastViewed";
const char kLinkNode[] = "link";
const char kMd5ChecksumNode[] = "md5Checksum";
const char kModifiedByMeDateNode[] = "modifiedByMeDate";
@@ -97,6 +98,7 @@
const char kItemsPerPageField[] = "openSearch$itemsPerPage.$t";
const char kLabelField[] = "label";
const char kLargestChangestampField[] = "docs$largestChangestamp.value";
+const char kLastViewedField[] = "gd$lastViewed.$t";
const char kLinkField[] = "link";
const char kMD5Field[] = "docs$md5Checksum.$t";
const char kNameField[] = "name.$t";
@@ -614,6 +616,9 @@
converter->RegisterCustomField<base::Time>(
kPublishedField, &DocumentEntry::published_time_,
&gdata::util::GetTimeFromString);
+ converter->RegisterCustomField<base::Time>(
+ kLastViewedField, &DocumentEntry::last_viewed_time_,
+ &gdata::util::GetTimeFromString);
converter->RegisterRepeatedMessage(
kFeedLinkField, &DocumentEntry::feed_links_);
converter->RegisterNestedField(kContentField, &DocumentEntry::content_);
@@ -840,6 +845,11 @@
if (xml_reader->ReadElementContent(&size))
base::StringToInt64(size, &entry->file_size_);
skip_read = true;
+ } else if (xml_reader->NodeName() == kLastViewedNode) {
+ std::string time;
+ if (xml_reader->ReadElementContent(&time))
+ gdata::util::GetTimeFromString(time, &entry->last_viewed_time_);
+ skip_read = true;
} else {
DVLOG(1) << "Unknown node " << xml_reader->NodeName();
}
@@ -911,6 +921,7 @@
}
// entry->categories_
entry->updated_time_ = file.modified_by_me_date();
+ entry->last_viewed_time_ = file.last_viewed_by_me_date();
entry->FillRemainingFields();
return entry.release();
diff --git a/chrome/browser/google_apis/gdata_wapi_parser.h b/chrome/browser/google_apis/gdata_wapi_parser.h
index 98363fa..05dc656e 100644
--- a/chrome/browser/google_apis/gdata_wapi_parser.h
+++ b/chrome/browser/google_apis/gdata_wapi_parser.h
@@ -401,6 +401,9 @@
// Document entry published time.
base::Time published_time() const { return published_time_; }
+ // Document entry last viewed time.
+ base::Time last_viewed_time() const { return last_viewed_time_; }
+
// List of document feed labels.
const std::vector<string16>& labels() const { return labels_; }
@@ -499,6 +502,8 @@
DriveEntryKind kind_;
string16 title_;
base::Time published_time_;
+ // Last viewed value may be unreliable. See: crbug.com/152628.
+ base::Time last_viewed_time_;
std::vector<string16> labels_;
Content content_;
ScopedVector<FeedLink> feed_links_;