Remove stl_util's deletion functions from net/url_request/.

net/url_request/report_sender.cc

 // 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.
 
 #include "net/url_request/report_sender.h"
 
 #include <utility>
 
-#include "base/stl_util.h"
 #include "net/base/elements_upload_data_stream.h"
 #include "net/base/load_flags.h"
 #include "net/base/request_priority.h"
 #include "net/base/upload_bytes_element_reader.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_status.h"
 
 namespace net {
 
 ReportSender::ReportSender(URLRequestContext* request_context,
                            CookiesPreference cookies_preference)
     : ReportSender(request_context, cookies_preference, ErrorCallback()) {}
 
 ReportSender::ReportSender(URLRequestContext* request_context,
                            CookiesPreference cookies_preference,
                            const ErrorCallback& error_callback)
     : request_context_(request_context),
       cookies_preference_(cookies_preference),
       error_callback_(error_callback) {}
 
 ReportSender::~ReportSender() {
-  // Cancel all of the uncompleted requests.
-  base::STLDeleteElements(&inflight_requests_);
 }
 
 void ReportSender::Send(const GURL& report_uri,
                         base::StringPiece content_type,
                         base::StringPiece report) {
   DCHECK(!content_type.empty());
   std::unique_ptr<URLRequest> url_request =
       request_context_->CreateRequest(report_uri, DEFAULT_PRIORITY, this);
 
   int load_flags =
       LOAD_BYPASS_CACHE | LOAD_DISABLE_CACHE | LOAD_DO_NOT_SEND_AUTH_DATA;
   if (cookies_preference_ != SEND_COOKIES) {
     load_flags |= LOAD_DO_NOT_SEND_COOKIES | LOAD_DO_NOT_SAVE_COOKIES;
   }
   url_request->SetLoadFlags(load_flags);
 
   HttpRequestHeaders extra_headers;
   extra_headers.SetHeader(HttpRequestHeaders::kContentType, content_type);
   url_request->SetExtraRequestHeaders(extra_headers);
 
   url_request->set_method("POST");
 
   std::vector<char> report_data(report.begin(), report.end());
   std::unique_ptr<UploadElementReader> reader(
       new UploadOwnedBytesElementReader(&report_data));
   url_request->set_upload(
       ElementsUploadDataStream::CreateWithReader(std::move(reader), 0));
 
   URLRequest* raw_url_request = url_request.get();
-  inflight_requests_.insert(url_request.release());
+  inflight_requests_.insert(std::move(url_request));
   raw_url_request->Start();
 }
 
 void ReportSender::SetErrorCallback(const ErrorCallback& error_callback) {
   error_callback_ = error_callback;
 }
 
 void ReportSender::OnResponseStarted(URLRequest* request, int net_error) {
   DCHECK_NE(ERR_IO_PENDING, net_error);
 
   if (net_error != OK) {
     DVLOG(1) << "Failed to send report for " << request->url().host();
     if (!error_callback_.is_null())
       error_callback_.Run(request->url(), net_error);
   }
 
-  CHECK_GT(inflight_requests_.erase(request), 0u);
-  // Clean up the request, which cancels it.
-  delete request;
+  auto it = std::find_if(inflight_requests_.begin(), inflight_requests_.end(),
+                         [request](const std::unique_ptr<URLRequest>& ptr) {
+                           return ptr.get() == request;
+                         });
+  CHECK(it != inflight_requests_.end());
+  inflight_requests_.erase(it);

[davidben, 2016/10/03 16:01:38]

I should have noticed this for the other CL, but if we're doing this, the
std::set is buying us nothing but overhead. It'd be strictly better to use a
std::vector of unique_ptr.

These can also potentially trigger one per URLRequest so I'm not very
comfortable with it being O(N). This is actually a really common pattern where
one object owns a bunch of jobs and drops them when completed. I think it would
be good to just write a base::OwnedSet<T> and use that. (Or maybe some linked
list thing? That would actually be a more efficient structure.)

[Avi (use Gerrit), 2016/10/03 16:27:25]

If we're talking efficiency, linked lists and sets are bad. They guarantee a
cache miss for every access, so that even an O(n) access into a vector is better
performing than an "O(log n)" access that has zero memory locality.

sets of owned objects are kinda broken in C++11 anyway, so I would be super
happy to swap in a vector here.

 }
 
 void ReportSender::OnReadCompleted(URLRequest* request, int bytes_read) {
   NOTREACHED();
 }
 
 }  // namespace net