Abstract history dependencies on bookmarks through HistoryClient

chrome/test/base/testing_profile.cc

 // 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.
 
 #include "chrome/test/base/testing_profile.h"
 
 #include "build/build_config.h"
 
 #include "base/base_paths.h"
 #include "base/command_line.h"
@@ skipping 11 matching lines @@
 #include "chrome/browser/content_settings/host_content_settings_map.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_special_storage_policy.h"
 #include "chrome/browser/extensions/extension_system_factory.h"
 #include "chrome/browser/extensions/test_extension_system.h"
 #include "chrome/browser/favicon/favicon_service.h"
 #include "chrome/browser/favicon/favicon_service_factory.h"
 #include "chrome/browser/geolocation/chrome_geolocation_permission_context.h"
 #include "chrome/browser/geolocation/chrome_geolocation_permission_context_factory.h"
 #include "chrome/browser/guest_view/guest_view_manager.h"
+#include "chrome/browser/history/chrome_history_client.h"
+#include "chrome/browser/history/chrome_history_client_factory.h"
 #include "chrome/browser/history/history_backend.h"
 #include "chrome/browser/history/history_db_task.h"
 #include "chrome/browser/history/history_service.h"
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/history/top_sites.h"
 #include "chrome/browser/history/web_history_service_factory.h"
 #include "chrome/browser/net/pref_proxy_config_tracker.h"
 #include "chrome/browser/net/proxy_service_factory.h"
 #include "chrome/browser/notifications/desktop_notification_service.h"
 #include "chrome/browser/notifications/desktop_notification_service_factory.h"
@@ skipping 376 matching lines @@
 static KeyedService* BuildFaviconService(content::BrowserContext* profile) {
   return new FaviconService(static_cast<Profile*>(profile));
 }
 
 void TestingProfile::CreateFaviconService() {
   // It is up to the caller to create the history service if one is needed.
   FaviconServiceFactory::GetInstance()->SetTestingFactory(
       this, BuildFaviconService);
 }
 
-static KeyedService* BuildHistoryService(content::BrowserContext* profile) {
-  return new HistoryService(NULL, static_cast<Profile*>(profile));
+static KeyedService* BuildHistoryService(content::BrowserContext* context) {
+  Profile* profile = static_cast<Profile*>(context);
+  return new HistoryService(
+      ChromeHistoryClientFactory::GetForProfile(profile), profile);
 }
 
 bool TestingProfile::CreateHistoryService(bool delete_file, bool no_db) {
   DestroyHistoryService();
   if (delete_file) {
     base::FilePath path = GetPath();
     path = path.Append(chrome::kHistoryFilename);
     if (!base::DeleteFile(path, false) || base::PathExists(path))
       return false;
   }
   // This will create and init the history service.
   HistoryService* history_service = static_cast<HistoryService*>(
       HistoryServiceFactory::GetInstance()->SetTestingFactoryAndUse(
           this, BuildHistoryService));
-  if (!history_service->Init(this->GetPath(),
-                             BookmarkModelFactory::GetForProfile(this),
-                             no_db)) {
+  if (!history_service->Init(this->GetPath(), no_db)) {
     HistoryServiceFactory::GetInstance()->SetTestingFactoryAndUse(this, NULL);
   }
   // Disable WebHistoryService by default, since it makes network requests.
   WebHistoryServiceFactory::GetInstance()->SetTestingFactory(this, NULL);
   return true;
 }
 
 void TestingProfile::DestroyHistoryService() {
   HistoryService* history_service =
       HistoryServiceFactory::GetForProfileWithoutCreating(this);
@@ skipping 43 matching lines @@
   bookmark_client->model()->Load(
       profile->GetPrefs(),
       profile->GetPrefs()->GetString(prefs::kAcceptLanguages),
       profile->GetPath(),
       profile->GetIOTaskRunner(),
       content::BrowserThread::GetMessageLoopProxyForThread(
           content::BrowserThread::UI));
   return bookmark_client;
 }
 
+static KeyedService* BuildChromeHistoryClient(
+    content::BrowserContext* context) {
+  Profile* profile = static_cast<Profile*>(context);
+  return new ChromeHistoryClient(
+      BookmarkModelFactory::GetForProfile(profile));

[blundell, 2014/06/02 13:02:36]

Is this needed? i.e., is it different from what ChromeHistoryClientFactory would
do otherwise?

[sdefresne, 2014/06/03 10:42:04]

Yes this is required, since ChromeHistoryClientFactory returns NULL for a
testing profile. And if I change it to returns non-NULL, then other tests
asserts as we try to create a ChromeHistoryClient when there is no BookmarkModel
created (ie. BookmarkModelFactory returns NULL for a testing profile).

+}
+
 void TestingProfile::CreateBookmarkModel(bool delete_file) {
   if (delete_file) {
     base::FilePath path = GetPath().Append(bookmarks::kBookmarksFileName);
     base::DeleteFile(path, false);
   }
   // This will create a bookmark model.
-  BookmarkModel* bookmark_service =
-      static_cast<ChromeBookmarkClient*>(
-          BookmarkModelFactory::GetInstance()->SetTestingFactoryAndUse(
-              this, BuildBookmarkModel))->model();
-
-  HistoryService* history_service =
-      HistoryServiceFactory::GetForProfileWithoutCreating(this);
-  if (history_service) {
-    history_service->history_backend_->bookmark_service_ = bookmark_service;
-    history_service->history_backend_->expirer_.bookmark_service_ =
-        bookmark_service;
-  }
+  ChromeHistoryClientFactory::GetInstance()->SetTestingFactory(
+      this, BuildChromeHistoryClient);
+  BookmarkModelFactory::GetInstance()->SetTestingFactoryAndUse(
+      this, BuildBookmarkModel);
 }
 
 static KeyedService* BuildWebDataService(content::BrowserContext* profile) {
   return new WebDataServiceWrapper(static_cast<Profile*>(profile));
 }
 
 void TestingProfile::CreateWebDataService() {
   WebDataServiceFactory::GetInstance()->SetTestingFactory(
       this, BuildWebDataService);
 }
@@ skipping 434 matching lines @@
       path_,
       delegate_,
       extension_policy_,
       pref_service_.Pass(),
       incognito_,
       guest_session_,
       managed_user_id_,
       policy_service_.Pass(),
       testing_factories_));
 }