Makes TopSites not DCHECK if HistoryLoaded or MigrateFromHistory is

invoked as the result of unloading history. I've convinced myself
ignoring these are ok. If unloading happens before migration TopSites
is going to trigger history to be loaded again to complete migration,
but that's rare and not worth trying to deal with. That said, TopSites
really needs some unload code, but that's a separate issue.

BUG=93721
TEST=covered by tests, but make sure you don't see anything weird with
most visited on new tab page.
R=brettw@chromium.org

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

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

3 files changed, 61 insertions, 4 deletions

diff --git a/chrome/browser/history/top_sites.cc b/chrome/browser/history/top_sites.cc
index 99b84dc..21994e2 100644
--- a/chrome/browser/history/top_sites.cc
+++ b/chrome/browser/history/top_sites.cc
@@ -286,7 +286,11 @@ static int IndexOf(const MostVisitedURLList& urls, const GURL& url) {
 
 void TopSites::MigrateFromHistory() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK_EQ(history_state_, HISTORY_LOADING);
+
+  if (history_state_ != HISTORY_LOADING) {
+    // This can happen if history was unloaded then loaded again.
+    return;
+  }
 
   history_state_ = HISTORY_MIGRATING;
   profile_->GetHistoryService(Profile::EXPLICIT_ACCESS)->ScheduleDBTask(
@@ -330,7 +334,6 @@ void TopSites::FinishHistoryMigration(const ThumbnailMigration& data) {
 
 void TopSites::HistoryLoaded() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK_NE(history_state_, HISTORY_LOADED);
 
   if (history_state_ != HISTORY_MIGRATING) {
     // No migration from history is needed.
@@ -343,6 +346,7 @@ void TopSites::HistoryLoaded() {
       MoveStateToLoaded();
     }
   }
+  // else case can happen if history is unloaded, then loaded again.
 }
 
 void TopSites::SyncWithHistory() {
diff --git a/chrome/browser/history/top_sites.h b/chrome/browser/history/top_sites.h
index cc1bb55..14c7c03 100644
--- a/chrome/browser/history/top_sites.h
+++ b/chrome/browser/history/top_sites.h
@@ -184,7 +184,9 @@ class TopSites
   typedef std::pair<GURL, Images> TempImage;
   typedef std::list<TempImage> TempImages;
 
-  // Enumeration of the possible states history can be in.
+  // Enumeration of the possible states history can be in. These values do not
+  // necessarily reflect the loaded state of history. In particular if the
+  // history backend is unloaded |history_state_| may be HISTORY_LOADED.
   enum HistoryLoadState {
     // We're waiting for history to finish loading.
     HISTORY_LOADING,
diff --git a/chrome/browser/history/top_sites_unittest.cc b/chrome/browser/history/top_sites_unittest.cc
index eefa4fe..6f9f572 100644
--- a/chrome/browser/history/top_sites_unittest.cc
+++ b/chrome/browser/history/top_sites_unittest.cc
@@ -21,9 +21,11 @@
 #include "chrome/browser/history/top_sites_database.h"
 #include "chrome/browser/ui/webui/ntp/most_visited_handler.h"
 #include "chrome/common/chrome_constants.h"
+#include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/test/base/testing_profile.h"
+#include "chrome/test/base/ui_test_utils.h"
 #include "chrome/tools/profiles/thumbnail-inl.h"
 #include "content/browser/browser_thread.h"
 #include "googleurl/src/gurl.h"
@@ -374,7 +376,6 @@ class TopSitesMigrationTest : public TopSitesTest {
   }
 
  private:
-
   DISALLOW_COPY_AND_ASSIGN(TopSitesMigrationTest);
 };
 
@@ -1336,4 +1337,54 @@ TEST_F(TopSitesTest, CreateTopSitesThenHistory) {
   EXPECT_TRUE(IsTopSitesLoaded());
 }
 
+class TopSitesUnloadTest : public TopSitesTest {
+ public:
+  TopSitesUnloadTest() {}
+
+  virtual bool CreateHistoryAndTopSites() OVERRIDE {
+    return false;
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TopSitesUnloadTest);
+};
+
+// Makes sure if history is unloaded after topsites is loaded we don't hit any
+// assertions.
+TEST_F(TopSitesUnloadTest, UnloadHistoryTest) {
+  profile()->CreateHistoryService(false, false);
+  profile()->CreateTopSites();
+  profile()->BlockUntilTopSitesLoaded();
+  profile()->GetHistoryService(Profile::EXPLICIT_ACCESS)->UnloadBackend();
+  profile()->BlockUntilHistoryProcessesPendingRequests();
+}
+
+// Makes sure if history (with migration code) is unloaded after topsites is
+// loaded we don't hit any assertions.
+TEST_F(TopSitesUnloadTest, UnloadWithMigration) {
+  // Set up history and thumbnails as they would be before migration.
+  FilePath data_path;
+  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &data_path));
+  data_path = data_path.AppendASCII("top_sites");
+  ASSERT_NO_FATAL_FAILURE(ExecuteSQLScript(
+      data_path.AppendASCII("history.19.sql"),
+      profile()->GetPath().Append(chrome::kHistoryFilename)));
+  ASSERT_NO_FATAL_FAILURE(ExecuteSQLScript(
+      data_path.AppendASCII("thumbnails.3.sql"),
+      profile()->GetPath().Append(chrome::kThumbnailsFilename)));
+
+  // Create history and block until its loaded.
+  profile()->CreateHistoryService(false, false);
+  profile()->BlockUntilHistoryProcessesPendingRequests();
+
+  // Create top sites and unload history.
+  ui_test_utils::WindowedNotificationObserver observer(
+      chrome::NOTIFICATION_TOP_SITES_LOADED,
+      Source<Profile>(profile()));
+  profile()->CreateTopSites();
+  profile()->GetHistoryService(Profile::EXPLICIT_ACCESS)->UnloadBackend();
+  profile()->BlockUntilHistoryProcessesPendingRequests();
+  observer.Wait();
+}
+
 }  // namespace history