Change ownership of BookmarkClient.

BookmarkModel now owns the BookmarkClient (there is no real need for
BookmarkClient to be a KeyedService). As was found in other components
having the client be owned by the service reduce the risk of breaking
the componentization.

BUG=None

Review URL: https://codereview.chromium.org/1586013002

Cr-Commit-Position: refs/heads/master@{#372654}

9 files changed, 11 insertions, 136 deletions

diff --git a/chrome/browser/bookmarks/bookmark_model_factory.cc b/chrome/browser/bookmarks/bookmark_model_factory.cc
index 341829c..36f85f0 100644
--- a/chrome/browser/bookmarks/bookmark_model_factory.cc
+++ b/chrome/browser/bookmarks/bookmark_model_factory.cc
@@ -11,7 +11,7 @@
 #include "base/values.h"
 #include "build/build_config.h"
 #include "chrome/browser/bookmarks/chrome_bookmark_client.h"
-#include "chrome/browser/bookmarks/chrome_bookmark_client_factory.h"
+#include "chrome/browser/bookmarks/managed_bookmark_service_factory.h"
 #include "chrome/browser/bookmarks/startup_task_runner_service_factory.h"
 #include "chrome/browser/profiles/incognito_helpers.h"
 #include "chrome/browser/profiles/profile.h"
@@ -56,7 +56,7 @@ BookmarkModelFactory::BookmarkModelFactory()
         "BookmarkModel",
         BrowserContextDependencyManager::GetInstance()) {
   DependsOn(BookmarkUndoServiceFactory::GetInstance());
-  DependsOn(ChromeBookmarkClientFactory::GetInstance());
+  DependsOn(ManagedBookmarkServiceFactory::GetInstance());
   DependsOn(StartupTaskRunnerServiceFactory::GetInstance());
 #if BUILDFLAG(ANDROID_JAVA_UI)
   if (offline_pages::IsOfflinePagesEnabled())
@@ -70,10 +70,9 @@ BookmarkModelFactory::~BookmarkModelFactory() {
 KeyedService* BookmarkModelFactory::BuildServiceInstanceFor(
     content::BrowserContext* context) const {
   Profile* profile = Profile::FromBrowserContext(context);
-  ChromeBookmarkClient* bookmark_client =
-      ChromeBookmarkClientFactory::GetForProfile(profile);
-  BookmarkModel* bookmark_model = new BookmarkModel(bookmark_client);
-  bookmark_client->Init(bookmark_model);
+  BookmarkModel* bookmark_model =
+      new BookmarkModel(make_scoped_ptr(new ChromeBookmarkClient(
+          profile, ManagedBookmarkServiceFactory::GetForProfile(profile))));
   bookmark_model->Load(profile->GetPrefs(),
                        profile->GetPrefs()->GetString(prefs::kAcceptLanguages),
                        profile->GetPath(),
diff --git a/chrome/browser/bookmarks/chrome_bookmark_client.cc b/chrome/browser/bookmarks/chrome_bookmark_client.cc
index afe3bc4..8666f81 100644
--- a/chrome/browser/bookmarks/chrome_bookmark_client.cc
+++ b/chrome/browser/bookmarks/chrome_bookmark_client.cc
@@ -32,11 +32,6 @@ void ChromeBookmarkClient::Init(bookmarks::BookmarkModel* model) {
     managed_bookmark_service_->BookmarkModelCreated(model);
 }
 
-void ChromeBookmarkClient::Shutdown() {
-  managed_bookmark_service_ = nullptr;
-  BookmarkClient::Shutdown();
-}
-
 bool ChromeBookmarkClient::PreferTouchIcon() {
   return false;
 }
diff --git a/chrome/browser/bookmarks/chrome_bookmark_client.h b/chrome/browser/bookmarks/chrome_bookmark_client.h
index 3fdde2c..621f408 100644
--- a/chrome/browser/bookmarks/chrome_bookmark_client.h
+++ b/chrome/browser/bookmarks/chrome_bookmark_client.h
@@ -33,12 +33,8 @@ class ChromeBookmarkClient : public bookmarks::BookmarkClient {
       bookmarks::ManagedBookmarkService* managed_bookmark_service);
   ~ChromeBookmarkClient() override;
 
-  void Init(bookmarks::BookmarkModel* model);
-
-  // KeyedService:
-  void Shutdown() override;
-
   // bookmarks::BookmarkClient:
+  void Init(bookmarks::BookmarkModel* model) override;
   bool PreferTouchIcon() override;
   base::CancelableTaskTracker::TaskId GetFaviconImageForPageURL(
       const GURL& page_url,
diff --git a/chrome/browser/bookmarks/chrome_bookmark_client_factory.cc b/chrome/browser/bookmarks/chrome_bookmark_client_factory.cc
deleted file mode 100644
index c84a374..0000000
--- a/chrome/browser/bookmarks/chrome_bookmark_client_factory.cc
+++ /dev/null
@@ -1,66 +0,0 @@
-// Copyright 2014 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/browser/bookmarks/chrome_bookmark_client_factory.h"
-
-#include "base/memory/singleton.h"
-#include "chrome/browser/bookmarks/chrome_bookmark_client.h"
-#include "chrome/browser/bookmarks/managed_bookmark_service_factory.h"
-#include "chrome/browser/profiles/incognito_helpers.h"
-#include "chrome/browser/profiles/profile.h"
-#include "components/bookmarks/managed/managed_bookmark_service.h"
-#include "components/keyed_service/content/browser_context_dependency_manager.h"
-
-namespace {
-
-scoped_ptr<KeyedService> BuildChromeBookmarkClient(
-    content::BrowserContext* context) {
-  Profile* profile = Profile::FromBrowserContext(context);
-  return make_scoped_ptr(new ChromeBookmarkClient(
-      profile, ManagedBookmarkServiceFactory::GetForProfile(profile)));
-}
-
-}  // namespace
-
-// static
-ChromeBookmarkClient* ChromeBookmarkClientFactory::GetForProfile(
-    Profile* profile) {
-  return static_cast<ChromeBookmarkClient*>(
-      GetInstance()->GetServiceForBrowserContext(profile, true));
-}
-
-// static
-ChromeBookmarkClientFactory* ChromeBookmarkClientFactory::GetInstance() {
-  return base::Singleton<ChromeBookmarkClientFactory>::get();
-}
-
-// static
-BrowserContextKeyedServiceFactory::TestingFactoryFunction
-ChromeBookmarkClientFactory::GetDefaultFactory() {
-  return &BuildChromeBookmarkClient;
-}
-
-ChromeBookmarkClientFactory::ChromeBookmarkClientFactory()
-    : BrowserContextKeyedServiceFactory(
-          "ChromeBookmarkClient",
-          BrowserContextDependencyManager::GetInstance()) {
-  DependsOn(ManagedBookmarkServiceFactory::GetInstance());
-}
-
-ChromeBookmarkClientFactory::~ChromeBookmarkClientFactory() {
-}
-
-KeyedService* ChromeBookmarkClientFactory::BuildServiceInstanceFor(
-    content::BrowserContext* context) const {
-  return BuildChromeBookmarkClient(context).release();
-}
-
-content::BrowserContext* ChromeBookmarkClientFactory::GetBrowserContextToUse(
-    content::BrowserContext* context) const {
-  return chrome::GetBrowserContextRedirectedInIncognito(context);
-}
-
-bool ChromeBookmarkClientFactory::ServiceIsNULLWhileTesting() const {
-  return true;
-}
diff --git a/chrome/browser/bookmarks/chrome_bookmark_client_factory.h b/chrome/browser/bookmarks/chrome_bookmark_client_factory.h
deleted file mode 100644
index 6976098..0000000
--- a/chrome/browser/bookmarks/chrome_bookmark_client_factory.h
+++ /dev/null
@@ -1,43 +0,0 @@
-// Copyright 2014 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.
-
-#ifndef CHROME_BROWSER_BOOKMARKS_CHROME_BOOKMARK_CLIENT_FACTORY_H_
-#define CHROME_BROWSER_BOOKMARKS_CHROME_BOOKMARK_CLIENT_FACTORY_H_
-
-#include "base/macros.h"
-#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
-
-namespace base {
-template <typename T>
-struct DefaultSingletonTraits;
-}  // namespace base
-
-class ChromeBookmarkClient;
-class Profile;
-
-// Singleton that owns all ChromeBookmarkClients and associates them with
-// Profiles.
-class ChromeBookmarkClientFactory : public BrowserContextKeyedServiceFactory {
- public:
-  static ChromeBookmarkClient* GetForProfile(Profile* profile);
-  static ChromeBookmarkClientFactory* GetInstance();
-  static TestingFactoryFunction GetDefaultFactory();
-
- private:
-  friend struct base::DefaultSingletonTraits<ChromeBookmarkClientFactory>;
-
-  ChromeBookmarkClientFactory();
-  ~ChromeBookmarkClientFactory() override;
-
-  // BrowserContextKeyedServiceFactory:
-  KeyedService* BuildServiceInstanceFor(
-      content::BrowserContext* context) const override;
-  content::BrowserContext* GetBrowserContextToUse(
-      content::BrowserContext* context) const override;
-  bool ServiceIsNULLWhileTesting() const override;
-
-  DISALLOW_COPY_AND_ASSIGN(ChromeBookmarkClientFactory);
-};
-
-#endif  // CHROME_BROWSER_BOOKMARKS_CHROME_BOOKMARK_CLIENT_FACTORY_H_
diff --git a/chrome/browser/sync/profile_sync_service_bookmark_unittest.cc b/chrome/browser/sync/profile_sync_service_bookmark_unittest.cc
index 04a1e10..2423538 100644
--- a/chrome/browser/sync/profile_sync_service_bookmark_unittest.cc
+++ b/chrome/browser/sync/profile_sync_service_bookmark_unittest.cc
@@ -28,7 +28,6 @@
 #include "build/build_config.h"
 #include "chrome/browser/bookmarks/bookmark_model_factory.h"
 #include "chrome/browser/bookmarks/chrome_bookmark_client.h"
-#include "chrome/browser/bookmarks/chrome_bookmark_client_factory.h"
 #include "chrome/browser/bookmarks/managed_bookmark_service_factory.h"
 #include "chrome/browser/favicon/favicon_service_factory.h"
 #include "chrome/browser/history/history_service_factory.h"
diff --git a/chrome/browser/ui/bookmarks/bookmark_editor_unittest.cc b/chrome/browser/ui/bookmarks/bookmark_editor_unittest.cc
index 1f51ee7..ad9381d 100644
--- a/chrome/browser/ui/bookmarks/bookmark_editor_unittest.cc
+++ b/chrome/browser/ui/bookmarks/bookmark_editor_unittest.cc
@@ -16,8 +16,7 @@ using bookmarks::BookmarkNode;
 namespace {
 
 TEST(BookmarkEditorTest, ApplyEditsWithNoFolderChange) {
-  bookmarks::TestBookmarkClient client;
-  scoped_ptr<BookmarkModel> model(client.CreateModel());
+  scoped_ptr<BookmarkModel> model(bookmarks::TestBookmarkClient::CreateModel());
   const BookmarkNode* bookmarkbar = model->bookmark_bar_node();
   model->AddURL(bookmarkbar, 0, ASCIIToUTF16("url0"), GURL("chrome://newtab"));
   model->AddURL(bookmarkbar, 1, ASCIIToUTF16("url1"), GURL("chrome://newtab"));
diff --git a/chrome/browser/ui/bookmarks/bookmark_ui_utils_desktop_unittest.cc b/chrome/browser/ui/bookmarks/bookmark_ui_utils_desktop_unittest.cc
index e297064..03797a8 100644
--- a/chrome/browser/ui/bookmarks/bookmark_ui_utils_desktop_unittest.cc
+++ b/chrome/browser/ui/bookmarks/bookmark_ui_utils_desktop_unittest.cc
@@ -22,8 +22,7 @@ class BookmarkUIUtilsTest : public testing::Test {
 };
 
 TEST_F(BookmarkUIUtilsTest, HasBookmarkURLs) {
-  bookmarks::TestBookmarkClient client;
-  scoped_ptr<BookmarkModel> model(client.CreateModel());
+  scoped_ptr<BookmarkModel> model(bookmarks::TestBookmarkClient::CreateModel());
 
   std::vector<const BookmarkNode*> nodes;
 
@@ -64,8 +63,7 @@ TEST_F(BookmarkUIUtilsTest, HasBookmarkURLs) {
 }
 
 TEST_F(BookmarkUIUtilsTest, HasBookmarkURLsAllowedInIncognitoMode) {
-  bookmarks::TestBookmarkClient client;
-  scoped_ptr<BookmarkModel> model(client.CreateModel());
+  scoped_ptr<BookmarkModel> model(bookmarks::TestBookmarkClient::CreateModel());
   TestingProfile profile;
 
   std::vector<const BookmarkNode*> nodes;
diff --git a/chrome/browser/ui/bookmarks/recently_used_folders_combo_model_unittest.cc b/chrome/browser/ui/bookmarks/recently_used_folders_combo_model_unittest.cc
index 45b66eb..952026f 100644
--- a/chrome/browser/ui/bookmarks/recently_used_folders_combo_model_unittest.cc
+++ b/chrome/browser/ui/bookmarks/recently_used_folders_combo_model_unittest.cc
@@ -64,8 +64,7 @@ RecentlyUsedFoldersComboModelTest::RecentlyUsedFoldersComboModelTest()
 
 // Verifies there are no duplicate nodes in the model.
 TEST_F(RecentlyUsedFoldersComboModelTest, NoDups) {
-  TestBookmarkClient client;
-  scoped_ptr<BookmarkModel> bookmark_model(client.CreateModel());
+  scoped_ptr<BookmarkModel> bookmark_model(TestBookmarkClient::CreateModel());
   const BookmarkNode* new_node = bookmark_model->AddURL(
       bookmark_model->bookmark_bar_node(), 0, base::ASCIIToUTF16("a"),
       GURL("http://a"));
@@ -79,8 +78,7 @@ TEST_F(RecentlyUsedFoldersComboModelTest, NoDups) {
 
 // Verifies that observers are notified on changes.
 TEST_F(RecentlyUsedFoldersComboModelTest, NotifyObserver) {
-  TestBookmarkClient client;
-  scoped_ptr<BookmarkModel> bookmark_model(client.CreateModel());
+  scoped_ptr<BookmarkModel> bookmark_model(TestBookmarkClient::CreateModel());
   const BookmarkNode* folder = bookmark_model->AddFolder(
       bookmark_model->bookmark_bar_node(), 0, base::ASCIIToUTF16("a"));
   const BookmarkNode* sub_folder = bookmark_model->AddFolder(