chromeos: Stop returning scoped_ptr from GDataCache::GetCacheEntry

chrome/browser/chromeos/gdata/gdata_cache.cc

Index: chrome/browser/chromeos/gdata/gdata_cache.cc
diff --git a/chrome/browser/chromeos/gdata/gdata_cache.cc b/chrome/browser/chromeos/gdata/gdata_cache.cc
index 3d13b7c2012321bb187fda5517595380240a195c..6994e77ea6aedbc0cf0b1bb0dbce8aa9e29e3350 100644
--- a/chrome/browser/chromeos/gdata/gdata_cache.cc
+++ b/chrome/browser/chromeos/gdata/gdata_cache.cc
@@ -711,11 +711,12 @@ void GDataCache::RequestInitializeOnUIThread() {
       base::Bind(&GDataCache::Initialize, base::Unretained(this)));
 }
 
-scoped_ptr<GDataCache::CacheEntry> GDataCache::GetCacheEntry(
-    const std::string& resource_id,
-    const std::string& md5) {
+bool GDataCache::GetCacheEntry(const std::string& resource_id,
+                               const std::string& md5,
+                               CacheEntry* entry) {
+  DCHECK(entry);
   AssertOnSequencedWorkerPool();
-  return metadata_->GetCacheEntry(resource_id, md5);
+  return metadata_->GetCacheEntry(resource_id, md5, entry);
 }
 
 // static
@@ -780,13 +781,13 @@ void GDataCache::GetFile(const std::string& resource_id,
   DCHECK(error);
   DCHECK(cache_file_path);
 
-  scoped_ptr<CacheEntry> cache_entry = GetCacheEntry(
-      resource_id, md5);
-  if (cache_entry.get() && cache_entry->IsPresent()) {
+  CacheEntry cache_entry;
+  if (GetCacheEntry(resource_id, md5, &cache_entry) &&
+      cache_entry.IsPresent()) {
     CachedFileOrigin file_origin;
-    if (cache_entry->IsMounted()) {
+    if (cache_entry.IsMounted()) {
       file_origin = CACHED_FILE_MOUNTED;
-    } else if (cache_entry->IsDirty()) {
+    } else if (cache_entry.IsDirty()) {
       file_origin = CACHED_FILE_LOCALLY_MODIFIED;
     } else {
       file_origin = CACHED_FILE_FROM_SERVER;
@@ -794,7 +795,7 @@ void GDataCache::GetFile(const std::string& resource_id,
     *cache_file_path = GetCacheFilePath(
         resource_id,
         md5,
-        cache_entry->GetSubDirectoryType(),
+        cache_entry.GetSubDirectoryType(),
         file_origin);
     *error = base::PLATFORM_FILE_OK;
   } else {
@@ -815,25 +816,25 @@ void GDataCache::Store(const std::string& resource_id,
   int cache_state = CACHE_STATE_PRESENT;
   CacheSubDirectoryType sub_dir_type = CACHE_TYPE_TMP;
 
-  scoped_ptr<CacheEntry> cache_entry = GetCacheEntry(resource_id, md5);
+  CacheEntry cache_entry;

[achuithb, 2012/07/11 19:09:59]

nit: Can we move this declaration to right before GetCacheEntry?

[hashimoto, 2012/07/12 05:15:19]

Done.

 
   // If file was previously pinned, store it in persistent dir and create
   // symlink in pinned dir.
-  if (cache_entry.get()) {  // File exists in cache.
+  if (GetCacheEntry(resource_id, md5, &cache_entry)) {  // File exists in cache.
     // If file is dirty or mounted, return error.
-    if (cache_entry->IsDirty() || cache_entry->IsMounted()) {
+    if (cache_entry.IsDirty() || cache_entry.IsMounted()) {
       LOG(WARNING) << "Can't store a file to replace a "
-                   << (cache_entry->IsDirty() ? "dirty" : "mounted")
+                   << (cache_entry.IsDirty() ? "dirty" : "mounted")
                    << " file: res_id=" << resource_id
                    << ", md5=" << md5;
       *error = base::PLATFORM_FILE_ERROR_IN_USE;
       return;
     }
 
-    cache_state |= cache_entry->cache_state;
+    cache_state |= cache_entry.cache_state;
 
     // If file is pinned, determines destination path.
-    if (cache_entry->IsPinned()) {
+    if (cache_entry.IsPinned()) {
       sub_dir_type = CACHE_TYPE_PERSISTENT;
       dest_path = GetCacheFilePath(resource_id, md5, sub_dir_type,
                                    CACHED_FILE_FROM_SERVER);
@@ -901,9 +902,10 @@ void GDataCache::Pin(const std::string& resource_id,
   int cache_state = CACHE_STATE_PINNED;
   CacheSubDirectoryType sub_dir_type = CACHE_TYPE_PERSISTENT;
 
-  scoped_ptr<CacheEntry> cache_entry = GetCacheEntry(resource_id, md5);
+  CacheEntry cache_entry;
 

[achuithb, 2012/07/11 19:09:59]

nit: Don't think this newline is necessary.

[hashimoto, 2012/07/12 05:15:19]

Done.

-  if (!cache_entry.get()) {  // Entry does not exist in cache.
+  if (!GetCacheEntry(resource_id, md5, &cache_entry)) {
+    // Entry does not exist in cache.
     // Set both |dest_path| and |source_path| to /dev/null, so that:
     // 1) ModifyCacheState won't move files when |source_path| and |dest_path|
     //    are the same.
@@ -916,25 +918,25 @@ void GDataCache::Pin(const std::string& resource_id,
     // then moved to 'persistent'.
     sub_dir_type = CACHE_TYPE_TMP;
   } else {  // File exists in cache, determines destination path.
-    cache_state |= cache_entry->cache_state;
+    cache_state |= cache_entry.cache_state;
 
     // Determine source and destination paths.
 
     // If file is dirty or mounted, don't move it, so determine |dest_path| and
     // set |source_path| the same, because ModifyCacheState only moves files if
     // source and destination are different.
-    if (cache_entry->IsDirty() || cache_entry->IsMounted()) {
-      DCHECK(cache_entry->IsPersistent());
+    if (cache_entry.IsDirty() || cache_entry.IsMounted()) {
+      DCHECK(cache_entry.IsPersistent());
       dest_path = GetCacheFilePath(resource_id,
                                    md5,
-                                   cache_entry->GetSubDirectoryType(),
+                                   cache_entry.GetSubDirectoryType(),
                                    CACHED_FILE_LOCALLY_MODIFIED);
       source_path = dest_path;
     } else {
       // Gets the current path of the file in cache.
       source_path = GetCacheFilePath(resource_id,
                                      md5,
-                                     cache_entry->GetSubDirectoryType(),
+                                     cache_entry.GetSubDirectoryType(),
                                      CACHED_FILE_FROM_SERVER);
 
       // If file was pinned before but actual file blob doesn't exist in cache:
@@ -942,7 +944,7 @@ void GDataCache::Pin(const std::string& resource_id,
       //   because ModifyCacheState only moves files if source and destination
       //   are different
       // - don't create symlink since it already exists.
-      if (!cache_entry->IsPresent()) {
+      if (!cache_entry.IsPresent()) {
         dest_path = source_path;
         create_symlink = false;
       } else {  // File exists, move it to persistent dir.
@@ -984,10 +986,10 @@ void GDataCache::Unpin(const std::string& resource_id,
   AssertOnSequencedWorkerPool();
   DCHECK(error);
 
-  scoped_ptr<CacheEntry> cache_entry = GetCacheEntry(resource_id, md5);
+  CacheEntry cache_entry;
 

[achuithb, 2012/07/11 19:09:59]

nit: Move this to right before GetCacheEntry.

[hashimoto, 2012/07/12 05:15:19]

Done.

   // Unpinning a file means its entry must exist in cache.
-  if (!cache_entry.get()) {
+  if (!GetCacheEntry(resource_id, md5, &cache_entry)) {
     LOG(WARNING) << "Can't unpin a file that wasn't pinned or cached: res_id="
                  << resource_id
                  << ", md5=" << md5;
@@ -1004,26 +1006,26 @@ void GDataCache::Unpin(const std::string& resource_id,
   // If file is dirty or mounted, don't move it, so determine |dest_path| and
   // set |source_path| the same, because ModifyCacheState moves files if source
   // and destination are different.
-  if (cache_entry->IsDirty() || cache_entry->IsMounted()) {
+  if (cache_entry.IsDirty() || cache_entry.IsMounted()) {
     sub_dir_type = CACHE_TYPE_PERSISTENT;
-    DCHECK(cache_entry->IsPersistent());
+    DCHECK(cache_entry.IsPersistent());
     dest_path = GetCacheFilePath(resource_id,
                                  md5,
-                                 cache_entry->GetSubDirectoryType(),
+                                 cache_entry.GetSubDirectoryType(),
                                  CACHED_FILE_LOCALLY_MODIFIED);
     source_path = dest_path;
   } else {
     // Gets the current path of the file in cache.
     source_path = GetCacheFilePath(resource_id,
                                    md5,
-                                   cache_entry->GetSubDirectoryType(),
+                                   cache_entry.GetSubDirectoryType(),
                                    CACHED_FILE_FROM_SERVER);
 
     // If file was pinned but actual file blob still doesn't exist in cache,
     // don't need to move the file, so set |dest_path| to |source_path|, because
     // ModifyCacheState only moves files if source and destination are
     // different.
-    if (!cache_entry->IsPresent()) {
+    if (!cache_entry.IsPresent()) {
       dest_path = source_path;
     } else {  // File exists, move it to tmp dir.
       dest_path = GetCacheFilePath(resource_id, md5,
@@ -1035,7 +1037,7 @@ void GDataCache::Unpin(const std::string& resource_id,
   // If file was pinned, get absolute path of symlink in pinned dir so as to
   // remove it.
   FilePath symlink_path;
-  if (cache_entry->IsPinned()) {
+  if (cache_entry.IsPinned()) {
     symlink_path = GetCacheFilePath(resource_id,
                                     std::string(),
                                     CACHE_TYPE_PINNED,
@@ -1051,7 +1053,7 @@ void GDataCache::Unpin(const std::string& resource_id,
 
   if (*error == base::PLATFORM_FILE_OK) {
     // Now that file operations have completed, update cache map.
-    int cache_state = ClearCachePinned(cache_entry->cache_state);
+    int cache_state = ClearCachePinned(cache_entry.cache_state);
     UpdateCacheWithSubDirectoryType(resource_id,
                                     md5,
                                     sub_dir_type,
@@ -1076,20 +1078,19 @@ void GDataCache::SetMountedState(const FilePath& file_path,
   DCHECK(!to_mount == (extra_extension == util::kMountedArchiveFileExtension));
 
   // Get cache entry associated with the resource_id and md5
-  scoped_ptr<CacheEntry> cache_entry = GetCacheEntry(
-      resource_id, md5);
-  if (!cache_entry.get()) {
+  CacheEntry cache_entry;
+  if (!GetCacheEntry(resource_id, md5, &cache_entry)) {
     *error = base::PLATFORM_FILE_ERROR_NOT_FOUND;
     return;
   }
-  if (to_mount == cache_entry->IsMounted()) {
+  if (to_mount == cache_entry.IsMounted()) {
     *error = base::PLATFORM_FILE_ERROR_INVALID_OPERATION;
     return;
   }
 
   // Get the subdir type and path for the unmounted state.
   CacheSubDirectoryType unmounted_subdir =
-      cache_entry->IsPinned() ? CACHE_TYPE_PERSISTENT : CACHE_TYPE_TMP;
+      cache_entry.IsPinned() ? CACHE_TYPE_PERSISTENT : CACHE_TYPE_TMP;
   FilePath unmounted_path = GetCacheFilePath(
       resource_id, md5, unmounted_subdir, CACHED_FILE_FROM_SERVER);
 
@@ -1101,7 +1102,7 @@ void GDataCache::SetMountedState(const FilePath& file_path,
   // Determine the source and destination paths for moving the cache blob.
   FilePath source_path;
   CacheSubDirectoryType dest_subdir;
-  int cache_state = cache_entry->cache_state;
+  int cache_state = cache_entry.cache_state;
   if (to_mount) {
     source_path = unmounted_path;
     *cache_file_path = mounted_path;
@@ -1139,12 +1140,12 @@ void GDataCache::MarkDirty(const std::string& resource_id,
   // would have lost the md5 info during cache initialization, because the file
   // would have been renamed to .local extension.
   // So, search for entry in cache without comparing md5.
-  scoped_ptr<CacheEntry> cache_entry =
-      GetCacheEntry(resource_id, std::string());
+  CacheEntry cache_entry;

[achuithb, 2012/07/11 19:09:59]

same.

[hashimoto, 2012/07/12 05:15:19]

Done.

 
   // Marking a file dirty means its entry and actual file blob must exist in
   // cache.
-  if (!cache_entry.get() || !cache_entry->IsPresent()) {
+  if (!GetCacheEntry(resource_id, std::string(), &cache_entry) ||
+      !cache_entry.IsPresent()) {
     LOG(WARNING) << "Can't mark dirty a file that wasn't cached: res_id="
                  << resource_id
                  << ", md5=" << md5;
@@ -1158,9 +1159,9 @@ void GDataCache::MarkDirty(const std::string& resource_id,
   // not being uploaded.  However, for now, cache doesn't know if uploading of a
   // file is in progress.  Per zel, the upload process should be canceled before
   // MarkDirtyInCache is called again.
-  if (cache_entry->IsDirty()) {
+  if (cache_entry.IsDirty()) {
     // The file must be in persistent dir.
-    DCHECK(cache_entry->IsPersistent());
+    DCHECK(cache_entry.IsPersistent());
 
     // Determine symlink path in outgoing dir, so as to remove it.
     FilePath symlink_path = GetCacheFilePath(
@@ -1196,7 +1197,7 @@ void GDataCache::MarkDirty(const std::string& resource_id,
   FilePath source_path = GetCacheFilePath(
       resource_id,
       md5,
-      cache_entry->GetSubDirectoryType(),
+      cache_entry.GetSubDirectoryType(),
       CACHED_FILE_FROM_SERVER);
 
   // Determine destination path.
@@ -1208,7 +1209,7 @@ void GDataCache::MarkDirty(const std::string& resource_id,
 
   // If file is pinned, update symlink in pinned dir.
   FilePath symlink_path;
-  if (cache_entry->IsPinned()) {
+  if (cache_entry.IsPinned()) {
     symlink_path = GetCacheFilePath(resource_id,
                                     std::string(),
                                     CACHE_TYPE_PINNED,
@@ -1224,7 +1225,7 @@ void GDataCache::MarkDirty(const std::string& resource_id,
 
   if (*error == base::PLATFORM_FILE_OK) {
     // Now that file operations have completed, update cache map.
-    int cache_state = SetCacheDirty(cache_entry->cache_state);
+    int cache_state = SetCacheDirty(cache_entry.cache_state);
     UpdateCacheWithSubDirectoryType(resource_id,
                                     md5,
                                     sub_dir_type,
@@ -1243,12 +1244,12 @@ void GDataCache::CommitDirty(const std::string& resource_id,
   // would have lost the md5 info during cache initialization, because the file
   // would have been renamed to .local extension.
   // So, search for entry in cache without comparing md5.
-  scoped_ptr<CacheEntry> cache_entry =
-      GetCacheEntry(resource_id, std::string());
+  CacheEntry cache_entry;

[achuithb, 2012/07/11 19:09:59]

same

[hashimoto, 2012/07/12 05:15:19]

Done.

 
   // Committing a file dirty means its entry and actual file blob must exist in
   // cache.
-  if (!cache_entry.get() || !cache_entry->IsPresent()) {
+  if (!GetCacheEntry(resource_id, std::string(), &cache_entry) ||
+      !cache_entry.IsPresent()) {
     LOG(WARNING) << "Can't commit dirty a file that wasn't cached: res_id="
                  << resource_id
                  << ", md5=" << md5;
@@ -1258,7 +1259,7 @@ void GDataCache::CommitDirty(const std::string& resource_id,
 
   // If a file is not dirty (it should have been marked dirty via
   // MarkDirtyInCache), committing it dirty is an invalid operation.
-  if (!cache_entry->IsDirty()) {
+  if (!cache_entry.IsDirty()) {
     LOG(WARNING) << "Can't commit a non-dirty file: res_id="
                  << resource_id
                  << ", md5=" << md5;
@@ -1267,7 +1268,7 @@ void GDataCache::CommitDirty(const std::string& resource_id,
   }
 
   // Dirty files must be in persistent dir.
-  DCHECK(cache_entry->IsPersistent());
+  DCHECK(cache_entry.IsPersistent());
 
   // Create symlink in outgoing dir.
   FilePath symlink_path = GetCacheFilePath(resource_id,
@@ -1278,7 +1279,7 @@ void GDataCache::CommitDirty(const std::string& resource_id,
   // Get target path of symlink i.e. current path of the file in cache.
   FilePath target_path = GetCacheFilePath(resource_id,
                                           md5,
-                                          cache_entry->GetSubDirectoryType(),
+                                          cache_entry.GetSubDirectoryType(),
                                           CACHED_FILE_LOCALLY_MODIFIED);
 
   // Since there's no need to move files, use |target_path| for both
@@ -1300,12 +1301,12 @@ void GDataCache::ClearDirty(const std::string& resource_id,
 
   // |md5| is the new .<md5> extension to rename the file to.
   // So, search for entry in cache without comparing md5.
-  scoped_ptr<CacheEntry> cache_entry =
-      GetCacheEntry(resource_id, std::string());
+  CacheEntry cache_entry;
 
   // Clearing a dirty file means its entry and actual file blob must exist in
   // cache.
-  if (!cache_entry.get() || !cache_entry->IsPresent()) {
+  if (!GetCacheEntry(resource_id, std::string(), &cache_entry) ||
+      !cache_entry.IsPresent()) {
     LOG(WARNING) << "Can't clear dirty state of a file that wasn't cached: "
                  << "res_id=" << resource_id
                  << ", md5=" << md5;
@@ -1315,7 +1316,7 @@ void GDataCache::ClearDirty(const std::string& resource_id,
 
   // If a file is not dirty (it should have been marked dirty via
   // MarkDirtyInCache), clearing its dirty state is an invalid operation.
-  if (!cache_entry->IsDirty()) {
+  if (!cache_entry.IsDirty()) {
     LOG(WARNING) << "Can't clear dirty state of a non-dirty file: res_id="
                  << resource_id
                  << ", md5=" << md5;
@@ -1324,19 +1325,19 @@ void GDataCache::ClearDirty(const std::string& resource_id,
   }
 
   // File must be dirty and hence in persistent dir.
-  DCHECK(cache_entry->IsPersistent());
+  DCHECK(cache_entry.IsPersistent());
 
   // Get the current path of the file in cache.
   FilePath source_path = GetCacheFilePath(resource_id,
                                           md5,
-                                          cache_entry->GetSubDirectoryType(),
+                                          cache_entry.GetSubDirectoryType(),
                                           CACHED_FILE_LOCALLY_MODIFIED);
 
   // Determine destination path.
   // If file is pinned, move it to persistent dir with .md5 extension;
   // otherwise, move it to tmp dir with .md5 extension.
   CacheSubDirectoryType sub_dir_type =
-      cache_entry->IsPinned() ? CACHE_TYPE_PERSISTENT : CACHE_TYPE_TMP;
+      cache_entry.IsPinned() ? CACHE_TYPE_PERSISTENT : CACHE_TYPE_TMP;
   FilePath dest_path = GetCacheFilePath(resource_id,
                                         md5,
                                         sub_dir_type,
@@ -1355,7 +1356,7 @@ void GDataCache::ClearDirty(const std::string& resource_id,
                             false /* don't create symlink */);
 
   // If file is pinned, update symlink in pinned dir.
-  if (*error == base::PLATFORM_FILE_OK && cache_entry->IsPinned()) {
+  if (*error == base::PLATFORM_FILE_OK && cache_entry.IsPinned()) {
     symlink_path = GetCacheFilePath(resource_id,
                                     std::string(),
                                     CACHE_TYPE_PINNED,
@@ -1373,7 +1374,7 @@ void GDataCache::ClearDirty(const std::string& resource_id,
 
   if (*error == base::PLATFORM_FILE_OK) {
     // Now that file operations have completed, update cache map.
-    int cache_state = ClearCacheDirty(cache_entry->cache_state);
+    int cache_state = ClearCacheDirty(cache_entry.cache_state);
     UpdateCacheWithSubDirectoryType(resource_id,
                                     md5,
                                     sub_dir_type,
@@ -1390,16 +1391,15 @@ void GDataCache::Remove(const std::string& resource_id,
   // RemoveFromCacheOnBlockingPool, because we would delete all cache files
   // corresponding to <resource_id> regardless of the md5.
   // So, search for entry in cache without taking md5 into account.
-  scoped_ptr<CacheEntry> cache_entry =
-      GetCacheEntry(resource_id, std::string());
+  CacheEntry cache_entry;
 
   // If entry doesn't exist or is dirty or mounted in cache, nothing to do.
-  if (!cache_entry.get() ||
-      cache_entry->IsDirty() ||
-      cache_entry->IsMounted()) {
+  const bool entry_found =
+      GetCacheEntry(resource_id, std::string(), &cache_entry);
+  if (!entry_found || cache_entry.IsDirty() || cache_entry.IsMounted()) {
     DVLOG(1) << "Entry is "
-             << (cache_entry.get() ?
-                 (cache_entry->IsDirty() ? "dirty" : "mounted") :
+             << (entry_found ?
+                 (cache_entry.IsDirty() ? "dirty" : "mounted") :
                  "non-existent")
              << " in cache, not removing";
     *error = base::PLATFORM_FILE_OK;
@@ -1506,10 +1506,7 @@ void GDataCache::GetCacheEntryHelper(const std::string& resource_id,
   DCHECK(success);
   DCHECK(cache_entry);
 
-  scoped_ptr<GDataCache::CacheEntry> value(GetCacheEntry(resource_id, md5));
-  *success = value.get();
-  if (*success)
-    *cache_entry = *value;
+  *success = GetCacheEntry(resource_id, md5, cache_entry);
 }
 
 void GDataCache::UpdateCacheWithSubDirectoryType(