Don't leak WebHistoryService::Request on HistoryService shutdown.

chrome/browser/history/history_service.cc

Index: chrome/browser/history/history_service.cc
diff --git a/chrome/browser/history/history_service.cc b/chrome/browser/history/history_service.cc
index 4799efa42f4eecbbb5c45cd10e0576abbed6c527..98a4c0993aa9334c20c0a213422511d094ab9103 100644
--- a/chrome/browser/history/history_service.cc
+++ b/chrome/browser/history/history_service.cc
@@ -29,6 +29,7 @@
 #include "base/message_loop/message_loop.h"
 #include "base/path_service.h"
 #include "base/prefs/pref_service.h"
+#include "base/stl_util.h"
 #include "base/thread_task_runner_handle.h"
 #include "base/threading/thread.h"
 #include "base/time/time.h"
@@ -114,14 +115,6 @@ class URLIteratorFromURLRows
   DISALLOW_COPY_AND_ASSIGN(URLIteratorFromURLRows);
 };
 
-// Callback from WebHistoryService::ExpireWebHistory().
-void ExpireWebHistoryComplete(
-    history::WebHistoryService::Request* request,
-    bool success) {
-  // Ignore the result and delete the request.
-  delete request;
-}
-
 }  // namespace
 
 // Sends messages from the backend to us on the main thread. This must be a
@@ -293,6 +286,9 @@ void HistoryService::Cleanup() {
     return;
   }
 
+  // Cancel all pending WebHistoryService requests.
+  STLDeleteElements(&pending_web_history_service_requests_);
+
   weak_ptr_factory_.InvalidateWeakPtrs();
 
   // Unload the backend.
@@ -999,6 +995,15 @@ void HistoryService::ScheduleTask(SchedulePriority priority,
   thread_->message_loop()->PostTask(FROM_HERE, task);
 }
 
+// Callback from WebHistoryService::ExpireWebHistory().
+void HistoryService::WebHistoryServiceRequestComplete(
+    history::WebHistoryService::Request* request,
+    bool success) {
+  // Ignore the result and delete the request.
+  pending_web_history_service_requests_.erase(request);
+  delete request;
+}
+
 // static
 bool HistoryService::CanAddURL(const GURL& url) {
   if (!url.is_valid())
@@ -1167,10 +1172,12 @@ void HistoryService::ExpireLocalAndRemoteHistoryBetween(
     scoped_ptr<history::WebHistoryService::Request> request =

[sky, 2014/02/19 17:33:21]

This seems like a hack to me. Expiring history doesn't seem like it should be
something that is cancelable. That is, you can't really cancel it part way.
Seems like WebHistoryService::ExpireHistoryBetween should not have a return
value.

         web_history->ExpireHistoryBetween(
             restrict_urls, begin_time, end_time,
-            base::Bind(&ExpireWebHistoryComplete));
+            base::Bind(&HistoryService::WebHistoryServiceRequestComplete,
+                       weak_ptr_factory_.GetWeakPtr()));
 
-    // The request will be freed when the callback is called.
-    CHECK(request.release());
+    // The request will be freed when the callback is called, or on
+    // HistoryService shutdown.
+    pending_web_history_service_requests_.insert(request.release());
   }
   ExpireHistoryBetween(restrict_urls, begin_time, end_time, callback, tracker);
 }