Migration to TopSites from ThumbnailDatabase.

BUG=None
TEST=TopSitesTest, HistoryBackendTest, ThumbnailDatabaseTest, HistoryTest

Review URL: http://codereview.chromium.org/2869018

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@50610 0039d316-1c4b-4281-b951-d872f2087c98

12 files changed, 231 insertions, 20 deletions

diff --git a/chrome/browser/history/history.cc b/chrome/browser/history/history.cc
index c692963..61d0ae0 100644
--- a/chrome/browser/history/history.cc
+++ b/chrome/browser/history/history.cc
@@ -41,6 +41,7 @@
 #include "chrome/browser/history/history_types.h"
 #include "chrome/browser/history/in_memory_database.h"
 #include "chrome/browser/history/in_memory_history_backend.h"
+#include "chrome/browser/history/top_sites.h"
 #include "chrome/browser/profile.h"
 #include "chrome/browser/visitedlink_master.h"
 #include "chrome/common/chrome_constants.h"
@@ -103,6 +104,11 @@ class HistoryService::BackendDelegate : public HistoryBackend::Delegate {
         &HistoryService::OnDBLoaded));
   }
 
+  virtual void StartTopSitesMigration() {
+    message_loop_->PostTask(FROM_HERE, NewRunnableMethod(history_service_.get(),
+        &HistoryService::StartTopSitesMigration));
+  }
+
  private:
   scoped_refptr<HistoryService> history_service_;
   MessageLoop* message_loop_;
@@ -743,3 +749,12 @@ void HistoryService::OnDBLoaded() {
                                          Source<Profile>(profile_),
                                          Details<HistoryService>(this));
 }
+
+void HistoryService::StartTopSitesMigration() {
+  history::TopSites* ts = profile_->GetTopSites();
+  ts->StartMigration();
+}
+
+void HistoryService::OnTopSitesReady() {
+  ScheduleAndForget(PRIORITY_NORMAL, &HistoryBackend::DeleteThumbnailsDatabase);
+}
diff --git a/chrome/browser/history/history.h b/chrome/browser/history/history.h
index e9abbac..f1c7544 100644
--- a/chrome/browser/history/history.h
+++ b/chrome/browser/history/history.h
@@ -546,6 +546,14 @@ class HistoryService : public CancelableRequestProvider,
   // The same as AddPageWithDetails() but takes a vector.
   void AddPagesWithDetails(const std::vector<history::URLRow>& info);
 
+  // Starts the TopSites migration in the HistoryThread. Called by the
+  // BackendDelegate.
+  void StartTopSitesMigration();
+
+  // Called by TopSites after the thumbnails were read and it is safe
+  // to delete the thumbnails DB.
+  void OnTopSitesReady();
+
  protected:
   ~HistoryService();
 
diff --git a/chrome/browser/history/history_backend.cc b/chrome/browser/history/history_backend.cc
index 2e57b46..742f458 100644
--- a/chrome/browser/history/history_backend.cc
+++ b/chrome/browser/history/history_backend.cc
@@ -6,6 +6,7 @@
 
 #include <set>
 
+#include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/file_util.h"
 #include "base/histogram.h"
@@ -22,6 +23,7 @@
 #include "chrome/browser/history/in_memory_history_backend.h"
 #include "chrome/browser/history/page_usage_data.h"
 #include "chrome/common/chrome_constants.h"
+#include "chrome/common/chrome_switches.h"
 #include "chrome/common/notification_type.h"
 #include "chrome/common/sqlite_utils.h"
 #include "chrome/common/url_constants.h"
@@ -587,6 +589,13 @@ void HistoryBackend::InitImpl() {
     thumbnail_db_.reset();
   }
 
+  if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kTopSites)) {
+    if (thumbnail_db_->NeedsMigrationToTopSites()) {
+      LOG(INFO) << "Starting TopSites migration";
+      delegate_->StartTopSitesMigration();
+    }
+  }
+
   // Archived database.
   if (db_->needs_version_17_migration()) {
     // See needs_version_17_migration() decl for more. In this case, we want
@@ -2137,4 +2146,8 @@ BookmarkService* HistoryBackend::GetBookmarkService() {
   return bookmark_service_;
 }
 
+void HistoryBackend::DeleteThumbnailsDatabase() {
+  thumbnail_db_->DropThumbnailsTable();
+}
+
 }  // namespace history
diff --git a/chrome/browser/history/history_backend.h b/chrome/browser/history/history_backend.h
index 37bc80e..16119cc 100644
--- a/chrome/browser/history/history_backend.h
+++ b/chrome/browser/history/history_backend.h
@@ -76,6 +76,9 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>,
 
     // Invoked when the backend has finished loading the db.
     virtual void DBLoaded() = 0;
+
+    // Tell TopSites to start reading thumbnails from the ThumbnailsDB.
+    virtual void StartTopSitesMigration() = 0;
   };
 
   // Init must be called to complete object creation. This object can be
@@ -191,6 +194,8 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>,
       const GURL& page_url,
       scoped_refptr<RefCountedBytes>* data);
 
+  void DeleteThumbnailsDatabase();
+
   // Favicon -------------------------------------------------------------------
 
   void GetFavIcon(scoped_refptr<GetFavIconRequest> request,
@@ -307,6 +312,7 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>,
   FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, ImportedFaviconsTest);
   FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, URLsNoLongerBookmarked);
   FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, StripUsernamePasswordTest);
+  FRIEND_TEST_ALL_PREFIXES(HistoryBackendTest, DeleteThumbnailsDatabaseTest);
   friend class ::TestingProfile;
 
   // Computes the name of the specified database on disk.
diff --git a/chrome/browser/history/history_backend_unittest.cc b/chrome/browser/history/history_backend_unittest.cc
index 4d1dca6..9af5662 100644
--- a/chrome/browser/history/history_backend_unittest.cc
+++ b/chrome/browser/history/history_backend_unittest.cc
@@ -42,6 +42,7 @@ class HistoryBackendTestDelegate : public HistoryBackend::Delegate {
   virtual void BroadcastNotifications(NotificationType type,
                                       HistoryDetails* details);
   virtual void DBLoaded();
+  virtual void StartTopSitesMigration();
 
  private:
   // Not owned by us.
@@ -166,6 +167,10 @@ void HistoryBackendTestDelegate::DBLoaded() {
   test_->loaded_ = true;
 }
 
+void HistoryBackendTestDelegate::StartTopSitesMigration() {
+  test_->backend_->DeleteThumbnailsDatabase();
+}
+
 TEST_F(HistoryBackendTest, Loaded) {
   ASSERT_TRUE(backend_.get());
   ASSERT_TRUE(loaded_);
@@ -592,4 +597,10 @@ TEST_F(HistoryBackendTest, StripUsernamePasswordTest) {
   ASSERT_EQ(1U, visits.size());
 }
 
+TEST_F(HistoryBackendTest, DeleteThumbnailsDatabaseTest) {
+  EXPECT_TRUE(backend_->thumbnail_db_->NeedsMigrationToTopSites());
+  backend_->delegate_->StartTopSitesMigration();
+  EXPECT_FALSE(backend_->thumbnail_db_->NeedsMigrationToTopSites());
+}
+
 }  // namespace history
diff --git a/chrome/browser/history/history_unittest.cc b/chrome/browser/history/history_unittest.cc
index a0e6e21..24b4119 100644
--- a/chrome/browser/history/history_unittest.cc
+++ b/chrome/browser/history/history_unittest.cc
@@ -99,7 +99,7 @@ class BackendDelegate : public HistoryBackend::Delegate {
   virtual void BroadcastNotifications(NotificationType type,
                                       HistoryDetails* details);
   virtual void DBLoaded() {}
-
+  virtual void StartTopSitesMigration() {}
  private:
   HistoryTest* history_test_;
 };
diff --git a/chrome/browser/history/thumbnail_database.cc b/chrome/browser/history/thumbnail_database.cc
index edfb6a8..a6c75e3 100644
--- a/chrome/browser/history/thumbnail_database.cc
+++ b/chrome/browser/history/thumbnail_database.cc
@@ -6,6 +6,7 @@
 
 #include "app/sql/statement.h"
 #include "app/sql/transaction.h"
+#include "base/command_line.h"
 #include "base/file_util.h"
 #if defined(OS_MACOSX)
 #include "base/mac_util.h"
@@ -16,6 +17,7 @@
 #include "chrome/browser/diagnostics/sqlite_diagnostics.h"
 #include "chrome/browser/history/history_publisher.h"
 #include "chrome/browser/history/url_database.h"
+#include "chrome/common/chrome_switches.h"
 #include "chrome/common/thumbnail_score.h"
 #include "gfx/codec/jpeg_codec.h"
 #include "third_party/skia/include/core/SkBitmap.h"
@@ -118,6 +120,8 @@ sql::InitStatus ThumbnailDatabase::Init(
 
 bool ThumbnailDatabase::InitThumbnailTable() {
   if (!db_.DoesTableExist("thumbnails")) {
+    if (CommandLine::ForCurrentProcess()-> HasSwitch(switches::kTopSites))
+      return true;
     if (!db_.Execute("CREATE TABLE thumbnails ("
         "url_id INTEGER PRIMARY KEY,"
         "boring_score DOUBLE DEFAULT 1.0,"
@@ -433,4 +437,18 @@ bool ThumbnailDatabase::CommitTemporaryFavIconTable() {
   return true;
 }
 
+bool ThumbnailDatabase::DropThumbnailsTable() {
+  DCHECK(NeedsMigrationToTopSites());
+  if (!db_.Execute("DROP TABLE thumbnails"))
+    return false;
+  CommitTransaction();
+  Vacuum();
+  BeginTransaction();
+  return true;
+}
+
+bool ThumbnailDatabase::NeedsMigrationToTopSites() {
+  return db_.DoesTableExist("thumbnails");
+}
+
 }  // namespace history
diff --git a/chrome/browser/history/thumbnail_database.h b/chrome/browser/history/thumbnail_database.h
index 547e930..fba8ca0 100644
--- a/chrome/browser/history/thumbnail_database.h
+++ b/chrome/browser/history/thumbnail_database.h
@@ -135,6 +135,15 @@ class ThumbnailDatabase {
   // will be deleted. Returns true on success.
   bool CommitTemporaryFavIconTable();
 
+  // Returns true iff the thumbnails table exists.
+  // Migrating to TopSites is dropping the thumbnails table.
+  // (TODO(nshkrob): renaming to favicons??)
+  bool NeedsMigrationToTopSites();
+
+  // Drops thumbnails table. Returns true iff the table was dropped
+  // successfully. False may mean that the table was already dropped.
+  bool DropThumbnailsTable();
+
  private:
   friend class ExpireHistoryBackend;
 
diff --git a/chrome/browser/history/thumbnail_database_unittest.cc b/chrome/browser/history/thumbnail_database_unittest.cc
index c0d9ff1..1542033 100644
--- a/chrome/browser/history/thumbnail_database_unittest.cc
+++ b/chrome/browser/history/thumbnail_database_unittest.cc
@@ -329,4 +329,13 @@ TEST_F(ThumbnailDatabaseTest, NeverAcceptTotallyBoringThumbnail) {
   ASSERT_TRUE(base_boring.Equals(score_out));
 }
 
+TEST_F(ThumbnailDatabaseTest, NeedsMigrationToTopSites) {
+  ThumbnailDatabase db;
+  ASSERT_EQ(sql::INIT_OK, db.Init(file_name_, NULL));
+  db.BeginTransaction();
+  EXPECT_TRUE(db.NeedsMigrationToTopSites());
+  EXPECT_TRUE(db.DropThumbnailsTable());
+  EXPECT_FALSE(db.NeedsMigrationToTopSites());
+}
+
 }  // namespace history
diff --git a/chrome/browser/history/top_sites.cc b/chrome/browser/history/top_sites.cc
index 49a2757..498a9ae 100644
--- a/chrome/browser/history/top_sites.cc
+++ b/chrome/browser/history/top_sites.cc
@@ -30,7 +30,8 @@ static const int64 kMaxUpdateIntervalMinutes = 60;
 
 TopSites::TopSites(Profile* profile) : profile_(profile),
                                        mock_history_service_(NULL),
-                                       last_num_urls_changed_(0) {
+                                       last_num_urls_changed_(0),
+                                       migration_in_progress_(false) {
   registrar_.Add(this, NotificationType::HISTORY_URLS_DELETED,
                  Source<Profile>(profile_));
   registrar_.Add(this, NotificationType::NAV_ENTRY_COMMITTED,
@@ -73,7 +74,7 @@ void TopSites::ReadDatabase() {
     GURL url = top_sites_[i].url;
     Images thumbnail = thumbnails[url];
     if (!thumbnail.thumbnail.get() || !thumbnail.thumbnail->size()) {
-      LOG(INFO) << "No thumnbail for " << url.spec();
+      LOG(INFO) << "No thumbnail for " << url.spec();
     } else {
       SetPageThumbnailNoDB(url, thumbnail.thumbnail,
                            thumbnail.thumbnail_score);
@@ -216,20 +217,51 @@ void TopSites::UpdateMostVisited(MostVisitedURLList most_visited) {
   }
 
   StoreMostVisited(&most_visited);
-  for (size_t i = 0; i < top_sites_.size(); i++) {
-    GURL& url = top_sites_[i].url;
-    Images& img = top_images_[url];
-    if (!img.thumbnail.get() || !img.thumbnail->size()) {
-      StartQueryForThumbnail(i);
+  if (migration_in_progress_) {
+    // Copy all thumnbails from the history service.
+    for (size_t i = 0; i < top_sites_.size(); i++) {
+      GURL& url = top_sites_[i].url;
+      Images& img = top_images_[url];
+      if (!img.thumbnail.get() || !img.thumbnail->size()) {
+        StartQueryForThumbnail(i);
+      }
     }
   }
 
+  // If we are not expecting any thumbnails, migration is done.
+  if (migration_in_progress_ && migration_pending_urls_.empty())
+    OnMigrationDone();
+
   timer_.Stop();
   timer_.Start(GetUpdateDelay(), this,
                &TopSites::StartQueryForMostVisited);
 }
 
+void TopSites::OnMigrationDone() {
+  migration_in_progress_ = false;
+  HistoryService* hs = profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
+  // |hs| may be null during unit tests.
+  if (!hs)
+    return;
+  hs->OnTopSitesReady();
+}
+
 void TopSites::StartQueryForThumbnail(size_t index) {
+  DCHECK(migration_in_progress_);
+  migration_pending_urls_.insert(top_sites_[index].url);
+
+  if (mock_history_service_) {
+    // Testing with a mockup.
+    // QueryMostVisitedURLs is not virtual, so we have to duplicate the code.
+    // This calls SetClientData.
+    mock_history_service_->GetPageThumbnail(
+        top_sites_[index].url,
+        &cancelable_consumer_,
+        NewCallback(this, &TopSites::OnThumbnailAvailable),
+        index);
+    return;
+  }
+
   HistoryService* hs = profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
   // |hs| may be null during unit tests.
   if (!hs)
@@ -355,6 +387,11 @@ void TopSites::StartQueryForMostVisited() {
   }
 }
 
+void TopSites::StartMigration() {
+  migration_in_progress_ = true;
+  StartQueryForMostVisited();
+}
+
 base::TimeDelta TopSites::GetUpdateDelay() {
   if (top_sites_.size() == 0)
     return base::TimeDelta::FromSeconds(30);
@@ -374,13 +411,26 @@ void TopSites::OnTopSitesAvailable(
 
 void TopSites::OnThumbnailAvailable(CancelableRequestProvider::Handle handle,
                                     scoped_refptr<RefCountedBytes> thumbnail) {
-  HistoryService* hs = profile_ ->GetHistoryService(Profile::EXPLICIT_ACCESS);
-  size_t index = cancelable_consumer_.GetClientData(hs, handle);
+  size_t index;
+  if (mock_history_service_) {
+    index = handle;
+  } else {
+    HistoryService* hs = profile_ ->GetHistoryService(Profile::EXPLICIT_ACCESS);
+    index = cancelable_consumer_.GetClientData(hs, handle);
+  }
   DCHECK(static_cast<size_t>(index) < top_sites_.size());
-  if (!thumbnail.get() || !thumbnail->size())
-    return;
-  const MostVisitedURL& url = top_sites_[index];
-  SetPageThumbnail(url.url, thumbnail, ThumbnailScore());
+
+  if (migration_in_progress_)
+    migration_pending_urls_.erase(top_sites_[index].url);
+
+  if (thumbnail.get() && thumbnail->size()) {
+    const MostVisitedURL& url = top_sites_[index];
+    SetPageThumbnail(url.url, thumbnail, ThumbnailScore());
+  }
+
+  if (migration_in_progress_ && migration_pending_urls_.empty() &&
+      !mock_history_service_)
+    OnMigrationDone();
 }
 
 void TopSites::SetMockHistoryService(MockHistoryService* mhs) {
diff --git a/chrome/browser/history/top_sites.h b/chrome/browser/history/top_sites.h
index 95f512c..a1eb531 100644
--- a/chrome/browser/history/top_sites.h
+++ b/chrome/browser/history/top_sites.h
@@ -57,6 +57,11 @@ class TopSites : public NotificationObserver,
         CancelableRequestConsumerBase* consumer,
         HistoryService::QueryMostVisitedURLsCallback* callback) = 0;
     virtual ~MockHistoryService() {}
+    virtual void GetPageThumbnail(
+        const GURL& page_url,
+        CancelableRequestConsumerTSimple<size_t>* consumer,
+        HistoryService::ThumbnailDataCallback* callback,
+        size_t index) = 0;
   };
 
   struct Images {
@@ -84,6 +89,12 @@ class TopSites : public NotificationObserver,
   // Get a thumbnail for a given page. Returns true iff we have the thumbnail.
   bool GetPageThumbnail(const GURL& url, RefCountedBytes** data) const;
 
+  // For testing with a HistoryService mock.
+  void SetMockHistoryService(MockHistoryService* mhs);
+
+  // Start reading thumbnails from the ThumbnailDatabase.
+  void StartMigration();
+
  private:
   friend class base::RefCountedThreadSafe<TopSites>;
   friend class TopSitesTest;
@@ -92,6 +103,7 @@ class TopSites : public NotificationObserver,
   friend class TopSitesTest_MockDatabase_Test;
   friend class TopSitesTest_DeleteNotifications_Test;
   friend class TopSitesTest_GetUpdateDelay_Test;
+  friend class TopSitesTest_Migration_Test;
 
   ~TopSites();
 
@@ -108,10 +120,10 @@ class TopSites : public NotificationObserver,
                         const RefCountedBytes* thumbnail,
                         const ThumbnailScore& score);
 
-  // Query history service for the list of available thumnbails.
+  // Query history service for the list of available thumbnails.
   void StartQueryForMostVisited();
 
-  // Query history service for the thumnbail for a given url. |index|
+  // Query history service for the thumbnail for a given url. |index|
   // is the index into top_sites_.
   void StartQueryForThumbnail(size_t index);
 
@@ -121,7 +133,7 @@ class TopSites : public NotificationObserver,
 
   // Called when history service returns a thumbnail.
   void OnThumbnailAvailable(CancelableRequestProvider::Handle handle,
-                            scoped_refptr<RefCountedBytes> thumnbail);
+                            scoped_refptr<RefCountedBytes> thumbnail);
 
   // Saves the set of the top URLs visited by this user. The 0th item is the
   // most popular.
@@ -160,9 +172,6 @@ class TopSites : public NotificationObserver,
                               std::vector<size_t>* deleted_urls,
                               std::vector<size_t>* moved_urls);
 
-  // For testing with a HistoryService mock.
-  void SetMockHistoryService(MockHistoryService* mhs);
-
   // Implementation of NotificationObserver.
   virtual void Observe(NotificationType type,
                        const NotificationSource& source,
@@ -190,6 +199,9 @@ class TopSites : public NotificationObserver,
   // Deletes the database file, then reinitializes the database.
   void ResetDatabase();
 
+  // Called after TopSites completes migration.
+  void OnMigrationDone();
+
   Profile* profile_;
   // A mockup to use for testing. If NULL, use the real HistoryService
   // from the profile_. See SetMockHistoryService.
@@ -221,6 +233,13 @@ class TopSites : public NotificationObserver,
   // The number of URLs changed on the last update.
   size_t last_num_urls_changed_;
 
+  // Are we in the middle of migration from ThumbnailsDatabase to
+  // TopSites?
+  bool migration_in_progress_;
+
+  // URLs for which we are expecting thumbnails.
+  std::set<GURL> migration_pending_urls_;
+
   // TODO(brettw): use the blacklist.
   // std::set<GURL> blacklist_;
 
diff --git a/chrome/browser/history/top_sites_unittest.cc b/chrome/browser/history/top_sites_unittest.cc
index f3406e5..a383ee8 100644
--- a/chrome/browser/history/top_sites_unittest.cc
+++ b/chrome/browser/history/top_sites_unittest.cc
@@ -6,6 +6,7 @@
 #include "base/string_util.h"
 #include "chrome/browser/history/top_sites.h"
 #include "chrome/common/chrome_paths.h"
+#include "chrome/browser/history/history_marshaling.h"
 #include "chrome/browser/history/top_sites_database.h"
 #include "chrome/browser/history/history_notifications.h"
 #include "chrome/test/testing_profile.h"
@@ -98,6 +99,8 @@ class TopSitesTest : public testing::Test {
 // A mockup of a HistoryService used for testing TopSites.
 class MockHistoryServiceImpl : public TopSites::MockHistoryService {
  public:
+  MockHistoryServiceImpl() : num_thumbnail_requests_(0) {}
+
   // Calls the callback directly with the results.
   HistoryService::Handle QueryMostVisitedURLs(
       int result_count, int days_back,
@@ -125,8 +128,34 @@ class MockHistoryServiceImpl : public TopSites::MockHistoryService {
     most_visited_urls_.pop_back();
   }
 
+  virtual void GetPageThumbnail(
+      const GURL& url,
+      CancelableRequestConsumerTSimple<size_t>* consumer,
+      HistoryService::ThumbnailDataCallback* callback,
+      size_t index) {
+    num_thumbnail_requests_++;
+    MostVisitedURL mvu;
+    mvu.url = url;
+    MostVisitedURLList::iterator pos = std::find(most_visited_urls_.begin(),
+                                                 most_visited_urls_.end(),
+                                                 mvu);
+    EXPECT_TRUE(pos != most_visited_urls_.end());
+    scoped_refptr<RefCountedBytes> thumbnail;
+    callback->Run(index, thumbnail);
+    delete callback;
+  }
+
+  void ResetNumberOfThumbnailRequests() {
+    num_thumbnail_requests_ = 0;
+  }
+
+  int GetNumberOfThumbnailRequests() {
+    return num_thumbnail_requests_;
+  }
+
  private:
   MostVisitedURLList most_visited_urls_;
+  int num_thumbnail_requests_;  // Number of calls to GetPageThumbnail.
 };
 
 
@@ -708,4 +737,28 @@ TEST_F(TopSitesTest, GetUpdateDelay) {
   EXPECT_EQ(1, top_sites().GetUpdateDelay().InMinutes());
 }
 
+TEST_F(TopSitesTest, Migration) {
+  ChromeThread db_loop(ChromeThread::DB, MessageLoop::current());
+  GURL google1_url("http://google.com");
+  GURL google2_url("http://google.com/redirect");
+  GURL google3_url("http://www.google.com");
+  string16 google_title(ASCIIToUTF16("Google"));
+  GURL news_url("http://news.google.com");
+  string16 news_title(ASCIIToUTF16("Google News"));
+
+  MockHistoryServiceImpl hs;
+
+  top_sites().Init(file_name());
+
+  hs.AppendMockPage(google1_url, google_title);
+  hs.AppendMockPage(news_url, news_title);
+  top_sites().SetMockHistoryService(&hs);
+
+  top_sites().StartMigration();
+  EXPECT_TRUE(top_sites().migration_in_progress_);
+  MessageLoop::current()->RunAllPending();
+  EXPECT_EQ(2, hs.GetNumberOfThumbnailRequests());
+  EXPECT_FALSE(top_sites().migration_in_progress_);
+}
+
 }  // namespace history