[Fallback Icons] Refactor FallbackIconService to be a BrowserContext-level singleton

chrome/browser/search/instant_service.cc

 // Copyright 2013 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/search/instant_service.h"
 
 #include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/favicon/fallback_icon_service_factory.h"
+#include "chrome/browser/favicon/favicon_service_factory.h"
 #include "chrome/browser/history/top_sites_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search/instant_io_context.h"
 #include "chrome/browser/search/instant_service_observer.h"
 #include "chrome/browser/search/most_visited_iframe_source.h"
 #include "chrome/browser/search/search.h"
 #include "chrome/browser/search/suggestions/suggestions_source.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
 #include "chrome/browser/search_engines/ui_thread_search_terms_data.h"
 #include "chrome/browser/themes/theme_properties.h"
 #include "chrome/browser/themes/theme_service.h"
 #include "chrome/browser/themes/theme_service_factory.h"
 #include "chrome/browser/thumbnails/thumbnail_list_source.h"
 #include "chrome/browser/ui/search/instant_search_prerenderer.h"
 #include "chrome/browser/ui/webui/fallback_icon_source.h"
 #include "chrome/browser/ui/webui/favicon_source.h"
 #include "chrome/browser/ui/webui/large_icon_source.h"
 #include "chrome/browser/ui/webui/ntp/thumbnail_source.h"
 #include "chrome/browser/ui/webui/theme_source.h"
 #include "chrome/common/render_messages.h"
+#include "components/favicon/core/fallback_icon_service.h"
+#include "components/favicon/core/favicon_service.h"
 #include "components/history/core/browser/top_sites.h"
+#include "components/keyed_service/core/service_access_type.h"
 #include "components/search_engines/template_url_service.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/url_data_source.h"
 #include "grit/theme_resources.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "ui/gfx/color_utils.h"
 #include "ui/gfx/image/image_skia.h"
@@ skipping 80 matching lines @@
 
   // TODO(aurimas) remove this #if once instant_service.cc is no longer compiled
   // on Android.
 #if !defined(OS_ANDROID)
   content::URLDataSource::Add(profile_, new LocalNtpSource(profile_));
   content::URLDataSource::Add(profile_, new ThumbnailSource(profile_, false));
   content::URLDataSource::Add(profile_, new ThumbnailSource(profile_, true));
   content::URLDataSource::Add(profile_, new ThumbnailListSource(profile_));
 #endif  // !defined(OS_ANDROID)
 
-  content::URLDataSource::Add(profile_, new LargeIconSource(profile));
-  content::URLDataSource::Add(profile_, new FallbackIconSource());
+  FaviconService* favicon_service = FaviconServiceFactory::GetForProfile(
+      profile_, ServiceAccessType::EXPLICIT_ACCESS);
+  FallbackIconService* fallback_icon_service =
+      FallbackIconServiceFactory::GetForProfile(
+          profile_, ServiceAccessType::EXPLICIT_ACCESS);
+
+  if (favicon_service && fallback_icon_service) {
+    content::URLDataSource::Add(profile_,
+        new LargeIconSource(favicon_service, fallback_icon_service));
+  }
+  if (fallback_icon_service) {
+    content::URLDataSource::Add(profile_,
+                                new FallbackIconSource(fallback_icon_service));

[beaudoin, 2015/03/26 12:53:55]

I'm not super aware of the lifecycle of these different objects, but I just want
to point out that |favicon_service| and|fallback_icon_service| have to outlive
the LargeIconSource and FallbackIconSource. If you're confident they do I'm
fine.

[huangs, 2015/03/26 18:26:06]

I added these because favicon_service === nullptr for a unit test, and this
triggers assert I added.  Come to think of it, doing defensive programming here
is likely bad.  I should be defensive within the services (observed a similar
pattern in previous codes).

+  }
   content::URLDataSource::Add(
       profile_, new FaviconSource(profile_, FaviconSource::FAVICON));
   content::URLDataSource::Add(profile_, new MostVisitedIframeSource());
   content::URLDataSource::Add(
       profile_, new suggestions::SuggestionsSource(profile_));
 }
 
 InstantService::~InstantService() {
   if (template_url_service_)
     template_url_service_->RemoveObserver(this);
@@ skipping 298 matching lines @@
 }
 
 void InstantService::ResetInstantSearchPrerenderer() {
   if (!chrome::ShouldPrefetchSearchResults())
     return;
 
   GURL url(chrome::GetSearchResultPrefetchBaseURL(profile_));
   instant_prerenderer_.reset(
       url.is_valid() ? new InstantSearchPrerenderer(profile_, url) : NULL);
 }