Remove a scoped_refptr<>::swap overload

net/disk_cache/blockfile/backend_impl.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 "net/disk_cache/blockfile/backend_impl.h"
 
 #include <limits>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 522 matching lines @@
     // We have an entry already. It could be the one we are looking for, or just
     // a hash conflict.
     bool error;
     EntryImpl* old_entry = MatchEntry(key, hash, false, Addr(), &error);
     if (old_entry)
       return ResurrectEntry(old_entry);
 
     EntryImpl* parent_entry = MatchEntry(key, hash, true, Addr(), &error);
     DCHECK(!error);
     if (parent_entry) {
-      parent.swap(&parent_entry);
+      parent = make_scoped_refptr(parent_entry);
+      parent_entry->Release();

[dcheng, 2017/03/02 18:40:19]

Is there any chance we can fix the //net code to just use scoped_refptr where
appropriate rather than relying on manual refcount adjustments? It seems quite
hard to verify that everything actually matches up...

[tzik, 2017/03/02 19:26:28]

Agree. It's very hard to read even for writing small CL like this.

[jkarlin, 2017/03/03 15:30:32]

+1

     } else if (data_->table[hash & mask_]) {
       // We should have corrected the problem.
       NOTREACHED();
       return NULL;
     }
   }
 
   // The general flow is to allocate disk space and initialize the entry data,
   // followed by saving that to disk, then linking the entry though the index
   // and finally through the lists. If there is a crash in this process, we may
@@ skipping 71 matching lines @@
   scoped_refptr<EntryImpl> entries[kListsToSearch];
   if (!iterator->my_rankings) {
     iterator->my_rankings = &rankings_;
     bool ret = false;
 
     // Get an entry from each list.
     for (int i = 0; i < kListsToSearch; i++) {
       EntryImpl* temp = NULL;
       ret |= OpenFollowingEntryFromList(static_cast<Rankings::List>(i),
                                         &iterator->nodes[i], &temp);
-      entries[i].swap(&temp);  // The entry was already addref'd.
+      if (temp) {
+        // The entry was already addref'd.
+        entries[i] = make_scoped_refptr(temp);
+        temp->Release();
+      }
     }
     if (!ret) {
       iterator->Reset();
       return NULL;
     }
   } else {
     // Get the next entry from the last list, and the actual entries for the
     // elements on the other lists.
     for (int i = 0; i < kListsToSearch; i++) {
       EntryImpl* temp = NULL;
       if (iterator->list == i) {
           OpenFollowingEntryFromList(
               iterator->list, &iterator->nodes[i], &temp);
       } else {
         temp = GetEnumeratedEntry(iterator->nodes[i],
                                   static_cast<Rankings::List>(i));
       }
 
-      entries[i].swap(&temp);  // The entry was already addref'd.
+      if (temp) {
+        // The entry was already addref'd.
+        entries[i] = make_scoped_refptr(temp);
+        temp->Release();
+      }
     }
   }
 
   int newest = -1;
   int oldest = -1;
   Time access_times[kListsToSearch];
   for (int i = 0; i < kListsToSearch; i++) {
     if (entries[i].get()) {
       access_times[i] = entries[i]->GetLastUsed();
       if (newest < 0) {
@@ skipping 934 matching lines @@
   cache_entry->SetDirtyFlag(GetCurrentEntryId());
 
   if (cache_entry->dirty()) {
     Trace("Dirty entry 0x%p 0x%x", reinterpret_cast<void*>(cache_entry.get()),
           address.value());
   }
 
   open_entries_[address.value()] = cache_entry.get();
 
   cache_entry->BeginLogging(net_log_, false);
-  cache_entry.swap(entry);
+  *entry = cache_entry.get();
+  (*entry)->AddRef();
   return 0;
 }
 
 EntryImpl* BackendImpl::MatchEntry(const std::string& key,
                                    uint32_t hash,
                                    bool find_parent,
                                    Addr entry_addr,
                                    bool* match_error) {
   Addr address(data_->table[hash & mask_]);
   scoped_refptr<EntryImpl> cache_entry, parent_entry;
-  EntryImpl* tmp = NULL;
   bool found = false;
   std::set<CacheAddr> visited;
   *match_error = false;
 
   for (;;) {
+    DCHECK(!cache_entry);
+
     if (disabled_)
       break;
 
     if (visited.find(address.value()) != visited.end()) {
       // It's possible for a buggy version of the code to write a loop. Just
       // break it.
       Trace("Hash collision loop 0x%x", address.value());
       address.set_value(0);
       parent_entry->SetNextAddress(address);
     }
     visited.insert(address.value());
 
     if (!address.is_initialized()) {
       if (find_parent)
         found = true;
       break;
     }
 
+    EntryImpl* tmp = nullptr;
     int error = NewEntry(address, &tmp);
-    cache_entry.swap(&tmp);
+    if (!error) {
+      cache_entry = make_scoped_refptr(tmp);
+      tmp->Release();
+    }
 
     if (error || cache_entry->dirty()) {
       // This entry is dirty on disk (it was not properly closed): we cannot
       // trust it.
       Addr child(0);
       if (!error)
         child.set_value(cache_entry->GetNextAddress());
 
       if (parent_entry.get()) {
         parent_entry->SetNextAddress(child);
@@ skipping 46 matching lines @@
     parent_entry = NULL;
 
   if (find_parent && entry_addr.is_initialized() && !cache_entry.get()) {
     *match_error = true;
     parent_entry = NULL;
   }
 
   if (cache_entry.get() && (find_parent || !found))
     cache_entry = NULL;
 
+  EntryImpl* tmp = NULL;
   if (find_parent)
-    parent_entry.swap(&tmp);
+    tmp = parent_entry.get();
   else
-    cache_entry.swap(&tmp);
+    tmp = cache_entry.get();
+
+  if (tmp)
+    tmp->AddRef();
 
   FlushIndex();
   return tmp;
 }
 
 bool BackendImpl::OpenFollowingEntryFromList(Rankings::List list,
                                              CacheRankingsBlock** from_entry,
                                              EntryImpl** next_entry) {
   if (disabled_)
     return false;
@@ skipping 327 matching lines @@
     if (!address.is_initialized())
       continue;
     for (;;) {
       EntryImpl* tmp;
       int ret = NewEntry(address, &tmp);
       if (ret) {
         STRESS_NOTREACHED();
         return ret;
       }
       scoped_refptr<EntryImpl> cache_entry;
-      cache_entry.swap(&tmp);
+      cache_entry = make_scoped_refptr(tmp);
+      tmp->Release();
 
       if (cache_entry->dirty())
         num_dirty++;
       else if (CheckEntry(cache_entry.get()))
         num_entries++;
       else
         return ERR_INVALID_ENTRY;
 
       DCHECK_EQ(i, cache_entry->entry()->Data()->hash & mask_);
       address.set_value(cache_entry->GetNextAddress());
@@ skipping 40 matching lines @@
     if (total_memory > kMaxBuffersSize || total_memory <= 0)
       total_memory = kMaxBuffersSize;
 
     done = true;
   }
 
   return static_cast<int>(total_memory);
 }
 
 }  // namespace disk_cache