diff options
| -rw-r--r-- | chrome/browser/extensions/favicon_downloader.cc | 124 | ||||
| -rw-r--r-- | chrome/browser/extensions/favicon_downloader.h | 101 | ||||
| -rw-r--r-- | chrome/browser/extensions/favicon_downloader_unittest.cc | 197 | ||||
| -rw-r--r-- | chrome/browser/extensions/tab_helper.cc | 102 | ||||
| -rw-r--r-- | chrome/browser/extensions/tab_helper.h | 12 | ||||
| -rw-r--r-- | chrome/browser/favicon/favicon_tab_helper.cc | 4 | ||||
| -rw-r--r-- | chrome/browser/favicon/favicon_tab_helper.h | 8 | ||||
| -rw-r--r-- | chrome/chrome_browser_extensions.gypi | 2 | ||||
| -rw-r--r-- | chrome/chrome_tests_unit.gypi | 1 | ||||
| -rw-r--r-- | content/renderer/render_view_browsertest.cc | 22 |
10 files changed, 537 insertions, 36 deletions
diff --git a/chrome/browser/extensions/favicon_downloader.cc b/chrome/browser/extensions/favicon_downloader.cc new file mode 100644 index 0000000..b518a32 --- /dev/null +++ b/chrome/browser/extensions/favicon_downloader.cc @@ -0,0 +1,124 @@ +// 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/extensions/favicon_downloader.h" + +#include "base/bind.h" +#include "chrome/browser/favicon/favicon_tab_helper.h" +#include "content/public/browser/web_contents.h" +#include "content/public/common/favicon_url.h" +#include "third_party/skia/include/core/SkBitmap.h" +#include "ui/gfx/size.h" + +FaviconDownloader::FaviconDownloader( + content::WebContents* web_contents, + const std::vector<GURL>& extra_favicon_urls, + FaviconDownloaderCallback callback) + : content::WebContentsObserver(web_contents), + got_favicon_urls_(false), + extra_favicon_urls_(extra_favicon_urls), + callback_(callback), + weak_ptr_factory_(this) { +} + +FaviconDownloader::~FaviconDownloader() { +} + +void FaviconDownloader::Start() { + FetchIcons(extra_favicon_urls_); + // If the candidates aren't loaded, icons will be fetched when + // DidUpdateFaviconURL() is called. + std::vector<content::FaviconURL> favicon_tab_helper_urls = + GetFaviconURLsFromWebContents(); + if (!favicon_tab_helper_urls.empty()) { + got_favicon_urls_ = true; + FetchIcons(favicon_tab_helper_urls); + } +} + +int FaviconDownloader::DownloadImage(const GURL& url) { + return web_contents()->DownloadImage( + url, + true, // is_favicon + 0, // no max size + base::Bind(&FaviconDownloader::DidDownloadFavicon, + weak_ptr_factory_.GetWeakPtr())); +} + +std::vector<content::FaviconURL> + FaviconDownloader::GetFaviconURLsFromWebContents() { + FaviconTabHelper* favicon_tab_helper = + web_contents() ? FaviconTabHelper::FromWebContents(web_contents()) : NULL; + // If favicon_urls() is empty, we are guaranteed that DidUpdateFaviconURLs has + // not yet been called for the current page's navigation. + return favicon_tab_helper ? favicon_tab_helper->favicon_urls() + : std::vector<content::FaviconURL>(); +} + +void FaviconDownloader::FetchIcons( + const std::vector<content::FaviconURL>& favicon_urls) { + std::vector<GURL> urls; + for (std::vector<content::FaviconURL>::const_iterator it = + favicon_urls.begin(); + it != favicon_urls.end(); ++it) { + if (it->icon_type != content::FaviconURL::INVALID_ICON) + urls.push_back(it->icon_url); + } + FetchIcons(urls); +} + +void FaviconDownloader::FetchIcons(const std::vector<GURL>& urls) { + // Download icons; put their download ids into |in_progress_requests_| and + // their urls into |processed_urls_|. + for (std::vector<GURL>::const_iterator it = urls.begin(); + it != urls.end(); ++it) { + // Only start the download if the url hasn't been processed before. + if (processed_urls_.insert(*it).second) + in_progress_requests_.insert(DownloadImage(*it)); + } + + // If no downloads were initiated, we can proceed directly to running the + // callback. + if (in_progress_requests_.empty() && got_favicon_urls_) + callback_.Run(true, favicon_map_); +} + +void FaviconDownloader::DidDownloadFavicon( + int id, + int http_status_code, + const GURL& image_url, + const std::vector<SkBitmap>& bitmaps, + const std::vector<gfx::Size>& original_bitmap_sizes) { + // Request may have been canceled by DidNavigateMainFrame(). + if (in_progress_requests_.erase(id) == 0) + return; + + favicon_map_[image_url] = bitmaps; + + // Once all requests have been resolved, perform post-download tasks. + if (in_progress_requests_.empty() && got_favicon_urls_) + callback_.Run(true, favicon_map_); +} + +// content::WebContentsObserver overrides: +void FaviconDownloader::DidNavigateMainFrame( + const content::LoadCommittedDetails& details, + const content::FrameNavigateParams& params) { + // Clear all pending requests. + in_progress_requests_.clear(); + favicon_map_.clear(); + callback_.Run(false, favicon_map_); +} + +void FaviconDownloader::DidUpdateFaviconURL( + int32 page_id, + const std::vector<content::FaviconURL>& candidates) { + // Only consider the first candidates we are given. This prevents pages that + // change their favicon from spamming us. + if (got_favicon_urls_) + return; + + got_favicon_urls_ = true; + FetchIcons(candidates); +} diff --git a/chrome/browser/extensions/favicon_downloader.h b/chrome/browser/extensions/favicon_downloader.h new file mode 100644 index 0000000..e42ee9b --- /dev/null +++ b/chrome/browser/extensions/favicon_downloader.h @@ -0,0 +1,101 @@ +// 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. + +#ifndef CHROME_BROWSER_EXTENSIONS_FAVICON_DOWNLOADER_H_ +#define CHROME_BROWSER_EXTENSIONS_FAVICON_DOWNLOADER_H_ + +#include <map> +#include <set> +#include <vector> + +#include "base/callback.h" +#include "base/memory/weak_ptr.h" +#include "content/public/browser/web_contents_observer.h" + +class SkBitmap; + +namespace content { +struct FaviconURL; +} + +namespace gfx { +class Size; +} + +// Class to help download all favicons for a tab. +class FaviconDownloader : public content::WebContentsObserver { + public: + typedef std::map<GURL, std::vector<SkBitmap> > FaviconMap; + typedef base::Callback<void( + bool, /* success */ + /* A map of icon urls to the bitmaps provided by that url. */ + const FaviconMap&)> + FaviconDownloaderCallback; + // |extra_favicon_urls| allows callers to provide icon urls that aren't + // |provided by the renderer (e.g touch icons on non-android environments). + FaviconDownloader(content::WebContents* web_contents, + const std::vector<GURL>& extra_favicon_urls, + FaviconDownloaderCallback callback); + virtual ~FaviconDownloader(); + + void Start(); + + private: + friend class TestFaviconDownloader; + + // Initiates a download of the image at |url| and returns the download id. + // This is overridden in testing. + virtual int DownloadImage(const GURL& url); + + // Queries FaviconTabHelper for the page's current favicon URLs. + // This is overridden in testing. + virtual std::vector<content::FaviconURL> GetFaviconURLsFromWebContents(); + + // Fetches icons for the given urls. + // |callback_| is run when all downloads complete. + void FetchIcons(const std::vector<content::FaviconURL>& favicon_urls); + void FetchIcons(const std::vector<GURL>& urls); + + // Icon download callback. + void DidDownloadFavicon(int id, + int http_status_code, + const GURL& image_url, + const std::vector<SkBitmap>& bitmaps, + const std::vector<gfx::Size>& original_bitmap_sizes); + + // content::WebContentsObserver overrides: + virtual void DidNavigateMainFrame( + const content::LoadCommittedDetails& details, + const content::FrameNavigateParams& params) OVERRIDE; + virtual void DidUpdateFaviconURL( + int32 page_id, + const std::vector<content::FaviconURL>& candidates) OVERRIDE; + + // Whether we have received favicons from the renderer. + bool got_favicon_urls_; + + // URLs that aren't given by WebContentsObserver::DidUpdateFaviconURL() that + // should be used for this favicon. This is necessary in order to get touch + // icons on non-android environments. + std::vector<GURL> extra_favicon_urls_; + + // The icons which were downloaded. Populated by FetchIcons(). + FaviconMap favicon_map_; + + // Request ids of in-progress requests. + std::set<int> in_progress_requests_; + + // Urls for which a download has already been initiated. Used to prevent + // duplicate downloads of the same url. + std::set<GURL> processed_urls_; + + // Callback to run on favicon download completion. + FaviconDownloaderCallback callback_; + + base::WeakPtrFactory<FaviconDownloader> weak_ptr_factory_; + + DISALLOW_COPY_AND_ASSIGN(FaviconDownloader); +}; + +#endif // CHROME_BROWSER_EXTENSIONS_FAVICON_DOWNLOADER_H_ diff --git a/chrome/browser/extensions/favicon_downloader_unittest.cc b/chrome/browser/extensions/favicon_downloader_unittest.cc new file mode 100644 index 0000000..a16cfde --- /dev/null +++ b/chrome/browser/extensions/favicon_downloader_unittest.cc @@ -0,0 +1,197 @@ +// 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/extensions/favicon_downloader.h" + +#include "base/files/scoped_temp_dir.h" +#include "chrome/test/base/chrome_render_view_host_test_harness.h" +#include "content/public/common/favicon_url.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "third_party/skia/include/core/SkBitmap.h" + +using content::RenderViewHostTester; + +namespace { + +// Creates valid SkBitmaps of the dimensions found in |sizes| and pushes them +// into |bitmaps|. +std::vector<SkBitmap> CreateTestBitmaps(const std::vector<gfx::Size>& sizes) { + std::vector<SkBitmap> bitmaps(sizes.size()); + for (size_t i = 0; i < sizes.size(); ++i) { + SkBitmap& bitmap = bitmaps[i]; + bitmap.setConfig(SkBitmap::kARGB_8888_Config, + sizes[i].width(), + sizes[i].height()); + bitmap.allocPixels(); + bitmap.eraseColor(SK_ColorRED); + } + return bitmaps; +} + +class FaviconDownloaderTest : public ChromeRenderViewHostTestHarness { + protected: + FaviconDownloaderTest() { + } + + virtual ~FaviconDownloaderTest() { + } + + private: + DISALLOW_COPY_AND_ASSIGN(FaviconDownloaderTest); +}; + +} // namespace + +class TestFaviconDownloader : public FaviconDownloader { + public: + TestFaviconDownloader(content::WebContents* web_contents, + std::vector<GURL> extra_favicon_urls) + : FaviconDownloader( + web_contents, + extra_favicon_urls, + base::Bind(&TestFaviconDownloader::DownloadsComplete, + base::Unretained(this))), + id_counter_(0) { + } + virtual ~TestFaviconDownloader() {} + + virtual int DownloadImage(const GURL& url) OVERRIDE { + return id_counter_++; + } + + virtual std::vector<content::FaviconURL> GetFaviconURLsFromWebContents() + OVERRIDE { + return initial_favicon_urls_; + } + + size_t pending_requests() const { + return in_progress_requests_.size(); + } + + void DownloadsComplete(bool success, + const FaviconDownloader::FaviconMap& map) { + favicon_map_ = map; + } + + FaviconDownloader::FaviconMap favicon_map() const { + return favicon_map_; + } + + void CompleteImageDownload( + int id, + const GURL& image_url, + const std::vector<gfx::Size>& original_bitmap_sizes) { + FaviconDownloader::DidDownloadFavicon(id, 200, image_url, + CreateTestBitmaps(original_bitmap_sizes), original_bitmap_sizes); + } + + void UpdateFaviconURLs(const std::vector<content::FaviconURL>& candidates) { + FaviconDownloader::DidUpdateFaviconURL(0, candidates); + } + + void set_initial_favicon_urls(const std::vector<content::FaviconURL>& urls) { + initial_favicon_urls_ = urls; + } + + private: + std::vector<content::FaviconURL> initial_favicon_urls_; + FaviconDownloader::FaviconMap favicon_map_; + int id_counter_; + DISALLOW_COPY_AND_ASSIGN(TestFaviconDownloader); +}; + +TEST_F(FaviconDownloaderTest, SimpleDownload) { + const GURL favicon_url("http://www.google.com/favicon.ico"); + TestFaviconDownloader downloader(web_contents(), std::vector<GURL>()); + + std::vector<content::FaviconURL> favicon_urls; + favicon_urls.push_back( + content::FaviconURL(favicon_url, + content::FaviconURL::FAVICON)); + downloader.set_initial_favicon_urls(favicon_urls); + EXPECT_EQ(0u, downloader.pending_requests()); + + downloader.Start(); + EXPECT_EQ(1u, downloader.pending_requests()); + + std::vector<gfx::Size> sizes(1, gfx::Size(32, 32)); + downloader.CompleteImageDownload(0, favicon_urls[0].icon_url, sizes); + EXPECT_EQ(0u, downloader.pending_requests()); + + EXPECT_EQ(1u, downloader.favicon_map().size()); + EXPECT_EQ(1u, downloader.favicon_map()[favicon_url].size()); +} + +TEST_F(FaviconDownloaderTest, DownloadWithUrlsFromWebContentsNotification) { + const GURL favicon_url("http://www.google.com/favicon.ico"); + TestFaviconDownloader downloader(web_contents(), std::vector<GURL>()); + + std::vector<content::FaviconURL> favicon_urls; + favicon_urls.push_back( + content::FaviconURL(favicon_url, + content::FaviconURL::FAVICON)); + EXPECT_EQ(0u, downloader.pending_requests()); + + // Start downloader before favicon URLs are loaded. + downloader.Start(); + EXPECT_EQ(0u, downloader.pending_requests()); + + downloader.UpdateFaviconURLs(favicon_urls); + EXPECT_EQ(1u, downloader.pending_requests()); + + std::vector<gfx::Size> sizes(1, gfx::Size(32, 32)); + downloader.CompleteImageDownload(0, favicon_urls[0].icon_url, sizes); + EXPECT_EQ(0u, downloader.pending_requests()); + + EXPECT_EQ(1u, downloader.favicon_map().size()); + EXPECT_EQ(1u, downloader.favicon_map()[favicon_url].size()); +} + +TEST_F(FaviconDownloaderTest, DownloadMultipleUrls) { + const GURL empty_favicon("http://www.google.com/empty_favicon.ico"); + const GURL favicon_url_1("http://www.google.com/favicon.ico"); + const GURL favicon_url_2("http://www.google.com/favicon2.ico"); + + std::vector<GURL> extra_urls; + // This should get downloaded. + extra_urls.push_back(favicon_url_2); + // This is duplicated in the favicon urls and should only be downloaded once. + extra_urls.push_back(empty_favicon); + + TestFaviconDownloader downloader(web_contents(), extra_urls); + std::vector<content::FaviconURL> favicon_urls; + favicon_urls.push_back( + content::FaviconURL(favicon_url_1, + content::FaviconURL::FAVICON)); + // This is duplicated in the favicon urls and should only be downloaded once. + favicon_urls.push_back( + content::FaviconURL(empty_favicon, + content::FaviconURL::FAVICON)); + // Invalid icons shouldn't get put into the download queue. + favicon_urls.push_back( + content::FaviconURL(GURL("http://www.google.com/invalid.ico"), + content::FaviconURL::INVALID_ICON)); + downloader.set_initial_favicon_urls(favicon_urls); + downloader.Start(); + EXPECT_EQ(3u, downloader.pending_requests()); + + std::vector<gfx::Size> sizes_1(1, gfx::Size(16, 16)); + downloader.CompleteImageDownload(0, favicon_url_1, sizes_1); + + std::vector<gfx::Size> sizes_2; + sizes_2.push_back(gfx::Size(32, 32)); + sizes_2.push_back(gfx::Size(64, 64)); + downloader.CompleteImageDownload(1, favicon_url_2, sizes_2); + + // Only 1 download should have been initiated for |empty_favicon| even though + // the URL was in both the web app info and the favicon urls. + downloader.CompleteImageDownload(2, empty_favicon, std::vector<gfx::Size>()); + EXPECT_EQ(0u, downloader.pending_requests()); + + EXPECT_EQ(3u, downloader.favicon_map().size()); + EXPECT_EQ(0u, downloader.favicon_map()[empty_favicon].size()); + EXPECT_EQ(1u, downloader.favicon_map()[favicon_url_1].size()); + EXPECT_EQ(2u, downloader.favicon_map()[favicon_url_2].size()); +} + diff --git a/chrome/browser/extensions/tab_helper.cc b/chrome/browser/extensions/tab_helper.cc index 124fef0..d26c213 100644 --- a/chrome/browser/extensions/tab_helper.cc +++ b/chrome/browser/extensions/tab_helper.cc @@ -6,6 +6,7 @@ #include "base/logging.h" #include "base/strings/string_util.h" +#include "base/strings/utf_string_conversions.h" #include "chrome/browser/chrome_notification_types.h" #include "chrome/browser/extensions/activity_log/activity_log.h" #include "chrome/browser/extensions/api/declarative/rules_registry_service.h" @@ -17,6 +18,7 @@ #include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/extensions/extension_system.h" #include "chrome/browser/extensions/extension_tab_util.h" +#include "chrome/browser/extensions/favicon_downloader.h" #include "chrome/browser/extensions/image_loader.h" #include "chrome/browser/extensions/page_action_controller.h" #include "chrome/browser/extensions/script_badge_controller.h" @@ -282,6 +284,71 @@ bool TabHelper::OnMessageReceived(const IPC::Message& message) { return handled; } +void TabHelper::CreateHostedApp() { + // Add urls from the WebApplicationInfo. + std::vector<GURL> web_app_info_icon_urls; + for (std::vector<WebApplicationInfo::IconInfo>::const_iterator it = + web_app_info_.icons.begin(); + it != web_app_info_.icons.end(); ++it) { + if (it->url.is_valid()) + web_app_info_icon_urls.push_back(it->url); + } + + favicon_downloader_.reset( + new FaviconDownloader(web_contents(), + web_app_info_icon_urls, + base::Bind(&TabHelper::FinishCreateHostedApp, + base::Unretained(this)))); + favicon_downloader_->Start(); +} + +void TabHelper::FinishCreateHostedApp( + bool success, + const std::map<GURL, std::vector<SkBitmap> >& bitmaps) { + // The tab has navigated away during the icon download. Cancel the hosted app + // creation. + if (!success) { + favicon_downloader_.reset(); + return; + } + + WebApplicationInfo install_info(web_app_info_); + if (install_info.app_url.is_empty()) + install_info.app_url = web_contents()->GetURL(); + + if (install_info.title.empty()) + install_info.title = web_contents()->GetTitle(); + if (install_info.title.empty()) + install_info.title = UTF8ToUTF16(install_info.app_url.spec()); + + install_info.urls.push_back(install_info.app_url); + install_info.is_bookmark_app = true; + + // Add the downloaded icons. + for (FaviconDownloader::FaviconMap::const_iterator map_it = bitmaps.begin(); + map_it != bitmaps.end(); ++map_it) { + for (std::vector<SkBitmap>::const_iterator bitmap_it = + map_it->second.begin(); + bitmap_it != map_it->second.end(); ++bitmap_it) { + if (!bitmap_it->empty()) { + WebApplicationInfo::IconInfo icon_info; + icon_info.data = *bitmap_it; + icon_info.width = icon_info.data.width(); + icon_info.height = icon_info.data.height(); + install_info.icons.push_back(icon_info); + } + } + } + + Profile* profile = + Profile::FromBrowserContext(web_contents()->GetBrowserContext()); + scoped_refptr<extensions::CrxInstaller> installer( + extensions::CrxInstaller::CreateSilent(profile->GetExtensionService())); + installer->set_error_on_unsupported_requirements(true); + installer->InstallWebApp(install_info); + favicon_downloader_.reset(); +} + void TabHelper::DidCloneToNewWebContents(WebContents* old_web_contents, WebContents* new_web_contents) { // When the WebContents that this is attached to is cloned, give the new clone @@ -312,7 +379,7 @@ void TabHelper::OnDidGetApplicationInfo(int32 page_id, break; } case CREATE_HOSTED_APP: { - CreateHostedApp(info); + CreateHostedApp(); break; } case UPDATE_SHORTCUT: { @@ -331,39 +398,6 @@ void TabHelper::OnDidGetApplicationInfo(int32 page_id, #endif } -void TabHelper::CreateHostedApp(const WebApplicationInfo& info) { - ShellIntegration::ShortcutInfo shortcut_info; - web_app::GetShortcutInfoForTab(web_contents(), &shortcut_info); - WebApplicationInfo web_app_info; - - web_app_info.is_bookmark_app = true; - web_app_info.app_url = shortcut_info.url; - web_app_info.title = shortcut_info.title; - web_app_info.urls.push_back(web_app_info.app_url); - - // TODO(calamity): this should attempt to download the best icon that it can - // from |info.icons| rather than just using the favicon as it scales up badly. - // Fix this once |info.icons| gets populated commonly. - - // Get the smallest icon in the icon family (should have only 1). - const gfx::Image* icon = shortcut_info.favicon.GetBest(0, 0); - SkBitmap bitmap = icon ? icon->AsBitmap() : SkBitmap(); - - if (!icon->IsEmpty()) { - WebApplicationInfo::IconInfo icon_info; - icon_info.data = bitmap; - icon_info.width = icon_info.data.width(); - icon_info.height = icon_info.data.height(); - web_app_info.icons.push_back(icon_info); - } - - ExtensionService* service = profile_->GetExtensionService(); - scoped_refptr<extensions::CrxInstaller> installer( - extensions::CrxInstaller::CreateSilent(service)); - installer->set_error_on_unsupported_requirements(true); - installer->InstallWebApp(web_app_info); -} - void TabHelper::OnInlineWebstoreInstall( int install_id, int return_route_id, diff --git a/chrome/browser/extensions/tab_helper.h b/chrome/browser/extensions/tab_helper.h index e41a10c..0905d71 100644 --- a/chrome/browser/extensions/tab_helper.h +++ b/chrome/browser/extensions/tab_helper.h @@ -23,6 +23,8 @@ #include "extensions/common/stack_frame.h" #include "third_party/skia/include/core/SkBitmap.h" +class FaviconDownloader; + namespace content { struct LoadCommittedDetails; } @@ -99,8 +101,6 @@ class TabHelper : public content::WebContentsObserver, void CreateHostedAppFromWebContents(); bool CanCreateApplicationShortcuts() const; - void CreateHostedApp(const WebApplicationInfo& info); - void set_pending_web_app_action(WebAppAction action) { pending_web_app_action_ = action; } @@ -170,6 +170,12 @@ class TabHelper : public content::WebContentsObserver, explicit TabHelper(content::WebContents* web_contents); friend class content::WebContentsUserData<TabHelper>; + // Creates a hosted app for the current tab. Requires the |web_app_info_| to + // be populated. + void CreateHostedApp(); + void FinishCreateHostedApp( + bool success, const std::map<GURL, std::vector<SkBitmap> >& bitmaps); + // content::WebContentsObserver overrides. virtual void RenderViewCreated( content::RenderViewHost* render_view_host) OVERRIDE; @@ -269,6 +275,8 @@ class TabHelper : public content::WebContentsObserver, scoped_ptr<ScriptBubbleController> script_bubble_controller_; + scoped_ptr<FaviconDownloader> favicon_downloader_; + Profile* profile_; // Vend weak pointers that can be invalidated to stop in-progress loads. diff --git a/chrome/browser/favicon/favicon_tab_helper.cc b/chrome/browser/favicon/favicon_tab_helper.cc index 4ab113b..cbc55a0 100644 --- a/chrome/browser/favicon/favicon_tab_helper.cc +++ b/chrome/browser/favicon/favicon_tab_helper.cc @@ -175,6 +175,7 @@ void FaviconTabHelper::DidStartNavigationToPendingEntry( void FaviconTabHelper::DidNavigateMainFrame( const content::LoadCommittedDetails& details, const content::FrameNavigateParams& params) { + favicon_urls_.clear(); // Get the favicon, either from history or request it from the net. FetchFavicon(details.entry->GetURL()); } @@ -182,6 +183,9 @@ void FaviconTabHelper::DidNavigateMainFrame( void FaviconTabHelper::DidUpdateFaviconURL( int32 page_id, const std::vector<content::FaviconURL>& candidates) { + DCHECK(!candidates.empty()); + favicon_urls_ = candidates; + favicon_handler_->OnUpdateFaviconURL(page_id, candidates); if (touch_icon_handler_.get()) touch_icon_handler_->OnUpdateFaviconURL(page_id, candidates); diff --git a/chrome/browser/favicon/favicon_tab_helper.h b/chrome/browser/favicon/favicon_tab_helper.h index b8255a6..7758121 100644 --- a/chrome/browser/favicon/favicon_tab_helper.h +++ b/chrome/browser/favicon/favicon_tab_helper.h @@ -51,6 +51,12 @@ class FaviconTabHelper : public content::WebContentsObserver, // space is provided for the favicon, and the favicon is never displayed. virtual bool ShouldDisplayFavicon(); + // Returns the current tab's favicon urls. If this is empty, + // DidUpdateFaviconURL has not yet been called for the current navigation. + const std::vector<content::FaviconURL>& favicon_urls() const { + return favicon_urls_; + } + // Allows the client to determine if they want to fetch the Favicons as // they are discovered. void set_should_fetch_icons(bool fetch) { @@ -94,6 +100,8 @@ class FaviconTabHelper : public content::WebContentsObserver, Profile* profile_; bool should_fetch_icons_; + std::vector<content::FaviconURL> favicon_urls_; + scoped_ptr<FaviconHandler> favicon_handler_; // Handles downloading touchicons. It is NULL if diff --git a/chrome/chrome_browser_extensions.gypi b/chrome/chrome_browser_extensions.gypi index 0e62089..a537372 100644 --- a/chrome/chrome_browser_extensions.gypi +++ b/chrome/chrome_browser_extensions.gypi @@ -800,6 +800,8 @@ 'browser/extensions/external_provider_impl.h', 'browser/extensions/external_registry_loader_win.cc', 'browser/extensions/external_registry_loader_win.h', + 'browser/extensions/favicon_downloader.cc', + 'browser/extensions/favicon_downloader.h', 'browser/extensions/global_shortcut_listener.cc', 'browser/extensions/global_shortcut_listener.h', 'browser/extensions/global_shortcut_listener_chromeos.cc', diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi index e7ca74c..f614049 100644 --- a/chrome/chrome_tests_unit.gypi +++ b/chrome/chrome_tests_unit.gypi @@ -911,6 +911,7 @@ 'browser/extensions/external_policy_loader_unittest.cc', 'browser/extensions/external_provider_impl_unittest.cc', 'browser/extensions/external_provider_impl_chromeos_unittest.cc', + 'browser/extensions/favicon_downloader_unittest.cc', 'browser/extensions/image_loader_unittest.cc', 'browser/extensions/menu_manager_unittest.cc', 'browser/extensions/pack_extension_unittest.cc', diff --git a/content/renderer/render_view_browsertest.cc b/content/renderer/render_view_browsertest.cc index d378ab6..6b9aa19 100644 --- a/content/renderer/render_view_browsertest.cc +++ b/content/renderer/render_view_browsertest.cc @@ -2180,4 +2180,26 @@ TEST_F(RenderViewImplTest, SendCandidateWindowEvents) { EXPECT_EQ(output, "\nResult:showupdatehide"); } +// Ensure the render view sends favicon url update events correctly. +TEST_F(RenderViewImplTest, SendFaviconURLUpdateEvent) { + // An event should be sent when a favicon url exists. + LoadHTML("<html>" + "<head>" + "<link rel='icon' href='http://www.google.com/favicon.ico'>" + "</head>" + "</html>"); + EXPECT_TRUE(render_thread_->sink().GetFirstMessageMatching( + ViewHostMsg_UpdateFaviconURL::ID)); + render_thread_->sink().ClearMessages(); + + // An event should not be sent if no favicon url exists. This is an assumption + // made by some of Chrome's favicon handling. + LoadHTML("<html>" + "<head>" + "</head>" + "</html>"); + EXPECT_FALSE(render_thread_->sink().GetFirstMessageMatching( + ViewHostMsg_UpdateFaviconURL::ID)); +} + } // namespace content |