Skip to content

Commit

Permalink
Revert of Revert of Move //components/favicon code into the favicon n…
Browse files Browse the repository at this point in the history
…amespace (patchset chromium#1 id:1 of https://codereview.chromium.org/1045823004/)

Reason for revert:
Not a culprit. Bring it back.

The layout tests continued to fail after reverting:   https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.8%20(retina)/builds/27707

Original issue's description:
> Revert of Move //components/favicon code into the favicon namespace (patchset chromium#4 id:80001 of https://codereview.chromium.org/1028503004/)
>
> Reason for revert:
> Blink gardening. Speculative revert for Blink layout test failures in WebKit Mac10.8 (retina):
> https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Mac10.8%20%28retina%29/builds/27696
> chromium range:  chromium revision 322799:322813
> https://chromium.googlesource.com/chromium/src/+log/494aec68691f10626b226892396d442503dd9af7%5E..e240ab4e9c5841de8cb65226a80008600416c365?pretty=fuller
>
> Original issue's description:
> > Move //components/favicon code into the favicon namespace
> >
> > Move all classes defined in //components/favicon into the favicon namespace
> > and update all client code forward-declaration and usage.
> >
> > Rename FaviconTabHelperObserver to FaviconDriverObserver as FaviconDriver is
> > the name of the base interface (and iOS wants to use that observer but will
> > not be able to use FaviconTabHelper).
> >
> > BUG=359513
> >
> > Committed: https://crrev.com/494aec68691f10626b226892396d442503dd9af7
> > Cr-Commit-Position: refs/heads/master@{#322799}
>
> TBR=sky@chromium.org,sdefresne@chromium.org
> NOPRESUBMIT=true
> NOTREECHECKS=true
> NOTRY=true
> BUG=359513
>
> Committed: https://crrev.com/c7c707f533da81e7c7d5c2201426ae60e4a0daf7
> Cr-Commit-Position: refs/heads/master@{#322969}

TBR=sky@chromium.org,sdefresne@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=359513

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

Cr-Commit-Position: refs/heads/master@{#322980}
  • Loading branch information
hayatoito authored and Commit bot committed Mar 31, 2015
1 parent 85dcb72 commit be13e67
Show file tree
Hide file tree
Showing 57 changed files with 348 additions and 261 deletions.
7 changes: 4 additions & 3 deletions chrome/browser/android/bookmarks/partner_bookmarks_reader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,10 @@ void SetFaviconTask(Profile* profile,
scoped_refptr<base::RefCountedMemory> bitmap_data(
new base::RefCountedBytes(image_data));
gfx::Size pixel_size(gfx::kFaviconSize, gfx::kFaviconSize);
FaviconService* favicon_service = FaviconServiceFactory::GetForProfile(
ProfileManager::GetActiveUserProfile(),
ServiceAccessType::EXPLICIT_ACCESS);
favicon::FaviconService* favicon_service =
FaviconServiceFactory::GetForProfile(
ProfileManager::GetActiveUserProfile(),
ServiceAccessType::EXPLICIT_ACCESS);
if (!favicon_service)
return;

Expand Down
10 changes: 6 additions & 4 deletions chrome/browser/android/favicon_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,9 @@ jboolean FaviconHelper::GetLocalFaviconImageForURL(
if (!profile)
return false;

FaviconService* favicon_service = FaviconServiceFactory::GetForProfile(
profile, ServiceAccessType::EXPLICIT_ACCESS);
favicon::FaviconService* favicon_service =
FaviconServiceFactory::GetForProfile(profile,
ServiceAccessType::EXPLICIT_ACCESS);
DCHECK(favicon_service);
if (!favicon_service)
return false;
Expand Down Expand Up @@ -151,8 +152,9 @@ void FaviconHelper::GetLargestRawFaviconForUrl(
if (!profile)
return;

FaviconService* favicon_service = FaviconServiceFactory::GetForProfile(
profile, ServiceAccessType::EXPLICIT_ACCESS);
favicon::FaviconService* favicon_service =
FaviconServiceFactory::GetForProfile(profile,
ServiceAccessType::EXPLICIT_ACCESS);
DCHECK(favicon_service);
if (!favicon_service)
return;
Expand Down
26 changes: 12 additions & 14 deletions chrome/browser/android/provider/chrome_browser_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -651,13 +651,13 @@ class AsyncServiceRequest : protected BlockingUIThreadAsyncRequest {
};

// Base class for all asynchronous blocking tasks that use the favicon service.
class FaviconServiceTask : public AsyncServiceRequest<FaviconService> {
class FaviconServiceTask : public AsyncServiceRequest<favicon::FaviconService> {
public:
FaviconServiceTask(base::CancelableTaskTracker* cancelable_tracker,
Profile* profile,
FaviconService* favicon_service)
: AsyncServiceRequest<FaviconService>(favicon_service,
cancelable_tracker),
favicon::FaviconService* favicon_service)
: AsyncServiceRequest<favicon::FaviconService>(favicon_service,
cancelable_tracker),
profile_(profile) {}

Profile* profile() const { return profile_; }
Expand All @@ -673,7 +673,7 @@ class BookmarkIconFetchTask : public FaviconServiceTask {
public:
BookmarkIconFetchTask(base::CancelableTaskTracker* cancelable_tracker,
Profile* profile,
FaviconService* favicon_service)
favicon::FaviconService* favicon_service)
: FaviconServiceTask(cancelable_tracker, profile, favicon_service) {}

favicon_base::FaviconRawBitmapResult Run(const GURL& url) {
Expand All @@ -684,15 +684,13 @@ class BookmarkIconFetchTask : public FaviconServiceTask {
if (service() == NULL)
return favicon_base::FaviconRawBitmapResult();

RunAsyncRequestOnUIThreadBlocking(
base::Bind(&FaviconService::GetRawFaviconForPageURL,
base::Unretained(service()),
url,
favicon_base::FAVICON | favicon_base::TOUCH_ICON,
desired_size_in_pixel,
base::Bind(&BookmarkIconFetchTask::OnFaviconRetrieved,
base::Unretained(this)),
cancelable_tracker()));
RunAsyncRequestOnUIThreadBlocking(base::Bind(
&favicon::FaviconService::GetRawFaviconForPageURL,
base::Unretained(service()), url,
favicon_base::FAVICON | favicon_base::TOUCH_ICON, desired_size_in_pixel,
base::Bind(&BookmarkIconFetchTask::OnFaviconRetrieved,
base::Unretained(this)),
cancelable_tracker()));
return result_;
}

Expand Down
7 changes: 5 additions & 2 deletions chrome/browser/android/provider/chrome_browser_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,12 @@
#include "components/history/core/browser/history_service_observer.h"

class AndroidHistoryProviderService;
class FaviconService;
class Profile;

namespace favicon {
class FaviconService;
}

namespace history {
class TopSites;
}
Expand Down Expand Up @@ -211,7 +214,7 @@ class ChromeBrowserProvider : public bookmarks::BaseBookmarkModelObserver,
Profile* profile_;
bookmarks::BookmarkModel* bookmark_model_;
scoped_refptr<history::TopSites> top_sites_;
FaviconService* favicon_service_;
favicon::FaviconService* favicon_service_;

scoped_ptr<AndroidHistoryProviderService> service_;

Expand Down
5 changes: 3 additions & 2 deletions chrome/browser/android/shortcut_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,9 @@ void ShortcutHelper::AddShortcutUsingFavicon() {
icon_types.push_back(favicon_base::FAVICON);
icon_types.push_back(favicon_base::TOUCH_PRECOMPOSED_ICON |
favicon_base::TOUCH_ICON);
FaviconService* favicon_service = FaviconServiceFactory::GetForProfile(
profile, ServiceAccessType::EXPLICIT_ACCESS);
favicon::FaviconService* favicon_service =
FaviconServiceFactory::GetForProfile(profile,
ServiceAccessType::EXPLICIT_ACCESS);

// Using favicon if its size is not smaller than platform required size,
// otherwise using the largest icon among all avaliable icons.
Expand Down
6 changes: 3 additions & 3 deletions chrome/browser/android/tab_android.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
#include "chrome/browser/ui/search/search_tab_helper_delegate.h"
#include "chrome/browser/ui/tab_contents/core_tab_helper_delegate.h"
#include "chrome/browser/ui/toolbar/toolbar_model.h"
#include "components/favicon/core/favicon_tab_helper_observer.h"
#include "components/favicon/core/favicon_driver_observer.h"
#include "components/sessions/session_id.h"
#include "content/public/browser/notification_observer.h"
#include "content/public/browser/notification_registrar.h"
Expand Down Expand Up @@ -53,7 +53,7 @@ class TabAndroid : public CoreTabHelperDelegate,
public InstantServiceObserver,
public SearchTabHelperDelegate,
public content::NotificationObserver,
public FaviconTabHelperObserver {
public favicon::FaviconDriverObserver {
public:
// A Java counterpart will be generated for this enum.
// GENERATED_JAVA_ENUM_PACKAGE: org.chromium.chrome.browser
Expand Down Expand Up @@ -137,7 +137,7 @@ class TabAndroid : public CoreTabHelperDelegate,
const content::NotificationSource& source,
const content::NotificationDetails& details) override;

// FaviconTabHelperObserver -----------------------------------------------
// favicon::FaviconDriverObserver -------------------------------------------
void OnFaviconAvailable(const gfx::Image& image) override;

// Methods called from Java via JNI -----------------------------------------
Expand Down
5 changes: 3 additions & 2 deletions chrome/browser/bookmarks/bookmark_html_writer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -458,8 +458,9 @@ bool BookmarkFaviconFetcher::FetchNextFavicon() {
// Filter out urls that we've already got favicon for.
URLFaviconMap::const_iterator iter = favicons_map_->find(url);
if (favicons_map_->end() == iter) {
FaviconService* favicon_service = FaviconServiceFactory::GetForProfile(
profile_, ServiceAccessType::EXPLICIT_ACCESS);
favicon::FaviconService* favicon_service =
FaviconServiceFactory::GetForProfile(
profile_, ServiceAccessType::EXPLICIT_ACCESS);
favicon_service->GetRawFaviconForPageURL(
GURL(url),
favicon_base::FAVICON,
Expand Down
5 changes: 3 additions & 2 deletions chrome/browser/bookmarks/chrome_bookmark_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,9 @@ ChromeBookmarkClient::GetFaviconImageForPageURL(
favicon_base::IconType type,
const favicon_base::FaviconImageCallback& callback,
base::CancelableTaskTracker* tracker) {
FaviconService* favicon_service = FaviconServiceFactory::GetForProfile(
profile_, ServiceAccessType::EXPLICIT_ACCESS);
favicon::FaviconService* favicon_service =
FaviconServiceFactory::GetForProfile(profile_,
ServiceAccessType::EXPLICIT_ACCESS);
if (!favicon_service)
return base::CancelableTaskTracker::kBadTaskId;
if (type == favicon_base::FAVICON) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,8 +249,9 @@ ChromeManagementAPIDelegate::GenerateAppForLinkFunctionDelegate(
content::BrowserContext* context,
const std::string& title,
const GURL& launch_url) const {
FaviconService* favicon_service = FaviconServiceFactory::GetForProfile(
Profile::FromBrowserContext(context), ServiceAccessType::EXPLICIT_ACCESS);
favicon::FaviconService* favicon_service =
FaviconServiceFactory::GetForProfile(Profile::FromBrowserContext(context),
ServiceAccessType::EXPLICIT_ACCESS);
DCHECK(favicon_service);

ChromeAppForLinkDelegate* delegate = new ChromeAppForLinkDelegate;
Expand Down
5 changes: 2 additions & 3 deletions chrome/browser/extensions/extension_web_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,8 @@ void RunFaviconCallbackAsync(

base::MessageLoopProxy::current()->PostTask(
FROM_HERE,
base::Bind(&FaviconService::FaviconResultsCallbackRunner,
callback,
base::Owned(favicon_bitmap_results)));
base::Bind(&favicon::FaviconService::FaviconResultsCallbackRunner,
callback, base::Owned(favicon_bitmap_results)));
}

bool ValidateOverrideURL(const base::Value* override_url_value,
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/favicon/chrome_favicon_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "chrome/browser/favicon/chrome_favicon_client.h"

#include "base/memory/singleton.h"
#include "chrome/browser/ui/webui/chrome_web_ui_controller_factory.h"
#include "chrome/common/url_constants.h"
#include "components/bookmarks/browser/bookmark_model.h"
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/favicon/chrome_favicon_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@ class BookmarkModel;
}

// ChromeFaviconClient implements the the FaviconClient interface.
class ChromeFaviconClient : public FaviconClient {
class ChromeFaviconClient : public favicon::FaviconClient {
public:
ChromeFaviconClient(Profile* profile,
bookmarks::BookmarkModel* bookmark_model);
~ChromeFaviconClient() override;

private:
// FaviconClient implementation:
// favicon::FaviconClient implementation:
bool IsBookmarked(const GURL& url) override;
bool IsNativeApplicationURL(const GURL& url) override;
base::CancelableTaskTracker::TaskId GetFaviconForNativeApplicationURL(
Expand Down
7 changes: 5 additions & 2 deletions chrome/browser/favicon/chrome_favicon_client_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@

#include "chrome/browser/favicon/chrome_favicon_client_factory.h"

#include "base/memory/singleton.h"
#include "chrome/browser/bookmarks/bookmark_model_factory.h"
#include "chrome/browser/favicon/chrome_favicon_client.h"
#include "chrome/browser/profiles/incognito_helpers.h"
#include "chrome/browser/profiles/profile.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h"
Expand All @@ -20,8 +22,9 @@ ChromeFaviconClientFactory::~ChromeFaviconClientFactory() {
}

// static
FaviconClient* ChromeFaviconClientFactory::GetForProfile(Profile* profile) {
return static_cast<FaviconClient*>(
favicon::FaviconClient* ChromeFaviconClientFactory::GetForProfile(
Profile* profile) {
return static_cast<favicon::FaviconClient*>(
GetInstance()->GetServiceForBrowserContext(profile, true));
}

Expand Down
13 changes: 9 additions & 4 deletions chrome/browser/favicon/chrome_favicon_client_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,24 @@
#ifndef CHROME_BROWSER_FAVICON_CHROME_FAVICON_CLIENT_FACTORY_H_
#define CHROME_BROWSER_FAVICON_CHROME_FAVICON_CLIENT_FACTORY_H_

#include "base/memory/singleton.h"
#include "chrome/browser/favicon/chrome_favicon_client.h"
#include "components/keyed_service/content/browser_context_keyed_service_factory.h"

template <typename T>
struct DefaultSingletonTraits;

class Profile;

namespace favicon {
class FaviconClient;
}

// Singleton that owns all ChromeFaviconClients and associates them with
// Profiles.
class ChromeFaviconClientFactory : public BrowserContextKeyedServiceFactory {
public:
// Returns the instance of FaviconClient associated with this profile
// Returns the instance of favicon::FaviconClient associated with |profile|
// (creating one if none exists).
static FaviconClient* GetForProfile(Profile* profile);
static favicon::FaviconClient* GetForProfile(Profile* profile);

// Returns an instance of the factory singleton.
static ChromeFaviconClientFactory* GetInstance();
Expand Down
Loading

0 comments on commit be13e67

Please sign in to comment.