Refactor History's CommitLaterTask with CancelableCallback

components/history/core/browser/history_backend.h

 // 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.
 
 #ifndef COMPONENTS_HISTORY_CORE_BROWSER_HISTORY_BACKEND_H_
 #define COMPONENTS_HISTORY_CORE_BROWSER_HISTORY_BACKEND_H_
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <memory>
 #include <set>
 #include <string>
 #include <utility>
 #include <vector>
 
+#include "base/cancelable_callback.h"
 #include "base/containers/hash_tables.h"
 #include "base/containers/mru_cache.h"
 #include "base/files/file_path.h"
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/memory/memory_pressure_listener.h"
 #include "base/observer_list.h"
 #include "base/single_thread_task_runner.h"
 #include "base/supports_user_data.h"
 #include "base/task/cancelable_task_tracker.h"
 #include "build/build_config.h"
 #include "components/favicon_base/favicon_usage_data.h"
 #include "components/history/core/browser/expire_history_backend.h"
 #include "components/history/core/browser/history_backend_notifier.h"
 #include "components/history/core/browser/history_types.h"
 #include "components/history/core/browser/keyword_id.h"
 #include "components/history/core/browser/thumbnail_database.h"
 #include "components/history/core/browser/visit_tracker.h"
 #include "sql/init_status.h"
 
 class SkBitmap;
 class TestingProfile;
 
 namespace base {
 class SingleThreadTaskRunner;
 }
 
 namespace history {
-class CommitLaterTask;
 struct DownloadRow;
 class HistoryBackendClient;
 class HistoryBackendDBBaseTest;
 class HistoryBackendObserver;
 class HistoryBackendTest;
 class HistoryDatabase;
 struct HistoryDatabaseParams;
 class HistoryDBTask;
 class InMemoryHistoryBackend;
 class TypedUrlSyncableService;
@@ skipping 417 matching lines @@
   // code to avoid syncing visits that would immediately be expired).
   virtual bool IsExpiredVisitTime(const base::Time& time);
 
   base::Time GetFirstRecordedTimeForTest() { return first_recorded_time_; }
 
  protected:
   ~HistoryBackend() override;
 
  private:
   friend class base::RefCountedThreadSafe<HistoryBackend>;
-  friend class CommitLaterTask;  // The commit task needs to call Commit().
   friend class HistoryBackendTest;
   friend class HistoryBackendDBBaseTest;  // So the unit tests can poke our
                                           // innards.
   FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, DeleteAll);
   FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, DeleteAllURLPreviouslyDeleted);
   FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, DeleteAllThenAddData);
   FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, AddPagesWithDetails);
   FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, UpdateURLs);
   FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, ImportedFaviconsTest);
   FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, URLsNoLongerBookmarked);
@@ skipping 327 matching lines @@
   // if it is. The thumbnail DB may be null when the history one isn't, but not
   // vice-versa.
   std::unique_ptr<HistoryDatabase> db_;
   bool scheduled_kill_db_;  // Database is being killed due to error.
   std::unique_ptr<ThumbnailDatabase> thumbnail_db_;
 
   // Manages expiration between the various databases.
   ExpireHistoryBackend expirer_;
 
   // A commit has been scheduled to occur sometime in the future. We can check
-  // non-null-ness to see if there is a commit scheduled in the future, and we
-  // can use the pointer to cancel the scheduled commit. There can be only one
+  // !IsCancelled() to see if there is a commit scheduled in the future, and we
+  // can use Cancel() to cancel the scheduled commit. There can be only one

[brettw, 2017/02/08 04:58:30]

Can you mention here that the initial state for a default-initialized
CancelableClosure is "canceled"? I had to go check the semantics because I was
suspicious of your unconditional usage of IsCanceled in the code.

[mastiz, 2017/02/08 08:04:29]

Done. I had a comment in the .cc file about this, added it here as well now.

   // scheduled commit at a time (see ScheduleCommit).
-  scoped_refptr<CommitLaterTask> scheduled_commit_;
+  base::CancelableClosure scheduled_commit_;
 
   // Maps recent redirect destination pages to the chain of redirects that
   // brought us to there. Pages that did not have redirects or were not the
   // final redirect in a chain will not be in this list, as well as pages that
   // redirected "too long" ago (as determined by ExpireOldRedirects above).
   // It is used to set titles & favicons for redirects to that of the
   // destination.
   //
   // As with AddPage, the last item in the redirect chain will be the
   // destination of the redirect (i.e., the key into recent_redirects_);
@@ skipping 44 matching lines @@
 
   // List of observers
   base::ObserverList<HistoryBackendObserver> observers_;
 
   DISALLOW_COPY_AND_ASSIGN(HistoryBackend);
 };
 
 }  // namespace history
 
 #endif  // COMPONENTS_HISTORY_CORE_BROWSER_HISTORY_BACKEND_H_