Remove stl_util's STLValueDeleter.

Remove stl_util's STLValueDeleter.

This is done by making SPDY's stream info map own the StreamInfos, and by making sync's MetahandlesMap own the EntryKernels.

BUG=555865
Committed: https://crrev.com/5cf400eafc3ad4da94998dcae6d465d9fc36a1c3
Cr-Commit-Position: refs/heads/master@{#415347}

[Avi (use Gerrit), 2016-08-26 23:32:51]

The CQ bit was checked by avi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-26 23:33:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-26 23:53:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-26 23:53:46]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Avi (use Gerrit), 2016-08-27 00:10:25]

The CQ bit was checked by avi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-27 00:11:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-27 00:52:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-27 00:52:20]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Avi (use Gerrit), 2016-08-28 20:13:24]

The CQ bit was checked by avi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-28 20:13:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-28 20:28:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-28 20:28:42]

Dry run: Try jobs failed on following builders:
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)

[Avi (use Gerrit), 2016-08-28 20:44:55]

The CQ bit was checked by avi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-28 20:44:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-28 21:16:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-28 21:16:51]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Avi (use Gerrit), 2016-08-29 00:11:34]

The CQ bit was checked by avi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-29 00:11:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-29 01:33:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-29 01:33:45]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Avi (use Gerrit), 2016-08-29 03:34:59]

The CQ bit was checked by avi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-29 03:35:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-29 05:04:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-29 05:04:46]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Avi (use Gerrit), 2016-08-29 15:03:52]

avi@chromium.org changed reviewers:
+ cbentzel@chromium.org, mark@chromium.org, svaldez@chromium.org,
zea@chromium.org

[Avi (use Gerrit), 2016-08-29 15:03:53]

zea: sync
svaldez: browser/download
cbentzel: net
mark: base

[svaldez, 2016-08-29 15:09:17]

chrome/browser/downloads lgtm.

[Mark Mentovai, 2016-08-29 16:12:26]

LGTM

[Avi (use Gerrit), 2016-08-29 18:04:59]

The CQ bit was checked by avi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-29 18:27:47]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Nicolas Zea, 2016-08-29 18:36:35]

lgtm with a nit

https://codereview.chromium.org/2287783002/diff/80001/components/sync/syncabl...
File components/sync/syncable/directory.cc (right):

https://codereview.chromium.org/2287783002/diff/80001/components/sync/syncabl...
components/sync/syncable/directory.cc:107: metahandles_map.clear();
Won't this happen automatically?

[cbentzel, 2016-08-29 18:48:01]

net/ LGTM

https://codereview.chromium.org/2287783002/diff/80001/net/spdy/http2_write_sc...
File net/spdy/http2_write_scheduler.h (right):

https://codereview.chromium.org/2287783002/diff/80001/net/spdy/http2_write_sc...
net/spdy/http2_write_scheduler.h:26: #include "base/stl_util.h"
Do you know if this can be removed as part of this change?

https://codereview.chromium.org/2287783002/diff/80001/net/spdy/http2_write_sc...
net/spdy/http2_write_scheduler.h:241: swap(new_stream_info_ptr->children,
parent->children);
This might just be me - but seems a little strange to continue using the raw
pointer after the std::move above. Could imagine moving
all_stream_infos_[stream_id] = std::move(new_stream_info) closer to the end of
this scope. Not a huge deal.

[Avi (use Gerrit), 2016-08-29 19:12:17]

https://codereview.chromium.org/2287783002/diff/80001/components/sync/syncabl...
File components/sync/syncable/directory.cc (right):

https://codereview.chromium.org/2287783002/diff/80001/components/sync/syncabl...
components/sync/syncable/directory.cc:107: metahandles_map.clear();
On 2016/08/29 18:36:35, Nicolas Zea wrote:
> Won't this happen automatically?

Done.

https://codereview.chromium.org/2287783002/diff/80001/net/spdy/http2_write_sc...
File net/spdy/http2_write_scheduler.h (right):

https://codereview.chromium.org/2287783002/diff/80001/net/spdy/http2_write_sc...
net/spdy/http2_write_scheduler.h:26: #include "base/stl_util.h"
On 2016/08/29 18:48:01, cbentzel wrote:
> Do you know if this can be removed as part of this change?

Missed it. Yes, it can.

https://codereview.chromium.org/2287783002/diff/80001/net/spdy/http2_write_sc...
net/spdy/http2_write_scheduler.h:241: swap(new_stream_info_ptr->children,
parent->children);
On 2016/08/29 18:48:01, cbentzel wrote:
> This might just be me - but seems a little strange to continue using the raw
> pointer after the std::move above. Could imagine moving
> all_stream_infos_[stream_id] = std::move(new_stream_info) closer to the end of
> this scope. Not a huge deal.

I wasn't sure if the order mattered, especially if the call to
UpdatePrioritiesUnder() would be changed if I did this.

[Avi (use Gerrit), 2016-08-29 19:13:05]

The CQ bit was checked by avi@chromium.org

[Avi (use Gerrit), 2016-08-29 19:13:05]

The patchset sent to the CQ was uploaded after l-g-t-m from
cbentzel@chromium.org, mark@chromium.org, svaldez@chromium.org, zea@chromium.org
Link to the patchset: https://codereview.chromium.org/2287783002/#ps100001
(title: "nits")

[cbentzel, 2016-08-29 19:19:18]

cbentzel@chromium.org changed reviewers:
+ bnc@chromium.org

[cbentzel, 2016-08-29 19:19:20]

+bnc as another set of eyes on the h2 priority stuff

https://codereview.chromium.org/2287783002/diff/80001/net/spdy/http2_write_sc...
File net/spdy/http2_write_scheduler.h (right):

https://codereview.chromium.org/2287783002/diff/80001/net/spdy/http2_write_sc...
net/spdy/http2_write_scheduler.h:241: swap(new_stream_info_ptr->children,
parent->children);
On 2016/08/29 19:12:17, Avi wrote:
> On 2016/08/29 18:48:01, cbentzel wrote:
> > This might just be me - but seems a little strange to continue using the raw
> > pointer after the std::move above. Could imagine moving
> > all_stream_infos_[stream_id] = std::move(new_stream_info) closer to the end
of
> > this scope. Not a huge deal.
> 
> I wasn't sure if the order mattered, especially if the call to
> UpdatePrioritiesUnder() would be changed if I did this.

Agreed. I don't know that area of code well, adding someone who does. But LGTM
as it is.

[Bence, 2016-08-29 19:43:07]

net/spdy/ LGTM.

[commit-bot: I haz the power, 2016-08-29 19:53:01]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-30 05:08:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-30 05:08:18]

Failed to apply patch for components/sync/syncable/directory.cc:
While running git apply --index -3 -p1;
  error: patch failed: components/sync/syncable/directory.cc:13
  Falling back to three-way merge...
  Applied patch to 'components/sync/syncable/directory.cc' with conflicts.
  U components/sync/syncable/directory.cc

Patch:       components/sync/syncable/directory.cc
Index: components/sync/syncable/directory.cc
diff --git a/components/sync/syncable/directory.cc
b/components/sync/syncable/directory.cc
index
1ed94cad1f5ea9d04384a634e733515044fedc1f..b5afe3848a6246cfc088b4af5ba7260e58a10675
100644
--- a/components/sync/syncable/directory.cc
+++ b/components/sync/syncable/directory.cc
@@ -13,6 +13,7 @@
 
 #include "base/base64.h"
 #include "base/guid.h"
+#include "base/memory/ptr_util.h"
 #include "base/metrics/histogram.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
@@ -102,10 +103,7 @@ Directory::Kernel::Kernel(
   DCHECK(transaction_observer.IsInitialized());
 }
 
-Directory::Kernel::~Kernel() {
-  base::STLDeleteContainerPairSecondPointers(metahandles_map.begin(),
-                                             metahandles_map.end());
-}
+Directory::Kernel::~Kernel() {}
 
 Directory::Directory(
     DirectoryBackingStore* store,
@@ -143,9 +141,9 @@ DirOpenResult Directory::Open(
 void Directory::InitializeIndices(MetahandlesMap* handles_map) {
   ScopedKernelLock lock(this);
   kernel_->metahandles_map.swap(*handles_map);
-  for (MetahandlesMap::const_iterator it = kernel_->metahandles_map.begin();
+  for (auto it = kernel_->metahandles_map.begin();
        it != kernel_->metahandles_map.end(); ++it) {
-    EntryKernel* entry = it->second;
+    EntryKernel* entry = it->second.get();
     if (ParentChildIndex::ShouldInclude(entry))
       kernel_->parent_child_index.Insert(entry);
     const int64_t metahandle = entry->ref(META_HANDLE);
@@ -185,10 +183,6 @@ DirOpenResult Directory::OpenImpl(
   // swap these later.
   Directory::MetahandlesMap tmp_handles_map;
 
-  // Avoids mem leaks on failure.  Harmlessly deletes the empty hash map after
-  // the swap in the success case.
-  base::STLValueDeleter<MetahandlesMap> deleter(&tmp_handles_map);
-
   JournalIndex delete_journals;
   MetahandleSet metahandles_to_purge;
 
@@ -287,7 +281,7 @@ EntryKernel* Directory::GetEntryByHandle(const
ScopedKernelLock& lock,
   MetahandlesMap::iterator found = kernel_->metahandles_map.find(metahandle);
   if (found != kernel_->metahandles_map.end()) {
     // Found it in memory.  Easy.
-    return found->second;
+    return found->second.get();
   }
   return NULL;
 }
@@ -367,7 +361,8 @@ bool Directory::InsertEntry(const ScopedKernelLock& lock,
   static const char error[] = "Entry already in memory index.";
 
   if (!SyncAssert(kernel_->metahandles_map
-                      .insert(std::make_pair(entry->ref(META_HANDLE), entry))
+                      .insert(std::make_pair(entry->ref(META_HANDLE),
+                                             base::WrapUnique(entry)))
                       .second,
                   FROM_HERE, error, trans)) {
     return false;
@@ -599,10 +594,12 @@ bool Directory::VacuumAfterSaveChanges(const
SaveChangesSnapshot& snapshot) {
     MetahandlesMap::iterator found =
         kernel_->metahandles_map.find((*i)->ref(META_HANDLE));
     EntryKernel* entry =
-        (found == kernel_->metahandles_map.end() ? NULL : found->second);
+        (found == kernel_->metahandles_map.end() ? NULL : found->second.get());
     if (entry && SafeToPurgeFromMemory(&trans, entry)) {
       // We now drop deleted metahandles that are up to date on both the client
       // and the server.
+      std::unique_ptr<EntryKernel> unique_entry = std::move(found->second);
+
       size_t num_erased = 0;
       num_erased = kernel_->metahandles_map.erase(entry->ref(META_HANDLE));
       DCHECK_EQ(1u, num_erased);
@@ -623,8 +620,6 @@ bool Directory::VacuumAfterSaveChanges(const
SaveChangesSnapshot& snapshot) {
         return false;
       RemoveFromAttachmentIndex(lock, entry->ref(META_HANDLE),
                                 entry->ref(ATTACHMENT_METADATA));
-
-      delete entry;
     }
     if (trans.unrecoverable_error_set())
       return false;
@@ -698,8 +693,11 @@ void Directory::DeleteEntry(const ScopedKernelLock& lock,
 
   kernel_->metahandles_to_purge.insert(handle);
 
+  std::unique_ptr<EntryKernel> entry_ptr =
+      std::move(kernel_->metahandles_map[handle]);
+
   size_t num_erased = 0;
-  num_erased = kernel_->metahandles_map.erase(entry->ref(META_HANDLE));
+  num_erased = kernel_->metahandles_map.erase(handle);
   DCHECK_EQ(1u, num_erased);
   num_erased = kernel_->ids_map.erase(entry->ref(ID).value());
   DCHECK_EQ(1u, num_erased);
@@ -721,9 +719,7 @@ void Directory::DeleteEntry(const ScopedKernelLock& lock,
   RemoveFromAttachmentIndex(lock, handle, entry->ref(ATTACHMENT_METADATA));
 
   if (save_to_journal) {
-    entries_to_journal->insert(entry);
-  } else {
-    delete entry;
+    entries_to_journal->insert(entry_ptr.release());
   }
 }
 
@@ -759,7 +755,7 @@ bool Directory::PurgeEntriesWithTypeIn(ModelTypeSet
disabled_types,
 
       for (MetahandlesMap::iterator it = kernel_->metahandles_map.begin();
            it != kernel_->metahandles_map.end();) {
-        EntryKernel* entry = it->second;
+        EntryKernel* entry = it->second.get();
         const sync_pb::EntitySpecifics& local_specifics =
entry->ref(SPECIFICS);
         const sync_pb::EntitySpecifics& server_specifics =
             entry->ref(SERVER_SPECIFICS);
@@ -1088,7 +1084,7 @@ void Directory::GetMetaHandlesOfType(const
ScopedKernelLock& lock,
   result->clear();
   for (MetahandlesMap::iterator it = kernel_->metahandles_map.begin();
        it != kernel_->metahandles_map.end(); ++it) {
-    EntryKernel* entry = it->second;
+    EntryKernel* entry = it->second.get();
     const ModelType entry_type =
         GetModelTypeFromSpecifics(entry->ref(SPECIFICS));
     if (entry_type == type)
@@ -1104,7 +1100,7 @@ void Directory::CollectMetaHandleCounts(
 
   for (MetahandlesMap::iterator it = kernel_->metahandles_map.begin();
        it != kernel_->metahandles_map.end(); ++it) {
-    EntryKernel* entry = it->second;
+    EntryKernel* entry = it->second.get();
     const ModelType type = GetModelTypeFromSpecifics(entry->ref(SPECIFICS));
     (*num_entries_by_type)[type]++;
     if (entry->ref(IS_DEL))
@@ -1124,7 +1120,7 @@ std::unique_ptr<base::ListValue>
Directory::GetNodeDetailsForType(
       continue;
     }
 
-    EntryKernel* kernel = it->second;
+    EntryKernel* kernel = it->second.get();
     std::unique_ptr<base::DictionaryValue> node(
         kernel->ToValue(GetCryptographer(trans)));

[Avi (use Gerrit), 2016-08-30 16:04:03]

The CQ bit was checked by avi@chromium.org

[Avi (use Gerrit), 2016-08-30 16:04:04]

The patchset sent to the CQ was uploaded after l-g-t-m from
cbentzel@chromium.org, mark@chromium.org, svaldez@chromium.org,
zea@chromium.org, bnc@chromium.org
Link to the patchset: https://codereview.chromium.org/2287783002/#ps120001
(title: "rebase")

[commit-bot: I haz the power, 2016-08-30 16:04:32]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-30 17:57:21]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-08-30 18:01:14]

Description was changed from

==========
Remove stl_util's STLValueDeleter.

This is done by making SPDY's stream info map own the StreamInfos, and by making
sync's MetahandlesMap own the EntryKernels.

BUG=555865
==========

to

==========
Remove stl_util's STLValueDeleter.

This is done by making SPDY's stream info map own the StreamInfos, and by making
sync's MetahandlesMap own the EntryKernels.

BUG=555865

Committed: https://crrev.com/5cf400eafc3ad4da94998dcae6d465d9fc36a1c3
Cr-Commit-Position: refs/heads/master@{#415347}
==========

[commit-bot: I haz the power, 2016-08-30 18:01:15]

Patchset 7 (id:??) landed as
https://crrev.com/5cf400eafc3ad4da94998dcae6d465d9fc36a1c3
Cr-Commit-Position: refs/heads/master@{#415347}