chromeos: Stop returning scoped_ptr from GDataCache::GetCacheEntry

chrome/browser/chromeos/gdata/gdata_cache_metadata.cc

 // 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/gdata_cache_metadata.h"
 
 #include "base/file_util.h"
 #include "chrome/browser/chromeos/gdata/gdata_util.h"
 
 namespace gdata {
@@ skipping 136 matching lines @@
 void GDataCacheMetadataMap::RemoveFromCache(const std::string& resource_id) {
   AssertOnSequencedWorkerPool();
 
   CacheMap::iterator iter = cache_map_.find(resource_id);
   if (iter != cache_map_.end()) {
     // Delete the CacheEntry and remove it from the map.
     cache_map_.erase(iter);
   }
 }
 
-scoped_ptr<GDataCache::CacheEntry> GDataCacheMetadataMap::GetCacheEntry(
-    const std::string& resource_id,
-    const std::string& md5) {
+bool GDataCacheMetadataMap::GetCacheEntry(const std::string& resource_id,
+                                          const std::string& md5,
+                                          GDataCache::CacheEntry* entry) {

[satorux1, 2012/06/20 15:29:23]

add DCHECK(entry); ?

[hashimoto, 2012/07/11 12:01:56]

Done.

   AssertOnSequencedWorkerPool();
 
   CacheMap::iterator iter = cache_map_.find(resource_id);
   if (iter == cache_map_.end()) {
     DVLOG(1) << "Can't find " << resource_id << " in cache map";
-    return scoped_ptr<GDataCache::CacheEntry>();
+    return false;
   }
 
-  scoped_ptr<GDataCache::CacheEntry> cache_entry(
-      new GDataCache::CacheEntry(iter->second));
+  GDataCache::CacheEntry cache_entry(iter->second);

[satorux1, 2012/06/20 15:29:23]

*entry = iter->second ? then, you don't have to copy twice.

[hashimoto, 2012/06/20 15:38:35]

I'd like to keep this copying code since I think it's better to avoid any
side-effect when returning false for MD5 match check failure.

[satorux1, 2012/06/20 15:41:20]

Good point. Then, you might want to make it a const reference.

const GDataCache::CacheEntry& cache_entry = iter->second;

[hashimoto, 2012/06/20 15:45:30]

Oh, I thought CacheEntry(iter->second) was doing some conversion but it's in
fact a simple copy ctor.
Then there is no need to make a new CacheEntry here at all…

[hashimoto, 2012/07/11 12:01:56]

Done.

 
-  if (!CheckIfMd5Matches(md5, *cache_entry)) {
+  if (!CheckIfMd5Matches(md5, cache_entry)) {
     DVLOG(1) << "Non-matching md5: want=" << md5
              << ", found=[res_id=" << resource_id
-             << ", " << cache_entry->ToString()
+             << ", " << cache_entry.ToString()
              << "]";
-    return scoped_ptr<GDataCache::CacheEntry>();
+    return false;
   }
 
   DVLOG(1) << "Found entry for res_id=" << resource_id
-           << ", " << cache_entry->ToString();
+           << ", " << cache_entry.ToString();
 
-  return cache_entry.Pass();
+  *entry = cache_entry;
+  return true;
 }
 
 void GDataCacheMetadataMap::RemoveTemporaryFiles() {
   AssertOnSequencedWorkerPool();
 
   CacheMap::iterator iter = cache_map_.begin();
   while (iter != cache_map_.end()) {
     if (iter->second.sub_dir_type == GDataCache::CACHE_TYPE_TMP) {
       // Post-increment the iterator to avoid iterator invalidation.
       cache_map_.erase(iter++);
@@ skipping 94 matching lines @@
     // If the MD5 matching is not requested, don't check MD5.
     return true;
   } else if (md5 == cache_entry.md5) {
     // Otherwise, compare the MD5.
     return true;
   }
   return false;
 }
 
 }  // namespace gdata