Abstract history dependencies on bookmarks through HistoryClient

chrome/browser/history/history_backend_unittest.cc

Index: chrome/browser/history/history_backend_unittest.cc
diff --git a/chrome/browser/history/history_backend_unittest.cc b/chrome/browser/history/history_backend_unittest.cc
index 5ab9a3a94bc152349b25b67aef4750a7bdef9cfb..254b60f21d20bbfef4aab41145cde3cab81b3cd4 100644
--- a/chrome/browser/history/history_backend_unittest.cc
+++ b/chrome/browser/history/history_backend_unittest.cc
@@ -20,7 +20,6 @@
 #include "base/strings/string16.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
-#include "chrome/browser/bookmarks/bookmark_model_factory.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/history/history_notifications.h"
 #include "chrome/browser/history/history_service.h"
@@ -32,10 +31,7 @@
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/importer/imported_favicon_usage.h"
 #include "chrome/test/base/testing_profile.h"
-#include "components/bookmarks/browser/bookmark_model.h"
-#include "components/bookmarks/browser/bookmark_utils.h"
-#include "components/bookmarks/test/bookmark_test_helpers.h"
-#include "components/bookmarks/test/test_bookmark_client.h"
+#include "components/history/core/browser/history_client_mock_bookmarks.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/test/test_browser_thread.h"
@@ -116,8 +112,7 @@ class HistoryBackendTestBase : public testing::Test {
   typedef std::vector<std::pair<int, HistoryDetails*> > NotificationList;
 
   HistoryBackendTestBase()
-      : bookmark_model_(bookmark_client_.CreateModel(false)),
-        loaded_(false),
+      : loaded_(false),
         ui_thread_(content::BrowserThread::UI, &message_loop_) {}
 
   virtual ~HistoryBackendTestBase() {
@@ -152,10 +147,9 @@ class HistoryBackendTestBase : public testing::Test {
         std::make_pair(type, details.release()));
   }
 
-  test::TestBookmarkClient bookmark_client_;
+  history::HistoryClientMockBookmarks history_client_;
   scoped_refptr<HistoryBackend> backend_;  // Will be NULL on init failure.
   scoped_ptr<InMemoryHistoryBackend> mem_backend_;
-  scoped_ptr<BookmarkModel> bookmark_model_;
   bool loaded_;
 
  private:
@@ -167,7 +161,7 @@ class HistoryBackendTestBase : public testing::Test {
                                       &test_dir_))
       return;
     backend_ = new HistoryBackend(
-        test_dir_, new HistoryBackendTestDelegate(this), bookmark_model_.get());
+        test_dir_, new HistoryBackendTestDelegate(this), &history_client_);
     backend_->Init(std::string(), false);
   }
 
@@ -178,6 +172,7 @@ class HistoryBackendTestBase : public testing::Test {
     mem_backend_.reset();
     base::DeleteFile(test_dir_, true);
     base::RunLoop().RunUntilIdle();
+    history_client_.ClearAllBookmarks();
   }
 
   void SetInMemoryBackend(scoped_ptr<InMemoryHistoryBackend> backend) {
@@ -611,8 +606,7 @@ TEST_F(HistoryBackendTest, DeleteAll) {
   EXPECT_TRUE(mem_backend_->db_->GetRowForURL(row1.url(), NULL));
 
   // Star row1.
-  bookmark_model_->AddURL(
-      bookmark_model_->bookmark_bar_node(), 0, base::string16(), row1.url());
+  history_client_.AddBookmark(row1.url());
 
   // Now finally clear all history.
   ClearBroadcastedNotifications();
@@ -676,7 +670,7 @@ TEST_F(HistoryBackendTest, DeleteAll) {
   EXPECT_EQ(out_favicon1, mappings[0].icon_id);
 
   // The first URL should still be bookmarked.
-  EXPECT_TRUE(bookmark_model_->IsBookmarked(row1.url()));
+  EXPECT_TRUE(history_client_.IsBookmarked(row1.url()));
 
   // Check that we fire the notification about all history having been deleted.
   ASSERT_EQ(1u, broadcasted_notifications().size());
@@ -774,10 +768,8 @@ TEST_F(HistoryBackendTest, URLsNoLongerBookmarked) {
   URLID row2_id = backend_->db_->GetRowForURL(row2.url(), NULL);
 
   // Star the two URLs.
-  bookmark_utils::AddIfNotBookmarked(
-      bookmark_model_.get(), row1.url(), base::string16());
-  bookmark_utils::AddIfNotBookmarked(
-      bookmark_model_.get(), row2.url(), base::string16());
+  history_client_.AddBookmark(row1.url());
+  history_client_.AddBookmark(row2.url());
 
   // Delete url 2. Because url 2 is starred this won't delete the URL, only
   // the visits.
@@ -795,7 +787,7 @@ TEST_F(HistoryBackendTest, URLsNoLongerBookmarked) {
                 favicon_url2, favicon_base::FAVICON, NULL));
 
   // Unstar row2.
-  bookmark_utils::RemoveAllBookmarks(bookmark_model_.get(), row2.url());
+  history_client_.DelBookmark(row2.url());
 
   // Tell the backend it was unstarred. We have to explicitly do this as
   // BookmarkModel isn't wired up to the backend during testing.
@@ -811,7 +803,8 @@ TEST_F(HistoryBackendTest, URLsNoLongerBookmarked) {
                 favicon_url2, favicon_base::FAVICON, NULL));
 
   // Unstar row 1.
-  bookmark_utils::RemoveAllBookmarks(bookmark_model_.get(), row1.url());
+  history_client_.DelBookmark(row1.url());
+
   // Tell the backend it was unstarred. We have to explicitly do this as
   // BookmarkModel isn't wired up to the backend during testing.
   unstarred_urls.clear();
@@ -1083,8 +1076,7 @@ TEST_F(HistoryBackendTest, ImportedFaviconsTest) {
   EXPECT_TRUE(backend_->db_->GetRowForURL(url3, &url_row3) == 0);
 
   // If the URL is bookmarked, it should get added to history with 0 visits.
-  bookmark_model_->AddURL(
-      bookmark_model_->bookmark_bar_node(), 0, base::string16(), url3);
+  history_client_.AddBookmark(url3);
   backend_->SetImportedFavicons(favicons);
   EXPECT_FALSE(backend_->db_->GetRowForURL(url3, &url_row3) == 0);
   EXPECT_TRUE(url_row3.visit_count() == 0);
@@ -1486,7 +1478,7 @@ TEST_F(HistoryBackendTest, MigrationVisitSource) {
 
   backend_ = new HistoryBackend(new_history_path,
                                 new HistoryBackendTestDelegate(this),
-                                bookmark_model_.get());
+                                &history_client_);
   backend_->Init(std::string(), false);
   backend_->Closing();
   backend_ = NULL;
@@ -2889,7 +2881,7 @@ TEST_F(HistoryBackendTest, MigrationVisitDuration) {
 
   backend_ = new HistoryBackend(new_history_path,
                                 new HistoryBackendTestDelegate(this),
-                                bookmark_model_.get());
+                                &history_client_);
   backend_->Init(std::string(), false);
   backend_->Closing();
   backend_ = NULL;
@@ -3125,17 +3117,12 @@ TEST_F(HistoryBackendTest, DeleteMatchingUrlsForKeyword) {
 TEST_F(HistoryBackendTest, RemoveNotification) {
   scoped_ptr<TestingProfile> profile(new TestingProfile());
 
-  ASSERT_TRUE(profile->CreateHistoryService(false, false));
-  profile->CreateBookmarkModel(true);
-  BookmarkModel* model = BookmarkModelFactory::GetForProfile(profile.get());
-  test::WaitForBookmarkModelToLoad(model);
-
   // Add a URL.
   GURL url("http://www.google.com");
-  bookmark_utils::AddIfNotBookmarked(model, url, base::string16());
-
-  HistoryService* service = HistoryServiceFactory::GetForProfile(
-      profile.get(), Profile::EXPLICIT_ACCESS);
+  history_client_.AddBookmark(url);
+  scoped_ptr<HistoryService> service(
+      new HistoryService(&history_client_, profile.get()));
+  EXPECT_TRUE(service->Init(profile->GetPath()));
 
   service->AddPage(
       url, base::Time::Now(), NULL, 1, GURL(), RedirectList(),
@@ -3143,6 +3130,8 @@ TEST_F(HistoryBackendTest, RemoveNotification) {
 
   // This won't actually delete the URL, rather it'll empty out the visits.
   // This triggers blocking on the BookmarkModel.
+  // TODO(sdefresne): mock the BlockTillBookmarksLoaded and check it has been
+  // called at least once!
   service->DeleteURL(url);
 }