Skip to content

Commit

Permalink
[Fallback Icons] Refactor FallbackIconService to be a BrowserContext-…
Browse files Browse the repository at this point in the history
…level singleton

- FallbackIconServiceFactory now get sFallbackIconService singleton from
  BrowserContext.
- FallbackIconClient & ChromeFallbackIconClient: wrap external code so the Favicon
  component won't have to include them. Specifically:
  - Choosing fonts: also removed duplicate code.
  - Extracting letter from URL to render Fallback.

BUG=455063

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

Cr-Commit-Position: refs/heads/master@{#322653}
  • Loading branch information
samuelhuang authored and Commit bot committed Mar 27, 2015
1 parent 328a417 commit f16444b
Show file tree
Hide file tree
Showing 23 changed files with 408 additions and 81 deletions.
1 change: 1 addition & 0 deletions chrome/browser/favicon/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ include_rules = [
"+chrome/browser/profiles/incognito_helpers.h",
"+chrome/browser/search/search.h",
"+components/favicon/core",
"+net/base/registry_controlled_domains/registry_controlled_domain.h",
"+third_party/skia/include/core/SkBitmap.h",

# TODO(caitkp): Bring this list to zero.
Expand Down
41 changes: 41 additions & 0 deletions chrome/browser/favicon/chrome_fallback_icon_client.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright 2015 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/favicon/chrome_fallback_icon_client.h"

#include "base/i18n/case_conversion.h"
#include "base/strings/utf_string_conversions.h"
#include "grit/platform_locale_settings.h"
#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
#include "ui/base/l10n/l10n_util.h"
#include "url/gurl.h"

ChromeFallbackIconClient::ChromeFallbackIconClient() {
#if defined(OS_CHROMEOS)
font_list_.push_back("Noto Sans");
#elif defined(OS_IOS)
font_list_.push_back("Helvetica Neue");
#else
font_list_.push_back(l10n_util::GetStringUTF8(IDS_SANS_SERIF_FONT_FAMILY));
#endif
}

ChromeFallbackIconClient::~ChromeFallbackIconClient() {
}

const std::vector<std::string>& ChromeFallbackIconClient::GetFontNameList()
const {
return font_list_;
}

// Returns a single character to represent |url|. To do this we take the first
// letter in a domain's name and make it upper case.
base::string16 ChromeFallbackIconClient::GetFallbackIconText(const GURL& url)
const {
// TODO(huangs): Handle non-ASCII ("xn--") domain names.
std::string domain = net::registry_controlled_domains::GetDomainAndRegistry(
url, net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES);
return domain.empty() ? base::string16() :
base::i18n::ToUpper(base::ASCIIToUTF16(domain.substr(0, 1)));
}
34 changes: 34 additions & 0 deletions chrome/browser/favicon/chrome_fallback_icon_client.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// Copyright 2015 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_FAVICON_CHROME_FALLBACK_ICON_CLIENT_H_
#define CHROME_BROWSER_FAVICON_CHROME_FALLBACK_ICON_CLIENT_H_

#include <string>
#include <vector>

#include "base/macros.h"
#include "base/strings/string16.h"
#include "components/favicon/core/fallback_icon_client.h"

class GURL;

// ChromeFallbackIconClient implements the FallbackIconClient interface.
class ChromeFallbackIconClient : public FallbackIconClient {
public:
ChromeFallbackIconClient();
~ChromeFallbackIconClient() override;

// FallbackIconClient implementation:
const std::vector<std::string>& GetFontNameList() const override;

base::string16 GetFallbackIconText(const GURL& url) const override;

private:
std::vector<std::string> font_list_;

DISALLOW_COPY_AND_ASSIGN(ChromeFallbackIconClient);
};

#endif // CHROME_BROWSER_FAVICON_CHROME_FALLBACK_ICON_CLIENT_H_
38 changes: 38 additions & 0 deletions chrome/browser/favicon/chrome_fallback_icon_client_factory.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Copyright 2015 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/favicon/chrome_fallback_icon_client_factory.h"

#include "base/memory/singleton.h"
#include "chrome/browser/favicon/chrome_fallback_icon_client.h"
#include "chrome/browser/profiles/incognito_helpers.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
FallbackIconClient* ChromeFallbackIconClientFactory::GetForBrowserContext(
content::BrowserContext* context) {
return static_cast<FallbackIconClient*>(
GetInstance()->GetServiceForBrowserContext(context, true));
}

// static
ChromeFallbackIconClientFactory*
ChromeFallbackIconClientFactory::GetInstance() {
return Singleton<ChromeFallbackIconClientFactory>::get();
}

KeyedService* ChromeFallbackIconClientFactory::BuildServiceInstanceFor(
content::BrowserContext* context) const {
return new ChromeFallbackIconClient();
}
42 changes: 42 additions & 0 deletions chrome/browser/favicon/chrome_fallback_icon_client_factory.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Copyright 2015 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_FAVICON_CHROME_FALLBACK_ICON_CLIENT_FACTORY_H_
#define CHROME_BROWSER_FAVICON_CHROME_FALLBACK_ICON_CLIENT_FACTORY_H_

#include "components/keyed_service/content/browser_context_keyed_service_factory.h"

template <typename T> struct DefaultSingletonTraits;

class FallbackIconClient;

namespace content {
class BrowserContext;
}

// Singleton that owns all ChromeFallbackIconClients and associates them with
// Profiles.
class ChromeFallbackIconClientFactory
: public BrowserContextKeyedServiceFactory {
public:
// Returns the instance of FallbackIconClient associated with this profile
// (creating one if none exists).
static FallbackIconClient* GetForBrowserContext(
content::BrowserContext* context);

// Returns an instance of the factory singleton.
static ChromeFallbackIconClientFactory* GetInstance();

private:
friend struct DefaultSingletonTraits<ChromeFallbackIconClientFactory>;

ChromeFallbackIconClientFactory();
~ChromeFallbackIconClientFactory() override;

// BrowserContextKeyedServiceFactory:
KeyedService* BuildServiceInstanceFor(
content::BrowserContext* context) const override;
};

#endif // CHROME_BROWSER_FAVICON_CHROME_FALLBACK_ICON_CLIENT_FACTORY_H_
43 changes: 43 additions & 0 deletions chrome/browser/favicon/fallback_icon_service_factory.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Copyright 2015 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/favicon/fallback_icon_service_factory.h"

#include "base/memory/singleton.h"
#include "chrome/browser/favicon/chrome_fallback_icon_client_factory.h"
#include "components/favicon/core/fallback_icon_service.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h"
#include "content/public/browser/browser_context.h"

// static
FallbackIconService* FallbackIconServiceFactory::GetForBrowserContext(
content::BrowserContext* context) {
return static_cast<FallbackIconService*>(
GetInstance()->GetServiceForBrowserContext(context, true));
}

// static
FallbackIconServiceFactory* FallbackIconServiceFactory::GetInstance() {
return Singleton<FallbackIconServiceFactory>::get();
}

FallbackIconServiceFactory::FallbackIconServiceFactory()
: BrowserContextKeyedServiceFactory(
"FallbackIconService",
BrowserContextDependencyManager::GetInstance()) {
DependsOn(ChromeFallbackIconClientFactory::GetInstance());
}

FallbackIconServiceFactory::~FallbackIconServiceFactory() {}

KeyedService* FallbackIconServiceFactory::BuildServiceInstanceFor(
content::BrowserContext* context) const {
FallbackIconClient* fallback_icon_client =
ChromeFallbackIconClientFactory::GetForBrowserContext(context);
return new FallbackIconService(fallback_icon_client);
}

bool FallbackIconServiceFactory::ServiceIsNULLWhileTesting() const {
return true;
}
41 changes: 41 additions & 0 deletions chrome/browser/favicon/fallback_icon_service_factory.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright 2015 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_FAVICON_FALLBACK_ICON_SERVICE_FACTORY_H_
#define CHROME_BROWSER_FAVICON_FALLBACK_ICON_SERVICE_FACTORY_H_

#include "components/keyed_service/content/browser_context_keyed_service_factory.h"

template <typename T> struct DefaultSingletonTraits;

class FallbackIconService;

namespace content {
class BrowserContext;
}

// Singleton that owns all FallbackIconService and associates them with
// BrowserContext instances.
class FallbackIconServiceFactory : public BrowserContextKeyedServiceFactory {
public:
static FallbackIconService* GetForBrowserContext(
content::BrowserContext* context);

static FallbackIconServiceFactory* GetInstance();

private:
friend struct DefaultSingletonTraits<FallbackIconServiceFactory>;

FallbackIconServiceFactory();
~FallbackIconServiceFactory() override;

// BrowserContextKeyedServiceFactory:
KeyedService* BuildServiceInstanceFor(
content::BrowserContext* context) const override;
bool ServiceIsNULLWhileTesting() const override;

DISALLOW_COPY_AND_ASSIGN(FallbackIconServiceFactory);
};

#endif // CHROME_BROWSER_FAVICON_FALLBACK_ICON_SERVICE_FACTORY_H_
2 changes: 1 addition & 1 deletion chrome/browser/favicon/favicon_service_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class Profile;
// Profiles.
class FaviconServiceFactory : public BrowserContextKeyedServiceFactory {
public:
// |access| defines what the caller plans to do with the service. See
// |sat| defines what the caller plans to do with the service. See
// the ServiceAccessType definition in profile.h.
static FaviconService* GetForProfile(Profile* profile, ServiceAccessType sat);

Expand Down
16 changes: 14 additions & 2 deletions chrome/browser/search/instant_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#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"
Expand All @@ -25,7 +27,10 @@
#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"
Expand Down Expand Up @@ -126,8 +131,15 @@ InstantService::InstantService(Profile* profile)
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::GetForBrowserContext(profile_);

content::URLDataSource::Add(profile_,
new LargeIconSource(favicon_service, fallback_icon_service));
content::URLDataSource::Add(
profile_, new FallbackIconSource(fallback_icon_service));
content::URLDataSource::Add(
profile_, new FaviconSource(profile_, FaviconSource::FAVICON));
content::URLDataSource::Add(profile_, new MostVisitedIframeSource());
Expand Down
43 changes: 24 additions & 19 deletions chrome/browser/ui/webui/fallback_icon_source.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,29 +4,21 @@

#include "chrome/browser/ui/webui/fallback_icon_source.h"

#include <string>
#include <vector>

#include "base/memory/ref_counted_memory.h"
#include "chrome/browser/search/instant_io_context.h"
#include "chrome/common/favicon/fallback_icon_url_parser.h"
#include "chrome/common/url_constants.h"
#include "grit/platform_locale_settings.h"
#include "components/favicon/core/fallback_icon_service.h"
#include "components/keyed_service/core/service_access_type.h"
#include "net/url_request/url_request.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/gfx/favicon_size.h"
#include "url/gurl.h"

FallbackIconSource::FallbackIconSource() {
std::vector<std::string> font_list;
#if defined(OS_CHROMEOS)
font_list.push_back("Noto Sans");
#elif defined(OS_IOS)
font_list.push_back("Helvetica Neue");
#else
font_list.push_back(l10n_util::GetStringUTF8(IDS_SANS_SERIF_FONT_FAMILY));
#endif
fallback_icon_service_.reset(new FallbackIconService(font_list));
FallbackIconSource::FallbackIconSource(
FallbackIconService* fallback_icon_service)
: fallback_icon_service_(fallback_icon_service) {
}

FallbackIconSource::~FallbackIconSource() {
Expand All @@ -47,15 +39,15 @@ void FallbackIconSource::StartDataRequest(
SendDefaultResponse(callback);
return;
}

GURL url(parsed.url_string());
if (url.is_empty() || !url.is_valid()) {
SendDefaultResponse(callback);
return;
}
std::vector<unsigned char> bitmap_data =
fallback_icon_service_->RenderFallbackIconBitmap(
url, parsed.size_in_pixels(), parsed.style());
callback.Run(base::RefCountedBytes::TakeVector(&bitmap_data));

SendFallbackIconHelper(
url, parsed.size_in_pixels(), parsed.style(), callback);
}

std::string FallbackIconSource::GetMimeType(const std::string&) const {
Expand All @@ -77,10 +69,23 @@ bool FallbackIconSource::ShouldServiceRequest(
return URLDataSource::ShouldServiceRequest(request);
}

void FallbackIconSource::SendDefaultResponse(
void FallbackIconSource::SendFallbackIconHelper(
const GURL& url,
int size_in_pixels,
const favicon_base::FallbackIconStyle& style,
const content::URLDataSource::GotDataCallback& callback) {
if (!fallback_icon_service_) { // Can be null for tests.
callback.Run(nullptr); // Trigger "Not Found" response.
return;
}
std::vector<unsigned char> bitmap_data =
fallback_icon_service_->RenderFallbackIconBitmap(
GURL(), gfx::kFaviconSize, favicon_base::FallbackIconStyle());
url, size_in_pixels, style);
callback.Run(base::RefCountedBytes::TakeVector(&bitmap_data));
}

void FallbackIconSource::SendDefaultResponse(
const content::URLDataSource::GotDataCallback& callback) {
favicon_base::FallbackIconStyle default_style;
SendFallbackIconHelper(GURL(), gfx::kFaviconSize, default_style, callback);
}
Loading

0 comments on commit f16444b

Please sign in to comment.