Remove TextDatabase from the history service.

chrome/browser/history/history_backend.cc

Index: chrome/browser/history/history_backend.cc
diff --git a/chrome/browser/history/history_backend.cc b/chrome/browser/history/history_backend.cc
index 4fc199f9f2a25c3c14c04821ff316d5d86ce9f21..dbea8a22e30f30d8903054d07dfc33bee92d4fe0 100644
--- a/chrome/browser/history/history_backend.cc
+++ b/chrome/browser/history/history_backend.cc
@@ -15,6 +15,7 @@
 #include "base/bind.h"
 #include "base/compiler_specific.h"
 #include "base/file_util.h"
+#include "base/files/file_enumerator.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_vector.h"
 #include "base/message_loop.h"
@@ -69,10 +70,6 @@ using base::TimeTicks;
 
       (this does not store visit segments as they expire after 3 mos.)
 
-    TextDatabaseManager (manages multiple text database for different times)
-      TextDatabase (represents a single month of full-text index).
-      ...more TextDatabase objects...
-
     ExpireHistoryBackend (manages moving things from HistoryDatabase to
                           the ArchivedDatabase and deleting)
 */
@@ -164,53 +161,6 @@ class CommitLaterTask : public base::RefCounted<CommitLaterTask> {
   scoped_refptr<HistoryBackend> history_backend_;
 };
 
-// Handles querying first the main database, then the full text database if that
-// fails. It will optionally keep track of all URLs seen so duplicates can be
-// eliminated. This is used by the querying sub-functions.
-//
-// TODO(brettw): This class may be able to be simplified or eliminated. After
-// this was written, QueryResults can efficiently look up by URL, so the need
-// for this extra set of previously queried URLs is less important.
-class HistoryBackend::URLQuerier {
- public:
-  URLQuerier(URLDatabase* main_db, URLDatabase* archived_db, bool track_unique)
-      : main_db_(main_db),
-        archived_db_(archived_db),
-        track_unique_(track_unique) {
-  }
-
-  // When we're tracking unique URLs, returns true if this URL has been
-  // previously queried. Only call when tracking unique URLs.
-  bool HasURL(const GURL& url) {
-    DCHECK(track_unique_);
-    return unique_urls_.find(url) != unique_urls_.end();
-  }
-
-  bool GetRowForURL(const GURL& url, URLRow* row) {
-    if (!main_db_->GetRowForURL(url, row)) {
-      if (!archived_db_ || !archived_db_->GetRowForURL(url, row)) {
-        // This row is neither in the main nor the archived DB.
-        return false;
-      }
-    }
-
-    if (track_unique_)
-      unique_urls_.insert(url);
-    return true;
-  }
-
- private:
-  URLDatabase* main_db_;  // Guaranteed non-NULL.
-  URLDatabase* archived_db_;  // Possibly NULL.
-
-  bool track_unique_;
-
-  // When track_unique_ is set, this is updated with every URL seen so far.
-  std::set<GURL> unique_urls_;
-
-  DISALLOW_COPY_AND_ASSIGN(URLQuerier);
-};
-
 // KillHistoryDatabaseErrorDelegate -------------------------------------------
 
 class KillHistoryDatabaseErrorDelegate : public sql::ErrorDelegate {
@@ -617,7 +567,7 @@ void HistoryBackend::AddPage(const HistoryAddPageArgs& request) {
     }
 
     // Last, save this redirect chain for later so we can set titles & favicons
-    // on the redirected pages properly. It is indexed by the destination page.
+    // on the redirected pages properly.
     recent_redirects_.Put(request.url, redirects);
   }
 
@@ -635,11 +585,6 @@ void HistoryBackend::AddPage(const HistoryAddPageArgs& request) {
                       last_ids.second);
   }
 
-  if (text_database_) {
-    text_database_->AddPageURL(request.url, last_ids.first, last_ids.second,
-                               request.time);
-  }
-
   ScheduleCommit();
 }
 
@@ -652,12 +597,14 @@ void HistoryBackend::InitImpl(const std::string& languages) {
 
   TimeTicks beginning_time = TimeTicks::Now();
 
-  // Compute the file names. Note that the index file can be removed when the
-  // text db manager is finished being hooked up.
+  // Compute the file names.
   base::FilePath history_name = history_dir_.Append(chrome::kHistoryFilename);
   base::FilePath thumbnail_name = GetThumbnailFileName();
   base::FilePath archived_name = GetArchivedFileName();
 
+  // Delete the old index database files which are no longer used.
+  DeleteFTSIndexDatabases();
+
   // History database.
   db_.reset(new HistoryDatabase());
 
@@ -697,8 +644,8 @@ void HistoryBackend::InitImpl(const std::string& languages) {
     delete mem_backend;  // Error case, run without the in-memory DB.
   db_->BeginExclusiveMode();  // Must be after the mem backend read the data.
 
-  // Create the history publisher which needs to be passed on to the text and
-  // thumbnail databases for publishing history.
+  // Create the history publisher which needs to be passed on to the thumbnail
+  // database for publishing history.
   history_publisher_.reset(new HistoryPublisher());
   if (!history_publisher_->Init()) {
     // The init may fail when there are no indexers wanting our history.
@@ -706,22 +653,6 @@ void HistoryBackend::InitImpl(const std::string& languages) {
     history_publisher_.reset();
   }
 
-  // Full-text database. This has to be first so we can pass it to the
-  // HistoryDatabase for migration.
-  text_database_.reset(new TextDatabaseManager(history_dir_,
-                                               db_.get(), db_.get()));
-  if (!text_database_->Init(history_publisher_.get())) {
-    LOG(WARNING) << "Text database initialization failed, running without it.";
-    text_database_.reset();
-  }
-  if (db_->needs_version_17_migration()) {
-    // See needs_version_17_migration() decl for more. In this case, we want
-    // to erase all the text database files. This must be done after the text
-    // database manager has been initialized, since it knows about all the
-    // files it manages.
-    text_database_->DeleteAll();
-  }
-
   // Thumbnail database.
   thumbnail_db_.reset(new ThumbnailDatabase());
   if (!db_->GetNeedsThumbnailMigration()) {
@@ -774,7 +705,7 @@ void HistoryBackend::InitImpl(const std::string& languages) {
   // The main DB initialization should intuitively be first (not that it
   // actually matters) and the expirer should be set last.
   expirer_.SetDatabases(db_.get(), archived_db_.get(),
-                        thumbnail_db_.get(), text_database_.get());
+                        thumbnail_db_.get());
 
   // Open the long-running transaction.
   db_->BeginTransaction();
@@ -782,8 +713,6 @@ void HistoryBackend::InitImpl(const std::string& languages) {
     thumbnail_db_->BeginTransaction();
   if (archived_db_)
     archived_db_->BeginTransaction();
-  if (text_database_)
-    text_database_->BeginTransaction();
 
   // Get the first item in our database.
   db_->GetStartDate(&first_recorded_time_);
@@ -817,10 +746,6 @@ void HistoryBackend::CloseAllDatabases() {
     archived_db_->CommitTransaction();
     archived_db_.reset();
   }
-  if (text_database_) {
-    text_database_->CommitTransaction();
-    text_database_.reset();
-  }
 }
 
 std::pair<URLID, VisitID> HistoryBackend::AddPageVisit(
@@ -875,14 +800,6 @@ std::pair<URLID, VisitID> HistoryBackend::AddPageVisit(
       return std::make_pair(0, 0);
     }
     url_info.id_ = url_id;
-
-    // We don't actually add the URL to the full text index at this point. It
-    // might be nice to do this so that even if we get no title or body, the
-    // user can search for URL components and get the page.
-    //
-    // However, in most cases, we'll get at least a title and usually contents,
-    // and this add will be redundant, slowing everything down. As a result,
-    // we ignore this edge case.
   }
 
   // Add the visit with the time to the database.
@@ -952,26 +869,6 @@ void HistoryBackend::AddPagesWithDetails(const URLRows& urls,
       }
     }
 
-    // Add the page to the full text index. This function is also used for
-    // importing. Even though we don't have page contents, we can at least
-    // add the title and URL to the index so they can be searched. We don't
-    // bother to delete any already-existing FTS entries for the URL, since
-    // this is normally called on import.
-    //
-    // If you ever import *after* first run (selecting import from the menu),
-    // then these additional entries will "shadow" the originals when querying
-    // for the most recent match only, and the user won't get snippets. This is
-    // a very minor issue, and fixing it will make import slower, so we don't
-    // bother.
-    bool has_indexed = false;
-    if (text_database_) {
-      // We do not have to make it update the visit database, below, we will
-      // create the visit entry with the indexed flag set.
-      has_indexed = text_database_->AddPageData(i->url(), url_id, 0,
-                                                i->last_visit(),
-                                                i->title(), string16());
-    }
-
     // Sync code manages the visits itself.
     if (visit_source != SOURCE_SYNCED) {
       // Make up a visit to correspond to the last visit to the page.
@@ -980,7 +877,6 @@ void HistoryBackend::AddPagesWithDetails(const URLRows& urls,
                               content::PAGE_TRANSITION_LINK |
                               content::PAGE_TRANSITION_CHAIN_START |
                               content::PAGE_TRANSITION_CHAIN_END), 0);
-      visit_info.is_indexed = has_indexed;
       if (!visit_database->AddVisit(&visit_info, visit_source)) {
         NOTREACHED() << "Adding visit failed.";
         return;
@@ -1015,10 +911,6 @@ void HistoryBackend::SetPageTitle(const GURL& url,
   if (!db_)
     return;
 
-  // Update the full text index.
-  if (text_database_)
-    text_database_->AddPageTitle(url, title);
-
   // Search for recent redirects which should get the same title. We make a
   // dummy list containing the exact URL visited if there are no redirects so
   // the processing below can be the same.
@@ -1519,59 +1411,6 @@ void HistoryBackend::QueryHistoryText(URLDatabase* url_db,
     result->set_reached_beginning(true);
 }
 
-void HistoryBackend::QueryHistoryFTS(const string16& text_query,
-                                     const QueryOptions& options,
-                                     QueryResults* result) {
-  if (!text_database_)
-    return;
-
-  // Full text query, first get all the FTS results in the time range.
-  std::vector<TextDatabase::Match> fts_matches;
-  Time first_time_searched;
-  text_database_->GetTextMatches(text_query, options,
-                                 &fts_matches, &first_time_searched);
-
-  URLQuerier querier(db_.get(), archived_db_.get(), true);
-
-  // Now get the row and visit information for each one.
-  URLResult url_result;  // Declare outside loop to prevent re-construction.
-  for (size_t i = 0; i < fts_matches.size(); i++) {
-    if (options.max_count != 0 &&
-        static_cast<int>(result->size()) >= options.max_count)
-      break;  // Got too many items.
-
-    // Get the URL, querying the main and archived databases as necessary. If
-    // this is not found, the history and full text search databases are out
-    // of sync and we give up with this result.
-    if (!querier.GetRowForURL(fts_matches[i].url, &url_result))
-      continue;
-
-    if (!url_result.url().is_valid())
-      continue;  // Don't report invalid URLs in case of corruption.
-
-    // Copy over the FTS stuff that the URLDatabase doesn't know about.
-    // We do this with swap() to avoid copying, since we know we don't
-    // need the original any more. Note that we override the title with the
-    // one from FTS, since that will match the title_match_positions (the
-    // FTS title and the history DB title may differ).
-    url_result.set_title(fts_matches[i].title);
-    url_result.title_match_positions_.swap(
-        fts_matches[i].title_match_positions);
-    url_result.snippet_.Swap(&fts_matches[i].snippet);
-
-    // The visit time also comes from the full text search database. Since it
-    // has the time, we can avoid an extra query of the visits table.
-    url_result.set_visit_time(fts_matches[i].time);
-
-    // Add it to the vector, this will clear our |url_row| object as a
-    // result of the swap.
-    result->AppendURLBySwapping(&url_result);
-  }
-
-  if (first_time_searched <= first_recorded_time_)
-    result->set_reached_beginning(true);
-}
-
 // Frontend to GetMostRecentRedirectsFrom from the history thread.
 void HistoryBackend::QueryRedirectsFrom(
     scoped_refptr<QueryRedirectsRequest> request,
@@ -1831,14 +1670,6 @@ void HistoryBackend::ScheduleAutocomplete(HistoryURLProvider* provider,
   provider->ExecuteWithDB(this, db_.get(), params);
 }
 
-void HistoryBackend::SetPageContents(const GURL& url,
-                                     const string16& contents) {
-  // This is histogrammed in the text database manager.
-  if (!text_database_)
-    return;
-  text_database_->AddPageContents(url, contents);
-}
-
 void HistoryBackend::SetPageThumbnail(
     const GURL& url,
     const gfx::Image* thumbnail,
@@ -1922,6 +1753,19 @@ void HistoryBackend::MigrateThumbnailsDatabase() {
   }
 }
 
+void HistoryBackend::DeleteFTSIndexDatabases() {
+  // Find files on disk matching the text databases file pattern so we can
+  // quickly test for and delete them.
+  base::FilePath::StringType filepattern =
+      FILE_PATH_LITERAL("History Index *");
+  base::FileEnumerator enumerator(
+      history_dir_, false, base::FileEnumerator::FILES, filepattern);
+  base::FilePath current_file;
+  while (!(current_file = enumerator.Next()).empty()) {
+    file_util::Delete(current_file, false);

[Scott Hess - ex-Googler, 2013/06/14 19:39:56]

This will potentially leave junk files.  Please use sql::Connection::Delete(),
which doesn't exist until someone reviews
https://codereview.chromium.org/17058004 .

[rmcilroy, 2013/06/17 14:11:49]

Done.

+  }

[Scott Hess - ex-Googler, 2013/06/14 19:39:56]

I think it would be reasonable to have this histogram either the number of
databases deleted, or the number of users for which any databases were deleted.

[rmcilroy, 2013/06/17 14:11:49]

Done.

+}
+
 bool HistoryBackend::GetThumbnailFromOlderRedirect(
     const GURL& page_url,
     std::vector<unsigned char>* data) {
@@ -2691,11 +2535,6 @@ void HistoryBackend::Commit() {
     archived_db_->CommitTransaction();
     archived_db_->BeginTransaction();
   }
-
-  if (text_database_) {
-    text_database_->CommitTransaction();
-    text_database_->BeginTransaction();
-  }
 }
 
 void HistoryBackend::ScheduleCommit() {
@@ -2912,7 +2751,7 @@ void HistoryBackend::KillHistoryDatabase() {
 
   // The expirer keeps tabs on the active databases. Tell it about the
   // databases which will be closed.
-  expirer_.SetDatabases(NULL, NULL, NULL, NULL);
+  expirer_.SetDatabases(NULL, NULL, NULL);
 
   // Reopen a new transaction for |db_| for the sake of CloseAllDatabases().
   db_->BeginTransaction();
@@ -3002,15 +2841,7 @@ void HistoryBackend::DeleteAllHistory() {
     LOG(ERROR) << "Main history could not be cleared";
   kept_urls.clear();
 
-  // Delete FTS files & archived history.
-  if (text_database_) {
-    // We assume that the text database has one transaction on them that we need
-    // to close & restart (the long-running history transaction).
-    text_database_->CommitTransaction();
-    text_database_->DeleteAll();
-    text_database_->BeginTransaction();
-  }
-
+  // Delete archived history.
   if (archived_db_) {
     // Close the database and delete the file.
     archived_db_.reset();