[Icons NTP] Refactor large_icon_source to extract the logic shared between desktop and Android to f…

chrome/browser/favicon/large_icon_service_factory.cc

Index: chrome/browser/favicon/large_icon_service_factory.cc
diff --git a/chrome/browser/favicon/chrome_fallback_icon_client_factory.cc b/chrome/browser/favicon/large_icon_service_factory.cc
similarity index 39%
copy from chrome/browser/favicon/chrome_fallback_icon_client_factory.cc
copy to chrome/browser/favicon/large_icon_service_factory.cc
index f7e41cd3d88c998518db074a28ea461a2357b7f9..59ce3334fbc0c6a0cb28f1376bc4c944a918d27b 100644
--- a/chrome/browser/favicon/chrome_fallback_icon_client_factory.cc
+++ b/chrome/browser/favicon/large_icon_service_factory.cc
@@ -2,38 +2,43 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/favicon/chrome_fallback_icon_client_factory.h"
+#include "chrome/browser/favicon/large_icon_service_factory.h"
 
 #include "base/memory/singleton.h"
-#include "chrome/browser/favicon/chrome_fallback_icon_client.h"
-#include "chrome/browser/profiles/incognito_helpers.h"
+#include "chrome/browser/favicon/favicon_service_factory.h"
+#include "components/favicon/core/favicon_service.h"
+#include "components/favicon/core/large_icon_service.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
 #include "content/public/browser/browser_context.h"
 
-ChromeFallbackIconClientFactory::ChromeFallbackIconClientFactory()
-    : BrowserContextKeyedServiceFactory(
-        "ChromeFallbackIconClient",
-        BrowserContextDependencyManager::GetInstance()) {
-}
-
-ChromeFallbackIconClientFactory::~ChromeFallbackIconClientFactory() {
-}
-
 // static
-favicon::FallbackIconClient*
-ChromeFallbackIconClientFactory::GetForBrowserContext(
+favicon::LargeIconService* LargeIconServiceFactory::GetForBrowserContext(
     content::BrowserContext* context) {
-  return static_cast<favicon::FallbackIconClient*>(
+  return static_cast<favicon::LargeIconService*>(
       GetInstance()->GetServiceForBrowserContext(context, true));
 }
 
 // static
-ChromeFallbackIconClientFactory*
-ChromeFallbackIconClientFactory::GetInstance() {
-  return Singleton<ChromeFallbackIconClientFactory>::get();
+LargeIconServiceFactory* LargeIconServiceFactory::GetInstance() {
+  return Singleton<LargeIconServiceFactory>::get();
 }
 
-KeyedService* ChromeFallbackIconClientFactory::BuildServiceInstanceFor(
+LargeIconServiceFactory::LargeIconServiceFactory()
+    : BrowserContextKeyedServiceFactory(
+        "LargeIconService",
+        BrowserContextDependencyManager::GetInstance()) {
+  DependsOn(FaviconServiceFactory::GetInstance());
+}
+
+LargeIconServiceFactory::~LargeIconServiceFactory() {}
+
+KeyedService* LargeIconServiceFactory::BuildServiceInstanceFor(
     content::BrowserContext* context) const {
-  return new ChromeFallbackIconClient();
+  favicon::FaviconService* favicon_service =
+      FaviconServiceFactory::GetForBrowserContext(context);

[pkotwicz, 2015/04/21 18:19:48]

Can't you pass in the profile (You can get the profile with
Profile::FromBrowserContext()) and EXPLICIT_ACCESS?
I am unsure why you added a new accessor to FaviconServiceFactory.

I have not checked but it looks like this might produce interesting behavior in
incognito mode

[beaudoin, 2015/04/21 19:03:22]

Reverted back to the approach I was using in:
https://codereview.chromium.org/1092873002/diff/160001/chrome/browser/favicon...

Discussed with huangs@ and he's OK with the revert.

+  return new favicon::LargeIconService(favicon_service);
+}
+
+bool LargeIconServiceFactory::ServiceIsNULLWhileTesting() const {
+  return true;
 }