Remove stl_util's deletion function use from components/history/.

components/history/core/browser/web_history_service.h

Index: components/history/core/browser/web_history_service.h
diff --git a/components/history/core/browser/web_history_service.h b/components/history/core/browser/web_history_service.h
index d5681a8d0f5db7dd9cab9d7e3091fd8d0704e021..992fafa0bf2ba5e5bc5b2581988038c1efd13bdf 100644
--- a/components/history/core/browser/web_history_service.h
+++ b/components/history/core/browser/web_history_service.h
@@ -7,7 +7,8 @@
 
 #include <stddef.h>
 
-#include <set>
+#include <map>
+#include <memory>
 #include <string>
 #include <vector>
 
@@ -212,18 +213,20 @@ class WebHistoryService : public KeyedService {
 
   // Pending expiration requests to be canceled if not complete by profile
   // shutdown.
-  std::set<Request*> pending_expire_requests_;
+  std::map<Request*, std::unique_ptr<Request>> pending_expire_requests_;

[sky, 2016/10/24 16:04:10]

Using a map purely for the ownership is bizarre. Can you instead use a custom
comparison function?

[Avi (use Gerrit), 2016/10/24 16:09:01]

It's not about comparison functions.

In general, I convert to map<pointer, owned pointer> because I've gotten
feedback that people don't like losing logarithmic lookup. I can switch to a
set, and use find_if if that isn't an issue.

The real tricky thing is that you can't extract ownership from the set once you
give it to the set. So I'd have to rewrite the functions that do the removal to
move the set element deletion to be the last thing. In this case I don't believe
it would matter.

[sky, 2016/10/24 16:40:07]

I was hoping a comparison function would give the logarithmic lookup time. But
that would require searches to be done by placing the pointer in a unique_ptr,
then doing the search. Which isn't really what you want, and is easy to get
wrong (have to be careful to remove the pointer from the unique_ptr after the
search).

It seems to me trying to use stl containers with unique_ptr to enforce ownership
is quirky... Was there ever a discussion of stl containers with unique_ptrs on
chromium-dev?

I'll approve this since this appears to be the path we're going down, but I
think we should consider ways to address the need of containers that own their
contents without the quirks that stl + unique_ptr gives you.

[Avi (use Gerrit), 2016/10/24 17:05:34]

There was a thread on cxx
(https://groups.google.com/a/chromium.org/d/msg/cxx/xJpM3J_gpZY/-EA_DuGdCwAJ)
but there were no solid conclusions.

 
   // Pending requests to be canceled if not complete by profile shutdown.
-  std::set<Request*> pending_audio_history_requests_;
+  std::map<Request*, std::unique_ptr<Request>> pending_audio_history_requests_;
 
   // Pending web and app activity queries to be canceled if not complete by
   // profile shutdown.
-  std::set<Request*> pending_web_and_app_activity_requests_;
+  std::map<Request*, std::unique_ptr<Request>>
+      pending_web_and_app_activity_requests_;
 
   // Pending queries for other forms of browsing history to be canceled if not
   // complete by profile shutdown.
-  std::set<Request*> pending_other_forms_of_browsing_history_requests_;
+  std::map<Request*, std::unique_ptr<Request>>
+      pending_other_forms_of_browsing_history_requests_;
 
   // Observers.
   base::ObserverList<WebHistoryServiceObserver, true> observer_list_;