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

chrome/browser/history/history_service.cc

 // Copyright (c) 2012 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.
 
 // The history system runs on a background thread so that potentially slow
 // database operations don't delay the browser. This backend processing is
 // represented by HistoryBackend. The HistoryService's job is to dispatch to
 // that thread.
 //
 // Main thread                       History thread
@@ skipping 97 matching lines @@
   }
 
  private:
   history::URLRows::const_iterator itr_;
   history::URLRows::const_iterator end_;
 
   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;
+void ExpireWebHistoryComplete(bool success) {
+  // Ignore the result.
+  //
+  // TODO(davidben): ExpireLocalAndRemoteHistoryBetween callback should not fire
+  // until this completes.
 }
 
 }  // namespace
 
 // Sends messages from the backend to us on the main thread. This must be a
 // separate class from the history service so that it can hold a reference to
 // the history service (otherwise we would have to manually AddRef and
 // Release when the Backend has a reference to us).
 class HistoryService::BackendDelegate : public HistoryBackend::Delegate {
  public:
@@ skipping 1023 matching lines @@
       WebHistoryServiceFactory::GetForProfile(profile_);
   if (web_history) {
     // TODO(dubroy): This API does not yet support deletion of specific URLs.
     DCHECK(restrict_urls.empty());
 
     delete_directive_handler_.CreateDeleteDirectives(
         std::set<int64>(), begin_time, end_time);
 
     // Attempt online deletion from the history server, but ignore the result.
     // Deletion directives ensure that the results will eventually be deleted.
-    // Pass ownership of the request to the callback.
-    scoped_ptr<history::WebHistoryService::Request> request =
-        web_history->ExpireHistoryBetween(
-            restrict_urls, begin_time, end_time,
-            base::Bind(&ExpireWebHistoryComplete));
-
-    // The request will be freed when the callback is called.
-    CHECK(request.release());
+    //
+    // TODO(davidben): |callback| should not run until this operation completes
+    // too.
+    web_history->ExpireHistoryBetween(
+        restrict_urls, begin_time, end_time,
+        base::Bind(&ExpireWebHistoryComplete));

[Patrick Dubroy, 2014/02/20 12:56:55]

I'd just use base::DoNothing() here.

[davidben, 2014/02/20 18:08:01]

Hrm. Does that work? There's an extra argument to be discarded.

   }
   ExpireHistoryBetween(restrict_urls, begin_time, end_time, callback, tracker);
 }
 
 void HistoryService::BroadcastNotificationsHelper(
     int type,
     history::HistoryDetails* details) {
   DCHECK(thread_checker_.CalledOnValidThread());
   // TODO(evanm): this is currently necessitated by generate_profile, which
   // runs without a browser process. generate_profile should really create
@@ skipping 71 matching lines @@
   DCHECK(thread_checker_.CalledOnValidThread());
   visit_database_observers_.RemoveObserver(observer);
 }
 
 void HistoryService::NotifyVisitDBObserversOnAddVisit(
     const history::BriefVisitInfo& info) {
   DCHECK(thread_checker_.CalledOnValidThread());
   FOR_EACH_OBSERVER(history::VisitDatabaseObserver, visit_database_observers_,
                     OnAddVisit(info));
 }