Remove stl_util's STLValueDeleter.

net/spdy/http2_write_scheduler.h

 // Copyright 2015 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.
 
 #ifndef NET_SPDY_HTTP2_WRITE_SCHEDULER_H_
 #define NET_SPDY_HTTP2_WRITE_SCHEDULER_H_
 
 #include <stdint.h>
 
 #include <algorithm>
 #include <cmath>
 #include <deque>
 #include <map>
 #include <memory>
 #include <queue>
 #include <set>
 #include <tuple>
 #include <unordered_map>
 #include <utility>
 #include <vector>
 
 #include "base/containers/linked_list.h"
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/memory/ptr_util.h"
 #include "base/stl_util.h"

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

Do you know if this can be removed as part of this change?

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

Missed it. Yes, it can.

 #include "net/spdy/spdy_bug_tracker.h"
 #include "net/spdy/spdy_protocol.h"
 #include "net/spdy/write_scheduler.h"
 
 namespace net {
 
 namespace test {
 template <typename StreamIdType>
 class Http2PriorityWriteSchedulerPeer;
 }
@@ skipping 38 matching lines @@
   size_t NumReadyStreams() const override;
 
   // Return the number of streams currently in the tree.
   int num_streams() const;
 
  private:
   friend class test::Http2PriorityWriteSchedulerPeer<StreamIdType>;
 
   struct StreamInfo;
   using StreamInfoVector = std::vector<StreamInfo*>;
-  using StreamInfoMap = std::unordered_map<StreamIdType, StreamInfo*>;
 
   struct StreamInfo : public base::LinkNode<StreamInfo> {
     // ID for this stream.
     StreamIdType id;
     // StreamInfo for parent stream.
     StreamInfo* parent = nullptr;
     // Weights can range between 1 and 256 (inclusive).
     int weight = kHttp2DefaultStreamWeight;
     // The total weight of this stream's direct descendants.
     int total_child_weights = 0;
@@ skipping 65 matching lines @@
   // Unless there are bugs, this should always return true.
   bool ValidateInvariantsForTests() const;
 
   // Returns true if the parent stream has the given stream in its children.
   bool StreamHasChild(const StreamInfo& parent_info,
                       const StreamInfo* child_info) const;
 
   // Pointee owned by all_stream_infos_.
   StreamInfo* root_stream_info_;
   // Maps from stream IDs to StreamInfo objects.
-  StreamInfoMap all_stream_infos_;
-  base::STLValueDeleter<StreamInfoMap> all_stream_infos_deleter_;
+  std::unordered_map<StreamIdType, std::unique_ptr<StreamInfo>>
+      all_stream_infos_;
   // Queue containing all ready streams, ordered with streams of higher
   // priority before streams of lower priority, and, among streams of equal
   // priority, streams with lower ordinal before those with higher
   // ordinal. Note that not all streams in scheduling_queue_ are eligible to be
   // picked as the next stream: some may have ancestor stream(s) that are ready
   // and unblocked. In these situations the occluded child streams are left in
   // the queue, to reduce churn.
   base::LinkedList<StreamInfo> scheduling_queue_;
   // Ordinal value to assign to next node inserted into scheduling_queue_ when
   // |add_to_front == true|. Decremented after each assignment.
   int64_t head_ordinal_ = -1;
   // Ordinal value to assign to next node inserted into scheduling_queue_ when
   // |add_to_front == false|. Incremented after each assignment.
   int64_t tail_ordinal_ = 0;
 
   DISALLOW_COPY_AND_ASSIGN(Http2PriorityWriteScheduler);
 };
 
 template <typename StreamIdType>
-Http2PriorityWriteScheduler<StreamIdType>::Http2PriorityWriteScheduler()
-    : all_stream_infos_deleter_(&all_stream_infos_) {
-  root_stream_info_ = new StreamInfo();
-  root_stream_info_->id = kHttp2RootStreamId;
-  root_stream_info_->weight = kHttp2DefaultStreamWeight;
-  root_stream_info_->parent = nullptr;
-  root_stream_info_->priority = 1.0;
-  root_stream_info_->ready = false;
-  all_stream_infos_[kHttp2RootStreamId] = root_stream_info_;
+Http2PriorityWriteScheduler<StreamIdType>::Http2PriorityWriteScheduler() {
+  std::unique_ptr<StreamInfo> root_stream_info = base::MakeUnique<StreamInfo>();
+  root_stream_info_ = root_stream_info.get();
+  root_stream_info->id = kHttp2RootStreamId;
+  root_stream_info->weight = kHttp2DefaultStreamWeight;
+  root_stream_info->parent = nullptr;
+  root_stream_info->priority = 1.0;
+  root_stream_info->ready = false;
+  all_stream_infos_[kHttp2RootStreamId] = std::move(root_stream_info);
 }
 
 template <typename StreamIdType>
 int Http2PriorityWriteScheduler<StreamIdType>::num_streams() const {
   return all_stream_infos_.size();
 }
 
 template <typename StreamIdType>
 bool Http2PriorityWriteScheduler<StreamIdType>::StreamRegistered(
     StreamIdType stream_id) const {
@@ skipping 12 matching lines @@
     return;
   }
 
   StreamInfo* parent = FindStream(precedence.parent_id());
   if (parent == nullptr) {
     // parent_id may legitimately not be registered yet--see b/15676312.
     DVLOG(1) << "Parent stream " << precedence.parent_id() << " not registered";
     parent = root_stream_info_;
   }
 
-  StreamInfo* new_stream_info = new StreamInfo;
-  new_stream_info->id = stream_id;
-  new_stream_info->weight = precedence.weight();
-  new_stream_info->parent = parent;
-  all_stream_infos_[stream_id] = new_stream_info;
+  std::unique_ptr<StreamInfo> new_stream_info = base::MakeUnique<StreamInfo>();
+  StreamInfo* new_stream_info_ptr = new_stream_info.get();
+  new_stream_info_ptr->id = stream_id;
+  new_stream_info_ptr->weight = precedence.weight();
+  new_stream_info_ptr->parent = parent;
+  all_stream_infos_[stream_id] = std::move(new_stream_info);
   if (precedence.is_exclusive()) {
     // Move the parent's current children below the new stream.
     using std::swap;
-    swap(new_stream_info->children, parent->children);
-    new_stream_info->total_child_weights = parent->total_child_weights;
+    swap(new_stream_info_ptr->children, parent->children);

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

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]

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

[cbentzel, 2016/08/29 19:19:19]

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

+    new_stream_info_ptr->total_child_weights = parent->total_child_weights;
     // Update each child's parent.
-    for (StreamInfo* child : new_stream_info->children) {
-      child->parent = new_stream_info;
+    for (StreamInfo* child : new_stream_info_ptr->children) {
+      child->parent = new_stream_info_ptr;
     }
     // Clear parent's old child data.
     DCHECK(parent->children.empty());
     parent->total_child_weights = 0;
   }
   // Add new stream to parent.
-  parent->children.push_back(new_stream_info);
+  parent->children.push_back(new_stream_info_ptr);
   parent->total_child_weights += precedence.weight();
 
   // Update all priorities under parent, since addition of a stream affects
   // sibling priorities as well.
   UpdatePrioritiesUnder(parent);
 
   // Stream starts with ready == false, so no need to schedule it yet.
-  DCHECK(!new_stream_info->ready);
+  DCHECK(!new_stream_info_ptr->ready);
 }
 
 template <typename StreamIdType>
 void Http2PriorityWriteScheduler<StreamIdType>::UnregisterStream(
     StreamIdType stream_id) {
   if (stream_id == kHttp2RootStreamId) {
     SPDY_BUG << "Cannot unregister root stream";
     return;
   }
   // Remove the stream from table.
-  typename StreamInfoMap::iterator it = all_stream_infos_.find(stream_id);
+  auto it = all_stream_infos_.find(stream_id);
   if (it == all_stream_infos_.end()) {
     SPDY_BUG << "Stream " << stream_id << " not registered";
     return;
   }
   std::unique_ptr<StreamInfo> stream_info(std::move(it->second));
   all_stream_infos_.erase(it);
   // If ready (and hence scheduled), unschedule.
   if (stream_info->ready) {
     Unschedule(stream_info.get());
   }
@@ skipping 287 matching lines @@
       return true;
     }
   }
   return false;
 }
 
 template <typename StreamIdType>
 const typename Http2PriorityWriteScheduler<StreamIdType>::StreamInfo*
 Http2PriorityWriteScheduler<StreamIdType>::FindStream(
     StreamIdType stream_id) const {
-  typename StreamInfoMap::const_iterator it = all_stream_infos_.find(stream_id);
-  return it == all_stream_infos_.end() ? nullptr : it->second;
+  auto it = all_stream_infos_.find(stream_id);
+  return it == all_stream_infos_.end() ? nullptr : it->second.get();
 }
 
 template <typename StreamIdType>
 typename Http2PriorityWriteScheduler<StreamIdType>::StreamInfo*
 Http2PriorityWriteScheduler<StreamIdType>::FindStream(StreamIdType stream_id) {
-  typename StreamInfoMap::iterator it = all_stream_infos_.find(stream_id);
-  return it == all_stream_infos_.end() ? nullptr : it->second;
+  auto it = all_stream_infos_.find(stream_id);
+  return it == all_stream_infos_.end() ? nullptr : it->second.get();
 }
 
 template <typename StreamIdType>
 void Http2PriorityWriteScheduler<StreamIdType>::UpdatePrioritiesUnder(
     StreamInfo* stream_info) {
   for (StreamInfo* child : stream_info->children) {
     child->priority = stream_info->priority *
                       (static_cast<float>(child->weight) /
                        static_cast<float>(stream_info->total_child_weights));
     if (child->ready) {
@@ skipping 83 matching lines @@
 template <typename StreamIdType>
 bool Http2PriorityWriteScheduler<StreamIdType>::ValidateInvariantsForTests()
     const {
   int total_streams = 0;
   int streams_visited = 0;
   // Iterate through all streams in the map.
   for (const auto& kv : all_stream_infos_) {
     ++total_streams;
     ++streams_visited;
     StreamIdType stream_id = kv.first;
-    const StreamInfo& stream_info = *kv.second;
+    const StreamInfo& stream_info = *kv.second.get();
 
     // Verify each StreamInfo mapped under the proper stream ID.
     if (stream_id != stream_info.id) {
       DLOG(INFO) << "Stream ID " << stream_id << " maps to StreamInfo with ID "
                  << stream_info.id;
       return false;
     }
 
     // All streams except the root should have a parent, and should appear in
     // the children of that parent.
@@ skipping 39 matching lines @@
   }
   // Validate the validation function; we should have visited each stream twice
   // (except for the root)
   DCHECK(streams_visited == 2 * num_streams() - 1);
   return true;
 }
 
 }  // namespace net
 
 #endif  // NET_SPDY_HTTP2_WRITE_SCHEDULER_H_