Cleanup HistoryService and HistoryBackend interface

chrome/browser/history/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 CHROME_BROWSER_HISTORY_HISTORY_BACKEND_H_
 #define CHROME_BROWSER_HISTORY_HISTORY_BACKEND_H_
 
 #include <set>
 #include <string>
 #include <utility>
@@ skipping 154 matching lines @@
 
   // Init must be called to complete object creation. This object can be
   // constructed on any thread, but all other functions including Init() must
   // be called on the history thread.
   //
   // |history_dir| is the directory where the history files will be placed.
   // See the definition of BroadcastNotificationsCallback above. This function
   // takes ownership of the callback pointer.
   //
   // |history_client| is used to determine bookmarked URLs when deleting and
-  // may be NULL.
+  // may be null.
   //
   // This constructor is fast and does no I/O, so can be called at any time.
-  HistoryBackend(const base::FilePath& history_dir,
-                 Delegate* delegate,
-                 HistoryClient* history_client);
+  HistoryBackend(Delegate* delegate, HistoryClient* history_client);
 
   // Must be called after creation but before any objects are created. If this
   // fails, all other functions will fail as well. (Since this runs on another
   // thread, we don't bother returning failure.)
   //
   // |languages| gives a list of language encodings with which the history
   // URLs and omnibox searches are interpreted.
   // |force_fail| can be set during unittests to unconditionally fail to init.
   void Init(const std::string& languages,
             bool force_fail,
@@ skipping 13 matching lines @@
   virtual void SetPageTitle(const GURL& url, const base::string16& title);
   void AddPageNoVisitForBookmark(const GURL& url, const base::string16& title);
   void UpdateWithPageEndTime(ContextID context_id,
                              int nav_entry_id,
                              const GURL& url,
                              base::Time end_ts);
 
   // Querying ------------------------------------------------------------------
 
   // Run the |callback| on the History thread.
-  // |callback| should handle the NULL database case.
+  // |callback| should handle the null database case.
   void ScheduleAutocomplete(const base::Callback<
       void(history::HistoryBackend*, history::URLDatabase*)>& callback);
 
   void QueryURL(const GURL& url,
                 bool want_visits,
                 QueryURLResult* query_url_result);
   void QueryHistory(const base::string16& text_query,
                     const QueryOptions& options,
                     QueryResults* query_results);
 
@@ skipping 497 matching lines @@
                          bool expired,
                          const URLRows& rows,
                          const std::set<GURL>& favicon_urls) override;
 
   // Deleting all history ------------------------------------------------------
 
   // Deletes all history. This is a special case of deleting that is separated
   // from our normal dependency-following method for performance reasons. The
   // logic lives here instead of ExpireHistoryBackend since it will cause
   // re-initialization of some databases (e.g. Thumbnails) that could fail.
-  // When these databases are not valid, our pointers must be NULL, so we need
+  // When these databases are not valid, our pointers must be null, so we need
   // to handle this type of operation to keep the pointers in sync.
   void DeleteAllHistory();
 
   // Given a vector of all URLs that we will keep, removes all thumbnails
   // referenced by any URL, and also all favicons that aren't used by those
   // URLs.
   bool ClearAllThumbnailHistory(const URLRows& kept_urls);
 
   // Deletes all information in the history database, except for the supplied
   // set of URLs in the URL table (these should correspond to the bookmarked
   // URLs).
   //
   // The IDs of the URLs may change.
   bool ClearAllMainHistory(const URLRows& kept_urls);
 
   // Deletes the FTS index database files, which are no longer used.
   void DeleteFTSIndexDatabases();
 
   // Returns the HistoryClient, blocking until the bookmarks are loaded. This
-  // may return NULL during testing.
+  // may return null during testing.
   HistoryClient* GetHistoryClient();
 
   // Notify any observers of an addition to the visit database.
   void NotifyVisitObservers(const VisitRow& visit);
 
   // Data ----------------------------------------------------------------------
 
   // Delegate. See the class definition above for more information. This will
-  // be NULL before Init is called and after Cleanup, but is guaranteed
-  // non-NULL in between.
+  // be null before Init is called and after Cleanup, but is guaranteed
+  // non-null in between.
   scoped_ptr<Delegate> delegate_;
 
-  // Directory where database files will be stored.
+  // Directory where database files will be stored, empty until Init is called.
   base::FilePath history_dir_;
 
-  // The history/thumbnail databases. Either MAY BE NULL if the database could
-  // not be opened, all users must first check for NULL and return immediately
-  // if it is. The thumbnail DB may be NULL when the history one isn't, but not
+  // The history/thumbnail databases. Either may be null if the database could
+  // not be opened, all users must first check for null and return immediately
+  // if it is. The thumbnail DB may be null when the history one isn't, but not
   // vice-versa.
   scoped_ptr<HistoryDatabase> db_;
   bool scheduled_kill_db_;  // Database is being killed due to error.
   scoped_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
@@ skipping 23 matching lines @@
   // Tracks page transition types.
   VisitTracker tracker_;
 
   // A boolean variable to track whether we have already purged obsolete segment
   // data.
   bool segment_queried_;
 
   // List of QueuedHistoryDBTasks to run;
   std::list<QueuedHistoryDBTask*> queued_history_db_tasks_;
 
-  // Used to determine if a URL is bookmarked; may be NULL.
+  // Used to determine if a URL is bookmarked; may be null.
   //
   // Use GetHistoryClient to access this, which makes sure the bookmarks are
   // loaded before returning.
   HistoryClient* history_client_;
 
 #if defined(OS_ANDROID)
   // Used to provide the Android ContentProvider APIs.
   scoped_ptr<AndroidProviderBackend> android_provider_backend_;
 #endif
 
-  // Used to manage syncing of the typed urls datatype. This will be NULL
-  // before Init is called.
-  // TODO(sdefresne): turn TypedUrlSyncableService into a HistoryBackendObserver
-  // and remove this field since it is only used for forwarding notifications
-  // about URL visited, modified, deleted.
+  // Used to manage syncing of the typed urls datatype. This will be null before
+  // Init is called.

[noyau (Ping after 24h), 2015/02/20 18:57:31]

you removed the todo without removing the instance variable. expected?

[sdefresne, 2015/02/23 10:27:17]

Yes, The interface of TypedUrlSyncableService is slighly different from
HistoryBackendObserver (it modifies some of the passed object) and thus it
cannot be converted to implement the interface, thus the removal of the TODO
since it is wrong.

   scoped_ptr<TypedUrlSyncableService> typed_url_syncable_service_;
 
   // Listens for the system being under memory pressure.
   scoped_ptr<base::MemoryPressureListener> memory_pressure_listener_;
 
   // List of observers
   ObserverList<HistoryBackendObserver> observers_;
 
   DISALLOW_COPY_AND_ASSIGN(HistoryBackend);
 };
 
 }  // namespace history
 
 #endif  // CHROME_BROWSER_HISTORY_HISTORY_BACKEND_H_