diff options
author | mek@chromium.org <mek@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-11-16 07:40:47 +0000 |
---|---|---|
committer | mek@chromium.org <mek@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-11-16 07:40:47 +0000 |
commit | ec7de0c5ac1ffe869a1c3e03c1f814fac2ae0746 (patch) | |
tree | ff4ce659a13029f30bf40bc6c98a9f9c1461fb9b | |
parent | c05318cc64b136f405fd9a117e0544433c2c698b (diff) | |
download | chromium_src-ec7de0c5ac1ffe869a1c3e03c1f814fac2ae0746.zip chromium_src-ec7de0c5ac1ffe869a1c3e03c1f814fac2ae0746.tar.gz chromium_src-ec7de0c5ac1ffe869a1c3e03c1f814fac2ae0746.tar.bz2 |
Add a class to replace ImageLoadingTracker with a nicer API.
This adds a new extensions::ImageLoader class, which mostly serves the same purpose as ImageLoadingTracker, but with some differences. It currently doesn't do any caching, and it uses callbacks instead of a delegate to pass the loaded image back to the caller. This should make this new class much nicer to use.
Also ported some ILT usage to the new ImageLoader class.
BUG=141673
Review URL: https://chromiumcodereview.appspot.com/11027044
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@168155 0039d316-1c4b-4281-b951-d872f2087c98
23 files changed, 960 insertions, 325 deletions
diff --git a/build/android/gtest_filter/unit_tests_disabled b/build/android/gtest_filter/unit_tests_disabled index 14070a5..1906e60 100644 --- a/build/android/gtest_filter/unit_tests_disabled +++ b/build/android/gtest_filter/unit_tests_disabled @@ -86,6 +86,7 @@ ExternalPolicyProviderTest.* MenuManagerTest.* PageActionControllerTest.* PermissionsUpdaterTest.* +ImageLoaderTest.* ImageLoadingTrackerTest.* ScriptBadgeControllerTest.* ExtensionSettingsFrontendTest.* diff --git a/chrome/browser/background/background_application_list_model.cc b/chrome/browser/background/background_application_list_model.cc index e0c6010..3b2faf0 100644 --- a/chrome/browser/background/background_application_list_model.cc +++ b/chrome/browser/background/background_application_list_model.cc @@ -16,7 +16,7 @@ #include "chrome/browser/browser_process.h" #include "chrome/browser/extensions/extension_prefs.h" #include "chrome/browser/extensions/extension_service.h" -#include "chrome/browser/extensions/image_loading_tracker.h" +#include "chrome/browser/extensions/image_loader.h" #include "chrome/browser/profiles/profile.h" #include "chrome/common/chrome_notification_types.h" #include "chrome/common/extensions/extension.h" @@ -60,7 +60,7 @@ bool ExtensionNameComparator::operator()(const Extension* x, // Background application representation, private to the // BackgroundApplicationListModel class. class BackgroundApplicationListModel::Application - : public ImageLoadingTracker::Observer { + : public base::SupportsWeakPtr<Application> { public: Application(BackgroundApplicationListModel* model, const Extension* an_extension); @@ -68,9 +68,7 @@ class BackgroundApplicationListModel::Application virtual ~Application(); // Invoked when a request icon is available. - virtual void OnImageLoaded(const gfx::Image& image, - const std::string& extension_id, - int index) OVERRIDE; + void OnImageLoaded(const gfx::Image& image); // Uses the FILE thread to request this extension's icon, sized // appropriately. @@ -79,7 +77,6 @@ class BackgroundApplicationListModel::Application const Extension* extension_; scoped_ptr<gfx::ImageSkia> icon_; BackgroundApplicationListModel* model_; - ImageLoadingTracker tracker_; }; namespace { @@ -141,14 +138,11 @@ BackgroundApplicationListModel::Application::Application( const Extension* extension) : extension_(extension), icon_(NULL), - model_(model), - ALLOW_THIS_IN_INITIALIZER_LIST(tracker_(this)) { + model_(model) { } void BackgroundApplicationListModel::Application::OnImageLoaded( - const gfx::Image& image, - const std::string& extension_id, - int index) { + const gfx::Image& image) { if (image.IsEmpty()) return; icon_.reset(image.CopyImageSkia()); @@ -159,8 +153,9 @@ void BackgroundApplicationListModel::Application::RequestIcon( extension_misc::ExtensionIcons size) { ExtensionResource resource = extension_->GetIconResource( size, ExtensionIconSet::MATCH_BIGGER); - tracker_.LoadImage(extension_, resource, gfx::Size(size, size), - ImageLoadingTracker::CACHE); + extensions::ImageLoader::Get(model_->profile_)->LoadImageAsync( + extension_, resource, gfx::Size(size, size), + base::Bind(&Application::OnImageLoaded, AsWeakPtr())); } BackgroundApplicationListModel::~BackgroundApplicationListModel() { diff --git a/chrome/browser/extensions/extension_icon_image_unittest.cc b/chrome/browser/extensions/extension_icon_image_unittest.cc index 8ca3747..e293ed8 100644 --- a/chrome/browser/extensions/extension_icon_image_unittest.cc +++ b/chrome/browser/extensions/extension_icon_image_unittest.cc @@ -7,7 +7,7 @@ #include "base/json/json_file_value_serializer.h" #include "base/message_loop.h" #include "base/path_service.h" -#include "chrome/browser/extensions/image_loading_tracker.h" +#include "chrome/browser/extensions/image_loader.h" #include "chrome/common/chrome_paths.h" #include "chrome/common/extensions/extension.h" #include "chrome/common/extensions/extension_constants.h" @@ -65,20 +65,16 @@ class MockImageSkiaSource : public gfx::ImageSkiaSource { }; // Helper class for synchronously loading extension image resource. -class TestImageLoader : public ImageLoadingTracker::Observer { +class TestImageLoader { public: explicit TestImageLoader(const Extension* extension) : extension_(extension), waiting_(false), - image_loaded_(false), - ALLOW_THIS_IN_INITIALIZER_LIST(tracker_(this)) { + image_loaded_(false) { } virtual ~TestImageLoader() {} - // ImageLoadingTracker::Observer override. - virtual void OnImageLoaded(const gfx::Image& image, - const std::string& extension_id, - int index) OVERRIDE { + void OnImageLoaded(const gfx::Image& image) { image_ = image; image_loaded_ = true; if (waiting_) @@ -86,14 +82,13 @@ class TestImageLoader : public ImageLoadingTracker::Observer { } SkBitmap LoadBitmap(const std::string& path, - int size, - ImageLoadingTracker::CacheParam cache_param) { + int size) { image_loaded_ = false; - tracker_.LoadImage(extension_, - extension_->GetResource(path), - gfx::Size(size, size), - cache_param); + image_loader_.LoadImageAsync( + extension_, extension_->GetResource(path), gfx::Size(size, size), + base::Bind(&TestImageLoader::OnImageLoaded, + base::Unretained(this))); // If |image_| still hasn't been loaded (i.e. it is being loaded // asynchronously), wait for it. @@ -113,7 +108,7 @@ class TestImageLoader : public ImageLoadingTracker::Observer { bool waiting_; bool image_loaded_; gfx::Image image_; - ImageLoadingTracker tracker_; + extensions::ImageLoader image_loader_; DISALLOW_COPY_AND_ASSIGN(TestImageLoader); }; @@ -192,10 +187,9 @@ class ExtensionIconImageTest : public testing::Test, // The image will be loaded from the relative path |path|. SkBitmap GetTestBitmap(const Extension* extension, const std::string& path, - int size, - ImageLoadingTracker::CacheParam cache_param) { + int size) { TestImageLoader image_loader(extension); - return image_loader.LoadBitmap(path, size, cache_param); + return image_loader.LoadBitmap(path, size); } private: @@ -221,13 +215,13 @@ TEST_F(ExtensionIconImageTest, Basic) { // Load images we expect to find as representations in icon_image, so we // can later use them to validate icon_image. SkBitmap bitmap_16 = - GetTestBitmap(extension, "16.png", 16, ImageLoadingTracker::DONT_CACHE); + GetTestBitmap(extension, "16.png", 16); ASSERT_FALSE(bitmap_16.empty()); // There is no image of size 32 defined in the extension manifest, so we // should expect manifest image of size 48 resized to size 32. SkBitmap bitmap_48_resized_to_32 = - GetTestBitmap(extension, "48.png", 32, ImageLoadingTracker::DONT_CACHE); + GetTestBitmap(extension, "48.png", 32); ASSERT_FALSE(bitmap_48_resized_to_32.empty()); IconImage image(extension, extension->icons(), 16, default_icon, this); @@ -287,7 +281,7 @@ TEST_F(ExtensionIconImageTest, FallbackToSmallerWhenNoBigger) { // Load images we expect to find as representations in icon_image, so we // can later use them to validate icon_image. SkBitmap bitmap_48 = - GetTestBitmap(extension, "48.png", 48, ImageLoadingTracker::DONT_CACHE); + GetTestBitmap(extension, "48.png", 48); ASSERT_FALSE(bitmap_48.empty()); IconImage image(extension, extension->icons(), 32, default_icon, this); @@ -322,7 +316,7 @@ TEST_F(ExtensionIconImageTest, FallbackToSmaller) { // Load images we expect to find as representations in icon_image, so we // can later use them to validate icon_image. SkBitmap bitmap_16 = - GetTestBitmap(extension, "16.png", 16, ImageLoadingTracker::DONT_CACHE); + GetTestBitmap(extension, "16.png", 16); ASSERT_FALSE(bitmap_16.empty()); IconImage image(extension, extension->icons(), 17, default_icon, this); @@ -474,10 +468,12 @@ TEST_F(ExtensionIconImageTest, LoadPrecachedImage) { gfx::ImageSkia default_icon = GetDefaultIcon(); - // Note the cache parameter. + // Store the image in the cache. SkBitmap bitmap_16 = - GetTestBitmap(extension, "16.png", 16, ImageLoadingTracker::CACHE); + GetTestBitmap(extension, "16.png", 16); ASSERT_FALSE(bitmap_16.empty()); + extension->SetCachedImage(extension->GetResource("16.png"), bitmap_16, + gfx::Size(16, 16)); IconImage image(extension, extension->icons(), 16, default_icon, this); @@ -510,7 +506,7 @@ TEST_F(ExtensionIconImageTest, IconImageDestruction) { // Load images we expect to find as representations in icon_image, so we // can later use them to validate icon_image. SkBitmap bitmap_16 = - GetTestBitmap(extension, "16.png", 16, ImageLoadingTracker::DONT_CACHE); + GetTestBitmap(extension, "16.png", 16); ASSERT_FALSE(bitmap_16.empty()); scoped_ptr<IconImage> image( diff --git a/chrome/browser/extensions/extension_install_prompt.cc b/chrome/browser/extensions/extension_install_prompt.cc index dff6daa..f38287f 100644 --- a/chrome/browser/extensions/extension_install_prompt.cc +++ b/chrome/browser/extensions/extension_install_prompt.cc @@ -15,6 +15,7 @@ #include "base/utf_string_conversions.h" #include "chrome/browser/extensions/bundle_installer.h" #include "chrome/browser/extensions/extension_install_ui.h" +#include "chrome/browser/extensions/image_loader.h" #include "chrome/browser/prefs/pref_service.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/signin/token_service.h" @@ -376,9 +377,9 @@ ExtensionInstallPrompt::ExtensionInstallPrompt( extension_(NULL), install_ui_(ExtensionInstallUI::Create(ProfileForWebContents(contents))), delegate_(NULL), - prompt_(ProfileForWebContents(contents), UNSET_PROMPT_TYPE), - prompt_type_(UNSET_PROMPT_TYPE), - ALLOW_THIS_IN_INITIALIZER_LIST(tracker_(this)) { + profile_(ProfileForWebContents(contents)), + prompt_(profile_, UNSET_PROMPT_TYPE), + prompt_type_(UNSET_PROMPT_TYPE) { } ExtensionInstallPrompt::~ExtensionInstallPrompt() { @@ -529,16 +530,15 @@ void ExtensionInstallPrompt::SetIcon(const SkBitmap* image) { } } -void ExtensionInstallPrompt::OnImageLoaded(const gfx::Image& image, - const std::string& extension_id, - int index) { +void ExtensionInstallPrompt::OnImageLoaded(const gfx::Image& image) { SetIcon(image.IsEmpty() ? NULL : image.ToSkBitmap()); FetchOAuthIssueAdviceIfNeeded(); } void ExtensionInstallPrompt::LoadImageIfNeeded() { // Bundle install prompts do not have an icon. - if (!icon_.empty()) { + // Also |profile_| can be NULL in unit tests. + if (!icon_.empty() || !profile_) { FetchOAuthIssueAdviceIfNeeded(); return; } @@ -552,9 +552,9 @@ void ExtensionInstallPrompt::LoadImageIfNeeded() { // TODO(tbarzic): We should use IconImage here and load the required bitmap // lazily. int pixel_size = GetSizeForMaxScaleFactor(kIconSize); - tracker_.LoadImage(extension_, image, - gfx::Size(pixel_size, pixel_size), - ImageLoadingTracker::DONT_CACHE); + extensions::ImageLoader::Get(profile_)->LoadImageAsync( + extension_, image, gfx::Size(pixel_size, pixel_size), + base::Bind(&ExtensionInstallPrompt::OnImageLoaded, AsWeakPtr())); } void ExtensionInstallPrompt::FetchOAuthIssueAdviceIfNeeded() { diff --git a/chrome/browser/extensions/extension_install_prompt.h b/chrome/browser/extensions/extension_install_prompt.h index 2d3914a..ad89799 100644 --- a/chrome/browser/extensions/extension_install_prompt.h +++ b/chrome/browser/extensions/extension_install_prompt.h @@ -13,7 +13,6 @@ #include "base/memory/scoped_ptr.h" #include "base/string16.h" #include "chrome/browser/extensions/crx_installer_error.h" -#include "chrome/browser/extensions/image_loading_tracker.h" #include "extensions/common/url_pattern.h" #include "google_apis/gaia/oauth2_mint_token_flow.h" #include "third_party/skia/include/core/SkBitmap.h" @@ -43,8 +42,9 @@ class PermissionSet; } // namespace extensions // Displays all the UI around extension installation. -class ExtensionInstallPrompt : public ImageLoadingTracker::Observer, - public OAuth2MintTokenFlow::Delegate { +class ExtensionInstallPrompt + : public OAuth2MintTokenFlow::Delegate, + public base::SupportsWeakPtr<ExtensionInstallPrompt> { public: enum PromptType { UNSET_PROMPT_TYPE = -1, @@ -260,11 +260,6 @@ class ExtensionInstallPrompt : public ImageLoadingTracker::Observer, // Installation failed. This is declared virtual for testing. virtual void OnInstallFailure(const extensions::CrxInstallerError& error); - // ImageLoadingTracker::Observer: - virtual void OnImageLoaded(const gfx::Image& image, - const std::string& extension_id, - int index) OVERRIDE; - protected: friend class extensions::ExtensionWebstorePrivateApiTest; friend class WebstoreStandaloneInstallUnpackFailureTest; @@ -280,6 +275,9 @@ class ExtensionInstallPrompt : public ImageLoadingTracker::Observer, // an empty bitmap, then a default icon will be used instead. void SetIcon(const SkBitmap* icon); + // ImageLoader callback. + void OnImageLoaded(const gfx::Image& image); + // Starts the process of showing a confirmation UI, which is split into two. // 1) Set off a 'load icon' task. // 2) Handle the load icon response and show the UI (OnImageLoaded). @@ -319,6 +317,8 @@ class ExtensionInstallPrompt : public ImageLoadingTracker::Observer, // The delegate we will call Proceed/Abort on after confirmation UI. Delegate* delegate_; + Profile* profile_; + // A pre-filled prompt. Prompt prompt_; @@ -327,10 +327,6 @@ class ExtensionInstallPrompt : public ImageLoadingTracker::Observer, scoped_ptr<OAuth2MintTokenFlow> token_flow_; - // Keeps track of extension images being loaded on the File thread for the - // purpose of showing the install UI. - ImageLoadingTracker tracker_; - // Used to show the confirm dialog. ShowDialogCallback show_dialog_callback_; }; diff --git a/chrome/browser/extensions/extension_protocols.cc b/chrome/browser/extensions/extension_protocols.cc index f33300d..a9edd50 100644 --- a/chrome/browser/extensions/extension_protocols.cc +++ b/chrome/browser/extensions/extension_protocols.cc @@ -18,7 +18,7 @@ #include "base/threading/worker_pool.h" #include "build/build_config.h" #include "chrome/browser/extensions/extension_info_map.h" -#include "chrome/browser/extensions/image_loading_tracker.h" +#include "chrome/browser/extensions/image_loader.h" #include "chrome/browser/net/chrome_url_request_context.h" #include "chrome/common/chrome_paths.h" #include "chrome/common/extensions/extension.h" @@ -378,7 +378,7 @@ ExtensionProtocolHandler::MaybeCreateJob( FilePath request_path = extension_file_util::ExtensionURLToRelativeFilePath(request->url()); int resource_id; - if (ImageLoadingTracker::IsComponentExtensionResource(extension, + if (extensions::ImageLoader::IsComponentExtensionResource(extension, request_path, &resource_id)) { relative_path = relative_path.Append(request_path); relative_path = relative_path.NormalizePathSeparators(); diff --git a/chrome/browser/extensions/extension_system.h b/chrome/browser/extensions/extension_system.h index ca3ac3e..cbab7f8 100644 --- a/chrome/browser/extensions/extension_system.h +++ b/chrome/browser/extensions/extension_system.h @@ -164,8 +164,9 @@ class ExtensionSystemImpl : public ExtensionSystem { virtual ExtensionDevToolsManager* devtools_manager() OVERRIDE; virtual ExtensionProcessManager* process_manager() OVERRIDE; virtual AlarmManager* alarm_manager() OVERRIDE; - virtual StateStore* state_store() OVERRIDE; - virtual ShellWindowGeometryCache* shell_window_geometry_cache() OVERRIDE; + virtual StateStore* state_store() OVERRIDE; // shared + virtual ShellWindowGeometryCache* shell_window_geometry_cache() + OVERRIDE; // shared virtual LazyBackgroundTaskQueue* lazy_background_task_queue() OVERRIDE; // shared virtual ExtensionInfoMap* info_map() OVERRIDE; // shared diff --git a/chrome/browser/extensions/extension_web_ui.cc b/chrome/browser/extensions/extension_web_ui.cc index b98641a..08f05d3 100644 --- a/chrome/browser/extensions/extension_web_ui.cc +++ b/chrome/browser/extensions/extension_web_ui.cc @@ -13,7 +13,7 @@ #include "chrome/browser/bookmarks/bookmark_manager_extension_api.h" #include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/extensions/extension_tab_util.h" -#include "chrome/browser/extensions/image_loading_tracker.h" +#include "chrome/browser/extensions/image_loader.h" #include "chrome/browser/prefs/pref_service.h" #include "chrome/browser/prefs/scoped_user_pref_update.h" #include "chrome/browser/profiles/profile.h" @@ -78,113 +78,49 @@ void UnregisterAndReplaceOverrideForWebContents( content::PAGE_TRANSITION_RELOAD, std::string()); } -// Helper class that is used to track the loading of the favicon of an -// extension. -class ExtensionWebUIImageLoadingTracker : public ImageLoadingTracker::Observer { - public: - ExtensionWebUIImageLoadingTracker(Profile* profile, - FaviconService::GetFaviconRequest* request, - const GURL& page_url) - : ALLOW_THIS_IN_INITIALIZER_LIST(tracker_(this)), - request_(request), - extension_(NULL) { - // Even when the extensions service is enabled by default, it's still - // disabled in incognito mode. - ExtensionService* service = profile->GetExtensionService(); - if (service) - extension_ = service->extensions()->GetByID(page_url.host()); - } - - void Init() { - if (extension_) { - // Fetch resources for all supported scale factors for which there are - // resources. Load image reps for all supported scale factors immediately - // instead of in an as needed fashion to be consistent with how favicons - // are requested for chrome:// and page URLs. - const std::vector<ui::ScaleFactor>& scale_factors = - ui::GetSupportedScaleFactors(); - std::vector<ImageLoadingTracker::ImageRepresentation> info_list; - for (size_t i = 0; i < scale_factors.size(); ++i) { - float scale = ui::GetScaleFactorScale(scale_factors[i]); - int pixel_size = static_cast<int>(gfx::kFaviconSize * scale); - ExtensionResource icon_resource = - extension_->GetIconResource(pixel_size, - ExtensionIconSet::MATCH_BIGGER); - - info_list.push_back( - ImageLoadingTracker::ImageRepresentation( - icon_resource, - ImageLoadingTracker::ImageRepresentation::ALWAYS_RESIZE, - gfx::Size(pixel_size, pixel_size), - scale_factors[i])); - } - - tracker_.LoadImages(extension_, info_list, - ImageLoadingTracker::DONT_CACHE); +// Forwards the result of the request. If no favicon was available then +// |image| will be empty. Once the result has been forwarded the instance is +// deleted. +void ForwardFaviconResult(FaviconService::GetFaviconRequest* request, + const gfx::Image& image) { + std::vector<history::FaviconBitmapResult> favicon_bitmap_results; + const std::vector<gfx::ImageSkiaRep>& image_reps = + image.AsImageSkia().image_reps(); + for (size_t i = 0; i < image_reps.size(); ++i) { + const gfx::ImageSkiaRep& image_rep = image_reps[i]; + scoped_refptr<base::RefCountedBytes> bitmap_data( + new base::RefCountedBytes()); + if (gfx::PNGCodec::EncodeBGRASkBitmap(image_rep.sk_bitmap(), + false, + &bitmap_data->data())) { + history::FaviconBitmapResult bitmap_result; + bitmap_result.bitmap_data = bitmap_data; + bitmap_result.pixel_size = gfx::Size(image_rep.pixel_width(), + image_rep.pixel_height()); + // Leave |bitmap_result|'s icon URL as the default of GURL(). + bitmap_result.icon_type = history::FAVICON; + + favicon_bitmap_results.push_back(bitmap_result); } else { - ForwardResult(gfx::Image()); + NOTREACHED() << "Could not encode extension favicon"; } } - virtual void OnImageLoaded(const gfx::Image& image, - const std::string& extension_id, - int index) OVERRIDE { - ForwardResult(image); - } - - private: - ~ExtensionWebUIImageLoadingTracker() {} - - // Forwards the result of the request. If no favicon was available then - // |image| will be empty. Once the result has been forwarded the instance is - // deleted. - void ForwardResult(const gfx::Image& image) { - std::vector<history::FaviconBitmapResult> favicon_bitmap_results; - const std::vector<gfx::ImageSkiaRep>& image_reps = - image.AsImageSkia().image_reps(); - for (size_t i = 0; i < image_reps.size(); ++i) { - const gfx::ImageSkiaRep& image_rep = image_reps[i]; - scoped_refptr<base::RefCountedBytes> bitmap_data( - new base::RefCountedBytes()); - if (gfx::PNGCodec::EncodeBGRASkBitmap(image_rep.sk_bitmap(), - false, - &bitmap_data->data())) { - history::FaviconBitmapResult bitmap_result; - bitmap_result.bitmap_data = bitmap_data; - bitmap_result.pixel_size = gfx::Size(image_rep.pixel_width(), - image_rep.pixel_height()); - // Leave |bitmap_result|'s icon URL as the default of GURL(). - bitmap_result.icon_type = history::FAVICON; - - favicon_bitmap_results.push_back(bitmap_result); - } else { - NOTREACHED() << "Could not encode extension favicon"; - } - } - - // Populate IconURLSizesMap such that all the icon URLs in - // |favicon_bitmap_results| are present in |icon_url_sizes|. - // Populate the favicon sizes with the relevant pixel sizes in the - // extension's icon set. - history::IconURLSizesMap icon_url_sizes; - for (size_t i = 0; i < favicon_bitmap_results.size(); ++i) { - const history::FaviconBitmapResult& bitmap_result = - favicon_bitmap_results[i]; - const GURL& icon_url = bitmap_result.icon_url; - icon_url_sizes[icon_url].push_back(bitmap_result.pixel_size); - } - - request_->ForwardResultAsync(request_->handle(), favicon_bitmap_results, - icon_url_sizes); - delete this; + // Populate IconURLSizesMap such that all the icon URLs in + // |favicon_bitmap_results| are present in |icon_url_sizes|. + // Populate the favicon sizes with the relevant pixel sizes in the + // extension's icon set. + history::IconURLSizesMap icon_url_sizes; + for (size_t i = 0; i < favicon_bitmap_results.size(); ++i) { + const history::FaviconBitmapResult& bitmap_result = + favicon_bitmap_results[i]; + const GURL& icon_url = bitmap_result.icon_url; + icon_url_sizes[icon_url].push_back(bitmap_result.pixel_size); } - ImageLoadingTracker tracker_; - scoped_refptr<FaviconService::GetFaviconRequest> request_; - const Extension* extension_; - - DISALLOW_COPY_AND_ASSIGN(ExtensionWebUIImageLoadingTracker); -}; + request->ForwardResultAsync(request->handle(), favicon_bitmap_results, + icon_url_sizes); +} } // namespace @@ -464,8 +400,43 @@ void ExtensionWebUI::UnregisterChromeURLOverrides( // static void ExtensionWebUI::GetFaviconForURL(Profile* profile, FaviconService::GetFaviconRequest* request, const GURL& page_url) { - // tracker deletes itself when done. - ExtensionWebUIImageLoadingTracker* tracker = - new ExtensionWebUIImageLoadingTracker(profile, request, page_url); - tracker->Init(); + // Even when the extensions service is enabled by default, it's still + // disabled in incognito mode. + ExtensionService* service = profile->GetExtensionService(); + if (!service) { + ForwardFaviconResult(request, gfx::Image()); + return; + } + const Extension* extension = service->extensions()->GetByID(page_url.host()); + if (!extension) { + ForwardFaviconResult(request, gfx::Image()); + return; + } + + // Fetch resources for all supported scale factors for which there are + // resources. Load image reps for all supported scale factors immediately + // instead of in an as needed fashion to be consistent with how favicons + // are requested for chrome:// and page URLs. + const std::vector<ui::ScaleFactor>& scale_factors = + ui::GetSupportedScaleFactors(); + std::vector<extensions::ImageLoader::ImageRepresentation> info_list; + for (size_t i = 0; i < scale_factors.size(); ++i) { + float scale = ui::GetScaleFactorScale(scale_factors[i]); + int pixel_size = static_cast<int>(gfx::kFaviconSize * scale); + ExtensionResource icon_resource = + extension->GetIconResource(pixel_size, + ExtensionIconSet::MATCH_BIGGER); + + info_list.push_back( + extensions::ImageLoader::ImageRepresentation( + icon_resource, + extensions::ImageLoader::ImageRepresentation::ALWAYS_RESIZE, + gfx::Size(pixel_size, pixel_size), + scale_factors[i])); + } + + extensions::ImageLoader::Get(profile)->LoadImagesAsync( + extension, info_list, + base::Bind(&ForwardFaviconResult, + scoped_refptr<FaviconService::GetFaviconRequest>(request))); } diff --git a/chrome/browser/extensions/image_loader.cc b/chrome/browser/extensions/image_loader.cc new file mode 100644 index 0000000..8060410 --- /dev/null +++ b/chrome/browser/extensions/image_loader.cc @@ -0,0 +1,327 @@ +// Copyright (c) 2012 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/image_loader.h" + +#include "base/callback.h" +#include "base/file_util.h" +#include "base/path_service.h" +#include "base/string_number_conversions.h" +#include "base/threading/sequenced_worker_pool.h" +#include "chrome/browser/extensions/image_loader_factory.h" +#include "chrome/common/chrome_paths.h" +#include "chrome/common/extensions/extension.h" +#include "content/public/browser/browser_thread.h" +#include "grit/chrome_unscaled_resources.h" +#include "grit/component_extension_resources_map.h" +#include "grit/theme_resources.h" +#include "skia/ext/image_operations.h" +#include "ui/base/resource/resource_bundle.h" +#include "ui/gfx/image/image_skia.h" +#include "webkit/glue/image_decoder.h" + +using content::BrowserThread; +using extensions::Extension; +using extensions::ImageLoader; + +namespace { + +bool ShouldResizeImageRepresentation( + ImageLoader::ImageRepresentation::ResizeCondition resize_method, + const gfx::Size& decoded_size, + const gfx::Size& desired_size) { + switch (resize_method) { + case ImageLoader::ImageRepresentation::ALWAYS_RESIZE: + return decoded_size != desired_size; + case ImageLoader::ImageRepresentation::RESIZE_WHEN_LARGER: + return decoded_size.width() > desired_size.width() || + decoded_size.height() > desired_size.height(); + default: + NOTREACHED(); + return false; + } +} + +SkBitmap ResizeIfNeeded(const SkBitmap& bitmap, + const ImageLoader::ImageRepresentation& image_info) { + gfx::Size original_size(bitmap.width(), bitmap.height()); + if (ShouldResizeImageRepresentation(image_info.resize_condition, + original_size, + image_info.desired_size)) { + return skia::ImageOperations::Resize( + bitmap, skia::ImageOperations::RESIZE_LANCZOS3, + image_info.desired_size.width(), image_info.desired_size.height()); + } + + return bitmap; +} + +void LoadResourceOnUIThread(int resource_id, SkBitmap* bitmap) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + + gfx::ImageSkia image( + *ResourceBundle::GetSharedInstance().GetImageSkiaNamed(resource_id)); + image.MakeThreadSafe(); + *bitmap = *image.bitmap(); +} + +void LoadImageOnBlockingPool(const ImageLoader::ImageRepresentation& image_info, + SkBitmap* bitmap) { + DCHECK(BrowserThread::GetBlockingPool()->RunsTasksOnCurrentThread()); + + // Read the file from disk. + std::string file_contents; + FilePath path = image_info.resource.GetFilePath(); + if (path.empty() || !file_util::ReadFileToString(path, &file_contents)) { + return; + } + + // Decode the bitmap using WebKit's image decoder. + const unsigned char* data = + reinterpret_cast<const unsigned char*>(file_contents.data()); + webkit_glue::ImageDecoder decoder; + // Note: This class only decodes bitmaps from extension resources. Chrome + // doesn't (for security reasons) directly load extension resources provided + // by the extension author, but instead decodes them in a separate + // locked-down utility process. Only if the decoding succeeds is the image + // saved from memory to disk and subsequently used in the Chrome UI. + // Chrome is therefore decoding bitmaps here that were generated by Chrome. + *bitmap = decoder.Decode(data, file_contents.length()); +} + +} // namespace + +namespace extensions { + +//////////////////////////////////////////////////////////////////////////////// +// ImageLoader::ImageRepresentation + +ImageLoader::ImageRepresentation::ImageRepresentation( + const ExtensionResource& resource, + ResizeCondition resize_condition, + const gfx::Size& desired_size, + ui::ScaleFactor scale_factor) + : resource(resource), + resize_condition(resize_condition), + desired_size(desired_size), + scale_factor(scale_factor) { +} + +ImageLoader::ImageRepresentation::~ImageRepresentation() { +} + +//////////////////////////////////////////////////////////////////////////////// +// ImageLoader::LoadResult + +struct ImageLoader::LoadResult { + LoadResult(const SkBitmap& bitmap, + const gfx::Size& original_size, + const ImageRepresentation& image_representation); + ~LoadResult(); + + SkBitmap bitmap; + gfx::Size original_size; + ImageRepresentation image_representation; +}; + +ImageLoader::LoadResult::LoadResult( + const SkBitmap& bitmap, + const gfx::Size& original_size, + const ImageLoader::ImageRepresentation& image_representation) + : bitmap(bitmap), + original_size(original_size), + image_representation(image_representation) { +} + +ImageLoader::LoadResult::~LoadResult() { +} + +//////////////////////////////////////////////////////////////////////////////// +// ImageLoader + +ImageLoader::ImageLoader() { +} + +ImageLoader::~ImageLoader() { +} + +// static +ImageLoader* ImageLoader::Get(Profile* profile) { + return ImageLoaderFactory::GetForProfile(profile); +} + +// static +bool ImageLoader::IsComponentExtensionResource(const Extension* extension, + const FilePath& resource_path, + int* resource_id) { + static const GritResourceMap kExtraComponentExtensionResources[] = { + {"web_store/webstore_icon_128.png", IDR_WEBSTORE_ICON}, + {"web_store/webstore_icon_16.png", IDR_WEBSTORE_ICON_16}, + {"chrome_app/product_logo_128.png", IDR_PRODUCT_LOGO_128}, + {"chrome_app/product_logo_16.png", IDR_PRODUCT_LOGO_16}, + {"settings_app/settings_app_icon_128.png", IDR_SETTINGS_APP_ICON_128}, + {"settings_app/settings_app_icon_16.png", IDR_SETTINGS_APP_ICON_16}, + }; + static const size_t kExtraComponentExtensionResourcesSize = + arraysize(kExtraComponentExtensionResources); + + if (extension->location() != Extension::COMPONENT) + return false; + + FilePath directory_path = extension->path(); + FilePath resources_dir; + FilePath relative_path; + if (!PathService::Get(chrome::DIR_RESOURCES, &resources_dir) || + !resources_dir.AppendRelativePath(directory_path, &relative_path)) { + return false; + } + relative_path = relative_path.Append(resource_path); + relative_path = relative_path.NormalizePathSeparators(); + + // TODO(tc): Make a map of FilePath -> resource ids so we don't have to + // covert to FilePaths all the time. This will be more useful as we add + // more resources. + for (size_t i = 0; i < kComponentExtensionResourcesSize; ++i) { + FilePath resource_path = + FilePath().AppendASCII(kComponentExtensionResources[i].name); + resource_path = resource_path.NormalizePathSeparators(); + + if (relative_path == resource_path) { + *resource_id = kComponentExtensionResources[i].value; + return true; + } + } + for (size_t i = 0; i < kExtraComponentExtensionResourcesSize; ++i) { + FilePath resource_path = + FilePath().AppendASCII(kExtraComponentExtensionResources[i].name); + resource_path = resource_path.NormalizePathSeparators(); + + if (relative_path == resource_path) { + *resource_id = kExtraComponentExtensionResources[i].value; + return true; + } + } + return false; +} + +void ImageLoader::LoadImageAsync( + const Extension* extension, + const ExtensionResource& resource, + const gfx::Size& max_size, + const base::Callback<void(const gfx::Image&)>& callback) { + std::vector<ImageRepresentation> info_list; + info_list.push_back(ImageRepresentation( + resource, + ImageRepresentation::RESIZE_WHEN_LARGER, + max_size, + ui::SCALE_FACTOR_100P)); + LoadImagesAsync(extension, info_list, callback); +} + +void ImageLoader::LoadImagesAsync( + const Extension* extension, + const std::vector<ImageRepresentation>& info_list, + const base::Callback<void(const gfx::Image&)>& callback) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + + // Loading an image from the cache and loading resources have to happen + // on the UI thread. So do those two things first, and pass the rest of the + // work of as a blocking pool task. + + std::vector<SkBitmap> bitmaps; + bitmaps.resize(info_list.size()); + + int i = 0; + for (std::vector<ImageRepresentation>::const_iterator it = info_list.begin(); + it != info_list.end(); ++it, ++i) { + DCHECK(it->resource.relative_path().empty() || + extension->path() == it->resource.extension_root()); + + int resource_id; + if (IsComponentExtensionResource(extension, it->resource.relative_path(), + &resource_id)) { + LoadResourceOnUIThread(resource_id, &bitmaps[i]); + } + } + + DCHECK(!BrowserThread::GetBlockingPool()->RunsTasksOnCurrentThread()); + BrowserThread::PostBlockingPoolTask( + FROM_HERE, + base::Bind(&ImageLoader::LoadImagesOnBlockingPool, base::Unretained(this), + info_list, bitmaps, callback)); +} + +void ImageLoader::LoadImagesOnBlockingPool( + const std::vector<ImageRepresentation>& info_list, + const std::vector<SkBitmap>& bitmaps, + const base::Callback<void(const gfx::Image&)>& callback) { + DCHECK(BrowserThread::GetBlockingPool()->RunsTasksOnCurrentThread()); + + gfx::ImageSkia image_skia; + + std::vector<LoadResult> load_result; + + int i = 0; + for (std::vector<ImageRepresentation>::const_iterator it = info_list.begin(); + it != info_list.end(); ++it, ++i) { + // If we don't have a path there isn't anything we can do, just skip it. + if (it->resource.relative_path().empty()) + continue; + + SkBitmap bitmap; + if (!bitmaps[i].isNull()) { + bitmap = bitmaps[i]; + } else { + LoadImageOnBlockingPool(*it, &bitmap); + } + + // If the image failed to load, skip it. + if (bitmap.isNull() || bitmap.empty()) + continue; + + gfx::Size original_size(bitmap.width(), bitmap.height()); + bitmap = ResizeIfNeeded(bitmap, *it); + + load_result.push_back(LoadResult(bitmap, original_size, *it)); + } + + gfx::Image image; + + if (!image_skia.isNull()) { + image_skia.MakeThreadSafe(); + image = gfx::Image(image_skia); + } + + BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, + base::Bind(&ImageLoader::ReplyBack, + base::Unretained(this), load_result, + callback)); +} + +void ImageLoader::ReplyBack( + const std::vector<LoadResult>& load_result, + const base::Callback<void(const gfx::Image&)>& callback) { + DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); + + gfx::ImageSkia image_skia; + + for (std::vector<LoadResult>::const_iterator it = load_result.begin(); + it != load_result.end(); ++it) { + const SkBitmap& bitmap = it->bitmap; + const ImageRepresentation& image_rep = it->image_representation; + + image_skia.AddRepresentation(gfx::ImageSkiaRep( + bitmap, image_rep.scale_factor)); + } + + gfx::Image image; + if (!image_skia.isNull()) { + image_skia.MakeThreadSafe(); + image = gfx::Image(image_skia); + } + + callback.Run(image); +} + +} // namespace extensions diff --git a/chrome/browser/extensions/image_loader.h b/chrome/browser/extensions/image_loader.h new file mode 100644 index 0000000..44af106 --- /dev/null +++ b/chrome/browser/extensions/image_loader.h @@ -0,0 +1,113 @@ +// Copyright (c) 2012 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_IMAGE_LOADER_H_ +#define CHROME_BROWSER_EXTENSIONS_IMAGE_LOADER_H_ + +#include <map> +#include <set> + +#include "base/callback_forward.h" +#include "base/gtest_prod_util.h" +#include "chrome/browser/profiles/profile_keyed_service.h" +#include "chrome/common/extensions/extension_resource.h" +#include "third_party/skia/include/core/SkBitmap.h" +#include "ui/base/layout.h" +#include "ui/gfx/size.h" + +class Profile; + +namespace gfx { +class Image; +} + +namespace extensions { + +class Extension; + +// This class is responsible for asynchronously loading extension images and +// calling a callback when an image is loaded. +// The views need to load their icons asynchronously might be deleted before +// the images have loaded. If you pass your callback using a weak_ptr, this +// will make sure the callback won't be called after the view is deleted. +class ImageLoader : public ProfileKeyedService { + public: + // Information about a singe image representation to load from an extension + // resource. + struct ImageRepresentation { + // Enum values to indicate whether to resize loaded bitmap when it is larger + // than |desired_size| or always resize it. + enum ResizeCondition { + RESIZE_WHEN_LARGER, + ALWAYS_RESIZE, + }; + + ImageRepresentation(const ExtensionResource& resource, + ResizeCondition resize_condition, + const gfx::Size& desired_size, + ui::ScaleFactor scale_factor); + ~ImageRepresentation(); + + // Extension resource to load. + ExtensionResource resource; + + ResizeCondition resize_condition; + + // When |resize_method| is ALWAYS_RESIZE or when the loaded image is larger + // than |desired_size| it will be resized to these dimensions. + gfx::Size desired_size; + + // |scale_factor| is used to construct the loaded gfx::ImageSkia. + ui::ScaleFactor scale_factor; + }; + + // Returns the instance for the given profile, or NULL if none. This is + // a convenience wrapper around ImageLoaderFactory::GetForProfile. + static ImageLoader* Get(Profile* profile); + + ImageLoader(); + virtual ~ImageLoader(); + + // Checks whether image is a component extension resource. Returns false + // if a given |resource| does not have a corresponding image in bundled + // resources. Otherwise fills |resource_id|. + static bool IsComponentExtensionResource( + const extensions::Extension* extension, + const FilePath& resource_path, + int* resource_id); + + // Specify image resource to load. If the loaded image is larger than + // |max_size| it will be resized to those dimensions. IMPORTANT NOTE: this + // function may call back your callback synchronously (ie before it returns) + // if the image was found in the cache. + // Note this method loads a raw bitmap from the resource. All sizes given are + // assumed to be in pixels. + void LoadImageAsync(const extensions::Extension* extension, + const ExtensionResource& resource, + const gfx::Size& max_size, + const base::Callback<void(const gfx::Image&)>& callback); + + // Same as LoadImage() above except it loads multiple images from the same + // extension. This is used to load multiple resolutions of the same image + // type. + void LoadImagesAsync(const extensions::Extension* extension, + const std::vector<ImageRepresentation>& info_list, + const base::Callback<void(const gfx::Image&)>& callback); + + private: + struct LoadResult; + + void LoadImagesOnBlockingPool( + const std::vector<ImageRepresentation>& info_list, + const std::vector<SkBitmap>& bitmaps, + const base::Callback<void(const gfx::Image&)>& callback); + + void ReplyBack( + const std::vector<LoadResult>& load_result, + const base::Callback<void(const gfx::Image&)>& callback); +}; + +} // namespace extensions + +#endif // CHROME_BROWSER_EXTENSIONS_IMAGE_LOADER_H_ diff --git a/chrome/browser/extensions/image_loader_factory.cc b/chrome/browser/extensions/image_loader_factory.cc new file mode 100644 index 0000000..21ad9ad --- /dev/null +++ b/chrome/browser/extensions/image_loader_factory.cc @@ -0,0 +1,50 @@ +// Copyright (c) 2012 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/image_loader_factory.h" + +#include "chrome/browser/extensions/image_loader.h" +#include "chrome/browser/profiles/profile_dependency_manager.h" + +namespace extensions { + +// static +ImageLoader* ImageLoaderFactory::GetForProfile(Profile* profile) { + return static_cast<ImageLoader*>( + GetInstance()->GetServiceForProfile(profile, true)); +} + +// static +void ImageLoaderFactory::ResetForProfile(Profile* profile) { + ImageLoaderFactory* factory = GetInstance(); + factory->ProfileShutdown(profile); + factory->ProfileDestroyed(profile); +} + +ImageLoaderFactory* ImageLoaderFactory::GetInstance() { + return Singleton<ImageLoaderFactory>::get(); +} + +ImageLoaderFactory::ImageLoaderFactory() + : ProfileKeyedServiceFactory("ImageLoader", + ProfileDependencyManager::GetInstance()) { +} + +ImageLoaderFactory::~ImageLoaderFactory() { +} + +ProfileKeyedService* ImageLoaderFactory::BuildServiceInstanceFor( + Profile* profile) const { + return new ImageLoader; +} + +bool ImageLoaderFactory::ServiceIsCreatedWithProfile() const { + return false; +} + +bool ImageLoaderFactory::ServiceRedirectedInIncognito() const { + return true; +} + +} // namespace extensions diff --git a/chrome/browser/extensions/image_loader_factory.h b/chrome/browser/extensions/image_loader_factory.h new file mode 100644 index 0000000..767316b --- /dev/null +++ b/chrome/browser/extensions/image_loader_factory.h @@ -0,0 +1,43 @@ +// Copyright (c) 2012 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_IMAGE_LOADER_FACTORY_H_ +#define CHROME_BROWSER_EXTENSIONS_IMAGE_LOADER_FACTORY_H_ + +#include "base/memory/singleton.h" +#include "chrome/browser/profiles/profile_keyed_service_factory.h" + +class Profile; + +namespace extensions { + +class ImageLoader; + +// Singleton that owns all ImageLoaders and associates them with +// Profiles. Listens for the Profile's destruction notification and cleans up +// the associated ImageLoader. +class ImageLoaderFactory : public ProfileKeyedServiceFactory { + public: + static ImageLoader* GetForProfile(Profile* profile); + + static void ResetForProfile(Profile* profile); + + static ImageLoaderFactory* GetInstance(); + + private: + friend struct DefaultSingletonTraits<ImageLoaderFactory>; + + ImageLoaderFactory(); + virtual ~ImageLoaderFactory(); + + // ProfileKeyedServiceFactory: + virtual ProfileKeyedService* BuildServiceInstanceFor( + Profile* profile) const OVERRIDE; + virtual bool ServiceIsCreatedWithProfile() const OVERRIDE; + virtual bool ServiceRedirectedInIncognito() const OVERRIDE; +}; + +} // namespace extensions + +#endif // CHROME_BROWSER_EXTENSIONS_IMAGE_LOADER_FACTORY_H_ diff --git a/chrome/browser/extensions/image_loader_unittest.cc b/chrome/browser/extensions/image_loader_unittest.cc new file mode 100644 index 0000000..8b0b483 --- /dev/null +++ b/chrome/browser/extensions/image_loader_unittest.cc @@ -0,0 +1,252 @@ +// Copyright (c) 2012 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/image_loader.h" + +#include "base/json/json_file_value_serializer.h" +#include "base/message_loop.h" +#include "base/path_service.h" +#include "chrome/common/chrome_notification_types.h" +#include "chrome/common/chrome_paths.h" +#include "chrome/common/extensions/extension.h" +#include "chrome/common/extensions/extension_constants.h" +#include "chrome/common/extensions/extension_icon_set.h" +#include "chrome/common/extensions/extension_resource.h" +#include "content/public/browser/notification_service.h" +#include "content/public/test/test_browser_thread.h" +#include "grit/component_extension_resources.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "third_party/skia/include/core/SkBitmap.h" +#include "ui/gfx/image/image.h" +#include "ui/gfx/image/image_skia.h" +#include "ui/gfx/size.h" + +using content::BrowserThread; +using extensions::Extension; +using extensions::ImageLoader; + +class ImageLoaderTest : public testing::Test { + public: + ImageLoaderTest() + : image_loaded_count_(0), + quit_in_image_loaded_(false), + ui_thread_(BrowserThread::UI, &ui_loop_), + file_thread_(BrowserThread::FILE), + io_thread_(BrowserThread::IO) { + } + + void OnImageLoaded(const gfx::Image& image) { + image_loaded_count_++; + if (quit_in_image_loaded_) + MessageLoop::current()->Quit(); + image_ = image; + } + + void WaitForImageLoad() { + quit_in_image_loaded_ = true; + MessageLoop::current()->Run(); + quit_in_image_loaded_ = false; + } + + int image_loaded_count() { + int result = image_loaded_count_; + image_loaded_count_ = 0; + return result; + } + + scoped_refptr<Extension> CreateExtension(const char* name, + Extension::Location location) { + // Create and load an extension. + FilePath test_file; + if (!PathService::Get(chrome::DIR_TEST_DATA, &test_file)) { + EXPECT_FALSE(true); + return NULL; + } + test_file = test_file.AppendASCII("extensions") + .AppendASCII(name); + int error_code = 0; + std::string error; + JSONFileValueSerializer serializer(test_file.AppendASCII("app.json")); + scoped_ptr<DictionaryValue> valid_value( + static_cast<DictionaryValue*>(serializer.Deserialize(&error_code, + &error))); + EXPECT_EQ(0, error_code) << error; + if (error_code != 0) + return NULL; + + EXPECT_TRUE(valid_value.get()); + if (!valid_value.get()) + return NULL; + + if (location == Extension::COMPONENT) { + if (!PathService::Get(chrome::DIR_RESOURCES, &test_file)) { + EXPECT_FALSE(true); + return NULL; + } + test_file = test_file.AppendASCII(name); + } + return Extension::Create(test_file, location, *valid_value, + Extension::NO_FLAGS, &error); + } + + gfx::Image image_; + + private: + virtual void SetUp() { + file_thread_.Start(); + io_thread_.Start(); + } + + int image_loaded_count_; + bool quit_in_image_loaded_; + MessageLoop ui_loop_; + content::TestBrowserThread ui_thread_; + content::TestBrowserThread file_thread_; + content::TestBrowserThread io_thread_; +}; + +// Tests loading an image works correctly. +TEST_F(ImageLoaderTest, LoadImage) { + scoped_refptr<Extension> extension(CreateExtension( + "image_loading_tracker", Extension::INVALID)); + ASSERT_TRUE(extension.get() != NULL); + + ExtensionResource image_resource = + extension->GetIconResource(extension_misc::EXTENSION_ICON_SMALLISH, + ExtensionIconSet::MATCH_EXACTLY); + gfx::Size max_size(extension_misc::EXTENSION_ICON_SMALLISH, + extension_misc::EXTENSION_ICON_SMALLISH); + ImageLoader loader; + loader.LoadImageAsync(extension.get(), + image_resource, + max_size, + base::Bind(&ImageLoaderTest::OnImageLoaded, + base::Unretained(this))); + + // The image isn't cached, so we should not have received notification. + EXPECT_EQ(0, image_loaded_count()); + + WaitForImageLoad(); + + // We should have gotten the image. + EXPECT_EQ(1, image_loaded_count()); + + // Check that the image was loaded. + EXPECT_EQ(extension_misc::EXTENSION_ICON_SMALLISH, + image_.ToSkBitmap()->width()); +} + +// Tests deleting an extension while waiting for the image to load doesn't cause +// problems. +TEST_F(ImageLoaderTest, DeleteExtensionWhileWaitingForCache) { + scoped_refptr<Extension> extension(CreateExtension( + "image_loading_tracker", Extension::INVALID)); + ASSERT_TRUE(extension.get() != NULL); + + ExtensionResource image_resource = + extension->GetIconResource(extension_misc::EXTENSION_ICON_SMALLISH, + ExtensionIconSet::MATCH_EXACTLY); + gfx::Size max_size(extension_misc::EXTENSION_ICON_SMALLISH, + extension_misc::EXTENSION_ICON_SMALLISH); + ImageLoader loader; + std::set<int> sizes; + sizes.insert(extension_misc::EXTENSION_ICON_SMALLISH); + loader.LoadImageAsync(extension.get(), + image_resource, + max_size, + base::Bind(&ImageLoaderTest::OnImageLoaded, + base::Unretained(this))); + + // The image isn't cached, so we should not have received notification. + EXPECT_EQ(0, image_loaded_count()); + + // Send out notification the extension was uninstalled. + extensions::UnloadedExtensionInfo details(extension.get(), + extension_misc::UNLOAD_REASON_UNINSTALL); + content::NotificationService::current()->Notify( + chrome::NOTIFICATION_EXTENSION_UNLOADED, + content::NotificationService::AllSources(), + content::Details<extensions::UnloadedExtensionInfo>(&details)); + + // Chuck the extension, that way if anyone tries to access it we should crash + // or get valgrind errors. + extension = NULL; + + WaitForImageLoad(); + + // Even though we deleted the extension, we should still get the image. + // We should still have gotten the image. + EXPECT_EQ(1, image_loaded_count()); + + // Check that the image was loaded. + EXPECT_EQ(extension_misc::EXTENSION_ICON_SMALLISH, + image_.ToSkBitmap()->width()); +} + +// Tests loading multiple dimensions of the same image. +TEST_F(ImageLoaderTest, MultipleImages) { + scoped_refptr<Extension> extension(CreateExtension( + "image_loading_tracker", Extension::INVALID)); + ASSERT_TRUE(extension.get() != NULL); + + std::vector<ImageLoader::ImageRepresentation> info_list; + int sizes[] = {extension_misc::EXTENSION_ICON_SMALLISH, + extension_misc::EXTENSION_ICON_BITTY}; + for (size_t i = 0; i < arraysize(sizes); ++i) { + ExtensionResource resource = + extension->GetIconResource(sizes[i], ExtensionIconSet::MATCH_EXACTLY); + info_list.push_back(ImageLoader::ImageRepresentation( + resource, + ImageLoader::ImageRepresentation::RESIZE_WHEN_LARGER, + gfx::Size(sizes[i], sizes[i]), + ui::SCALE_FACTOR_NONE)); + } + + ImageLoader loader; + loader.LoadImagesAsync(extension.get(), info_list, + base::Bind(&ImageLoaderTest::OnImageLoaded, + base::Unretained(this))); + + // The image isn't cached, so we should not have received notification. + EXPECT_EQ(0, image_loaded_count()); + + WaitForImageLoad(); + + // We should have gotten the image. + EXPECT_EQ(1, image_loaded_count()); + + // Check that all images were loaded. + std::vector<gfx::ImageSkiaRep> image_reps = + image_.ToImageSkia()->image_reps(); + ASSERT_EQ(2u, image_reps.size()); + const gfx::ImageSkiaRep* img_rep1 = &image_reps[0]; + const gfx::ImageSkiaRep* img_rep2 = &image_reps[1]; + if (img_rep1->pixel_width() > img_rep2->pixel_width()) { + std::swap(img_rep1, img_rep2); + } + EXPECT_EQ(extension_misc::EXTENSION_ICON_BITTY, + img_rep1->pixel_width()); + EXPECT_EQ(extension_misc::EXTENSION_ICON_SMALLISH, + img_rep2->pixel_width()); +} + +// Tests IsComponentExtensionResource function. +TEST_F(ImageLoaderTest, IsComponentExtensionResource) { + scoped_refptr<Extension> extension(CreateExtension( + "file_manager", Extension::COMPONENT)); + ASSERT_TRUE(extension.get() != NULL); + + ExtensionResource resource = + extension->GetIconResource(extension_misc::EXTENSION_ICON_BITTY, + ExtensionIconSet::MATCH_EXACTLY); + +#if defined(FILE_MANAGER_EXTENSION) + int resource_id; + ASSERT_EQ(true, + ImageLoader::IsComponentExtensionResource(extension.get(), + resource.relative_path(), + &resource_id)); + ASSERT_EQ(IDR_FILE_MANAGER_ICON_16, resource_id); +#endif +} diff --git a/chrome/browser/extensions/image_loading_tracker.cc b/chrome/browser/extensions/image_loading_tracker.cc index 7dc4773..cfd9209 100644 --- a/chrome/browser/extensions/image_loading_tracker.cc +++ b/chrome/browser/extensions/image_loading_tracker.cc @@ -9,20 +9,16 @@ #include "base/bind.h" #include "base/file_util.h" -#include "base/path_service.h" #include "base/threading/sequenced_worker_pool.h" +#include "chrome/browser/extensions/image_loader.h" #include "chrome/browser/ui/webui/extensions/extension_icon_source.h" #include "chrome/common/chrome_notification_types.h" -#include "chrome/common/chrome_paths.h" #include "chrome/common/extensions/extension.h" #include "chrome/common/extensions/extension_constants.h" #include "chrome/common/extensions/extension_file_util.h" #include "chrome/common/extensions/extension_resource.h" #include "content/public/browser/browser_thread.h" #include "content/public/browser/notification_service.h" -#include "grit/component_extension_resources_map.h" -#include "grit/chrome_unscaled_resources.h" -#include "grit/theme_resources.h" #include "skia/ext/image_operations.h" #include "third_party/skia/include/core/SkBitmap.h" #include "ui/base/resource/resource_bundle.h" @@ -309,68 +305,14 @@ void ImageLoadingTracker::LoadImages( loader_ = new ImageLoader(this); int resource_id = -1; - if (IsComponentExtensionResource(extension, it->resource.relative_path(), - &resource_id)) + if (extensions::ImageLoader::IsComponentExtensionResource( + extension, it->resource.relative_path(), &resource_id)) loader_->LoadResource(*it, id, resource_id); else loader_->LoadImage(*it, id); } } -bool ImageLoadingTracker::IsComponentExtensionResource( - const Extension* extension, - const FilePath& resource_path, - int* resource_id) { - static const GritResourceMap kExtraComponentExtensionResources[] = { - {"web_store/webstore_icon_128.png", IDR_WEBSTORE_ICON}, - {"web_store/webstore_icon_16.png", IDR_WEBSTORE_ICON_16}, - {"chrome_app/product_logo_128.png", IDR_PRODUCT_LOGO_128}, - {"chrome_app/product_logo_16.png", IDR_PRODUCT_LOGO_16}, - {"settings_app/settings_app_icon_128.png", IDR_SETTINGS_APP_ICON_128}, - {"settings_app/settings_app_icon_16.png", IDR_SETTINGS_APP_ICON_16}, - }; - static const size_t kExtraComponentExtensionResourcesSize = - arraysize(kExtraComponentExtensionResources); - - if (extension->location() != Extension::COMPONENT) - return false; - - FilePath directory_path = extension->path(); - FilePath resources_dir; - FilePath relative_path; - if (!PathService::Get(chrome::DIR_RESOURCES, &resources_dir) || - !resources_dir.AppendRelativePath(directory_path, &relative_path)) { - return false; - } - relative_path = relative_path.Append(resource_path); - relative_path = relative_path.NormalizePathSeparators(); - - // TODO(tc): Make a map of FilePath -> resource ids so we don't have to - // covert to FilePaths all the time. This will be more useful as we add - // more resources. - for (size_t i = 0; i < kComponentExtensionResourcesSize; ++i) { - FilePath resource_path = - FilePath().AppendASCII(kComponentExtensionResources[i].name); - resource_path = resource_path.NormalizePathSeparators(); - - if (relative_path == resource_path) { - *resource_id = kComponentExtensionResources[i].value; - return true; - } - } - for (size_t i = 0; i < kExtraComponentExtensionResourcesSize; ++i) { - FilePath resource_path = - FilePath().AppendASCII(kExtraComponentExtensionResources[i].name); - resource_path = resource_path.NormalizePathSeparators(); - - if (relative_path == resource_path) { - *resource_id = kExtraComponentExtensionResources[i].value; - return true; - } - } - return false; -} - void ImageLoadingTracker::OnBitmapLoaded( const SkBitmap* bitmap, const ImageRepresentation& image_info, diff --git a/chrome/browser/extensions/image_loading_tracker.h b/chrome/browser/extensions/image_loading_tracker.h index 5eefcd1..f820cda 100644 --- a/chrome/browser/extensions/image_loading_tracker.h +++ b/chrome/browser/extensions/image_loading_tracker.h @@ -122,14 +122,6 @@ class ImageLoadingTracker : public content::NotificationObserver { // OnImageLoaded() the next time LoadImage() is invoked. int next_id() const { return next_id_; } - // Checks whether image is a component extension resource. Returns false - // if a given |resource| does not have a corresponding image in bundled - // resources. Otherwise fills |resource_id|. - static bool IsComponentExtensionResource( - const extensions::Extension* extension, - const FilePath& resource_path, - int* resource_id); - private: // Information for pending resource load operation for one or more image // representations. diff --git a/chrome/browser/extensions/image_loading_tracker_unittest.cc b/chrome/browser/extensions/image_loading_tracker_unittest.cc index 220c5c0..5754d94 100644 --- a/chrome/browser/extensions/image_loading_tracker_unittest.cc +++ b/chrome/browser/extensions/image_loading_tracker_unittest.cc @@ -244,24 +244,3 @@ TEST_F(ImageLoadingTrackerTest, MultipleImages) { EXPECT_EQ(extension_misc::EXTENSION_ICON_SMALLISH, img_rep2->pixel_width()); } - -// Tests IsComponentExtensionResource function. -TEST_F(ImageLoadingTrackerTest, IsComponentExtensionResource) { - scoped_refptr<Extension> extension(CreateExtension( - "file_manager", Extension::COMPONENT)); - ASSERT_TRUE(extension.get() != NULL); - - ExtensionResource resource = - extension->GetIconResource(extension_misc::EXTENSION_ICON_BITTY, - ExtensionIconSet::MATCH_EXACTLY); - -#if defined(FILE_MANAGER_EXTENSION) - ImageLoadingTracker loader(this); - int resource_id; - ASSERT_EQ(true, - loader.IsComponentExtensionResource(extension.get(), - resource.relative_path(), - &resource_id)); - ASSERT_EQ(IDR_FILE_MANAGER_ICON_16, resource_id); -#endif -} diff --git a/chrome/browser/extensions/navigation_observer.h b/chrome/browser/extensions/navigation_observer.h index 480f983..acf78ad 100644 --- a/chrome/browser/extensions/navigation_observer.h +++ b/chrome/browser/extensions/navigation_observer.h @@ -10,6 +10,7 @@ #include "base/memory/scoped_ptr.h" #include "chrome/browser/extensions/extension_install_prompt.h" +#include "content/public/browser/notification_observer.h" #include "content/public/browser/notification_registrar.h" class Profile; diff --git a/chrome/browser/media/media_stream_capture_indicator.cc b/chrome/browser/media/media_stream_capture_indicator.cc index 244235c..87266ac 100644 --- a/chrome/browser/media/media_stream_capture_indicator.cc +++ b/chrome/browser/media/media_stream_capture_indicator.cc @@ -11,6 +11,7 @@ #include "chrome/app/chrome_command_ids.h" #include "chrome/browser/browser_process.h" #include "chrome/browser/extensions/extension_service.h" +#include "chrome/browser/extensions/image_loader.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/status_icons/status_icon.h" #include "chrome/browser/status_icons/status_tray.h" @@ -125,7 +126,7 @@ MediaStreamCaptureIndicator::MediaStreamCaptureIndicator() mic_image_(NULL), camera_image_(NULL), balloon_image_(NULL), - request_index_(0) { + should_show_balloon_(false) { } MediaStreamCaptureIndicator::~MediaStreamCaptureIndicator() { @@ -228,7 +229,6 @@ void MediaStreamCaptureIndicator::CreateStatusTray() { status_icon_ = status_tray->CreateStatusIcon(); EnsureStatusTrayIconResources(); - EnsureImageLoadingTracker(); } void MediaStreamCaptureIndicator::EnsureStatusTrayIconResources() { @@ -267,14 +267,23 @@ void MediaStreamCaptureIndicator::ShowBalloon( const extensions::Extension* extension = GetExtension(render_process_id, render_view_id); if (extension) { - pending_messages_[request_index_++] = + string16 message = l10n_util::GetStringFUTF16(message_id, UTF8ToUTF16(extension->name())); - tracker_->LoadImage( + + WebContents* web_contents = tab_util::GetWebContentsByID( + render_process_id, render_view_id); + + Profile* profile = + Profile::FromBrowserContext(web_contents->GetBrowserContext()); + + should_show_balloon_ = true; + extensions::ImageLoader::Get(profile)->LoadImageAsync( extension, extension->GetIconResource(32, ExtensionIconSet::MATCH_BIGGER), gfx::Size(32, 32), - ImageLoadingTracker::CACHE); + base::Bind(&MediaStreamCaptureIndicator::OnImageLoaded, + this, message)); return; } @@ -285,12 +294,10 @@ void MediaStreamCaptureIndicator::ShowBalloon( } void MediaStreamCaptureIndicator::OnImageLoaded( - const gfx::Image& image, - const std::string& extension_id, - int index) { - string16 message; - message.swap(pending_messages_[index]); - pending_messages_.erase(index); + const string16& message, + const gfx::Image& image) { + if (!should_show_balloon_) + return; const gfx::ImageSkia* image_skia = !image.IsEmpty() ? image.ToImageSkia() : ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed( @@ -302,8 +309,8 @@ void MediaStreamCaptureIndicator::Hide() { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); DCHECK(tabs_.empty()); - // We have to destroy |tracker_| on the UI thread. - tracker_.reset(); + // Make sure images that finish loading don't cause a balloon to be shown. + should_show_balloon_ = false; if (!status_icon_) return; @@ -468,13 +475,3 @@ bool MediaStreamCaptureIndicator::IsProcessCapturing(int render_process_id, return false; return (iter->audio_ref_count > 0 || iter->video_ref_count > 0); } - -void MediaStreamCaptureIndicator::EnsureImageLoadingTracker() { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - if (tracker_.get()) - return; - - tracker_.reset(new ImageLoadingTracker(this)); - pending_messages_.clear(); - request_index_ = 0; -} diff --git a/chrome/browser/media/media_stream_capture_indicator.h b/chrome/browser/media/media_stream_capture_indicator.h index 6b6becc..8696f91 100644 --- a/chrome/browser/media/media_stream_capture_indicator.h +++ b/chrome/browser/media/media_stream_capture_indicator.h @@ -10,7 +10,6 @@ #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" -#include "chrome/browser/extensions/image_loading_tracker.h" #include "content/public/common/media_stream_request.h" #include "ui/base/models/simple_menu_model.h" #include "ui/gfx/image/image_skia.h" @@ -22,8 +21,7 @@ class StatusTray; // is deleted. class MediaStreamCaptureIndicator : public base::RefCountedThreadSafe<MediaStreamCaptureIndicator>, - public ui::SimpleMenuModel::Delegate, - public ImageLoadingTracker::Observer { + public ui::SimpleMenuModel::Delegate { public: MediaStreamCaptureIndicator(); @@ -48,10 +46,8 @@ class MediaStreamCaptureIndicator // Returns true if the render process is capturing media. bool IsProcessCapturing(int render_process_id, int render_view_id) const; - // ImageLoadingTracker::Observer implementation. - virtual void OnImageLoaded(const gfx::Image& image, - const std::string& extension_id, - int index) OVERRIDE; + // ImageLoader callback. + void OnImageLoaded(const string16& message, const gfx::Image& image); private: // Struct to store the usage information of the capture devices for each tab. @@ -131,9 +127,6 @@ class MediaStreamCaptureIndicator // UpdateStatusTrayIconContextMenu(). void UpdateStatusTrayIconDisplay(bool audio, bool video); - // Initializes image loading state. - void EnsureImageLoadingTracker(); - // Reference to our status icon - owned by the StatusTray. If null, // the platform doesn't support status icons. StatusIcon* status_icon_; @@ -147,13 +140,7 @@ class MediaStreamCaptureIndicator typedef std::vector<CaptureDeviceTab> CaptureDeviceTabs; CaptureDeviceTabs tabs_; - // Tracks the load of extension icons. - scoped_ptr<ImageLoadingTracker> tracker_; - // The messages to display when extension images are loaded. The index - // corresponds to the index of the associated LoadImage request. - std::map<int, string16> pending_messages_; - // Tracks the number of requests to |tracker_|. - int request_index_; + bool should_show_balloon_; }; #endif // CHROME_BROWSER_MEDIA_MEDIA_STREAM_CAPTURE_INDICATOR_H_ diff --git a/chrome/browser/ui/webui/extensions/extension_icon_source.cc b/chrome/browser/ui/webui/extensions/extension_icon_source.cc index d2704717..78f360d 100644 --- a/chrome/browser/ui/webui/extensions/extension_icon_source.cc +++ b/chrome/browser/ui/webui/extensions/extension_icon_source.cc @@ -15,6 +15,7 @@ #include "base/threading/thread.h" #include "chrome/browser/extensions/extension_prefs.h" #include "chrome/browser/extensions/extension_service.h" +#include "chrome/browser/extensions/image_loader.h" #include "chrome/browser/favicon/favicon_service_factory.h" #include "chrome/browser/profiles/profile.h" #include "chrome/common/extensions/extension.h" @@ -57,9 +58,7 @@ SkBitmap* ToBitmap(const unsigned char* data, size_t size) { ExtensionIconSource::ExtensionIconSource(Profile* profile) : DataSource(chrome::kChromeUIExtensionIconHost, MessageLoop::current()), - profile_(profile), - next_tracker_id_(0) { - tracker_.reset(new ImageLoadingTracker(this)); + profile_(profile) { } struct ExtensionIconSource::ExtensionIconRequest { @@ -188,11 +187,10 @@ void ExtensionIconSource::LoadDefaultImage(int request_id) { void ExtensionIconSource::LoadExtensionImage(const ExtensionResource& icon, int request_id) { ExtensionIconRequest* request = GetData(request_id); - tracker_map_[next_tracker_id_++] = request_id; - tracker_->LoadImage(request->extension, - icon, - gfx::Size(request->size, request->size), - ImageLoadingTracker::DONT_CACHE); + extensions::ImageLoader::Get(profile_)->LoadImageAsync( + request->extension, icon, + gfx::Size(request->size, request->size), + base::Bind(&ExtensionIconSource::OnImageLoaded, this, request_id)); } void ExtensionIconSource::LoadFaviconImage(int request_id) { @@ -243,12 +241,8 @@ void ExtensionIconSource::OnFaviconDataAvailable( } } -void ExtensionIconSource::OnImageLoaded(const gfx::Image& image, - const std::string& extension_id, - int index) { - int request_id = tracker_map_[index]; - tracker_map_.erase(tracker_map_.find(index)); - +void ExtensionIconSource::OnImageLoaded(int request_id, + const gfx::Image& image) { if (image.IsEmpty()) LoadIconFailed(request_id); else diff --git a/chrome/browser/ui/webui/extensions/extension_icon_source.h b/chrome/browser/ui/webui/extensions/extension_icon_source.h index f5f445e..1a05224 100644 --- a/chrome/browser/ui/webui/extensions/extension_icon_source.h +++ b/chrome/browser/ui/webui/extensions/extension_icon_source.h @@ -9,10 +9,10 @@ #include <string> #include "base/basictypes.h" -#include "chrome/browser/extensions/image_loading_tracker.h" #include "chrome/browser/favicon/favicon_service.h" #include "chrome/browser/ui/webui/chrome_url_data_manager.h" #include "chrome/common/extensions/extension_icon_set.h" +#include "chrome/common/extensions/extension_resource.h" #include "third_party/skia/include/core/SkBitmap.h" class ExtensionIconSet; @@ -48,8 +48,7 @@ class Extension; // 2) If a 16px icon was requested, the favicon for extension's launch URL. // 3) The default extension / application icon if there are still no matches. // -class ExtensionIconSource : public ChromeURLDataManager::DataSource, - public ImageLoadingTracker::Observer { +class ExtensionIconSource : public ChromeURLDataManager::DataSource { public: explicit ExtensionIconSource(Profile* profile); @@ -109,10 +108,8 @@ class ExtensionIconSource : public ChromeURLDataManager::DataSource, FaviconService::Handle request_handle, const history::FaviconBitmapResult& bitmap_result); - // ImageLoadingTracker::Observer - virtual void OnImageLoaded(const gfx::Image& image, - const std::string& extension_id, - int id) OVERRIDE; + // ImageLoader callback + void OnImageLoaded(int request_id, const gfx::Image& image); // Called when the extension doesn't have an icon. We fall back to multiple // sources, using the following order: @@ -151,10 +148,6 @@ class ExtensionIconSource : public ChromeURLDataManager::DataSource, // Maps request_ids to ExtensionIconRequests. std::map<int, ExtensionIconRequest*> request_map_; - scoped_ptr<ImageLoadingTracker> tracker_; - - int next_tracker_id_; - scoped_ptr<SkBitmap> default_app_data_; scoped_ptr<SkBitmap> default_extension_data_; diff --git a/chrome/chrome_browser_extensions.gypi b/chrome/chrome_browser_extensions.gypi index f48d260..62a398e0 100644 --- a/chrome/chrome_browser_extensions.gypi +++ b/chrome/chrome_browser_extensions.gypi @@ -532,6 +532,10 @@ 'browser/extensions/external_registry_loader_win.h', 'browser/extensions/file_reader.cc', 'browser/extensions/file_reader.h', + 'browser/extensions/image_loader.cc', + 'browser/extensions/image_loader.h', + 'browser/extensions/image_loader_factory.cc', + 'browser/extensions/image_loader_factory.h', 'browser/extensions/image_loading_tracker.cc', 'browser/extensions/image_loading_tracker.h', 'browser/extensions/installed_loader.cc', diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi index 46f9be2..8fd1746 100644 --- a/chrome/chrome_tests_unit.gypi +++ b/chrome/chrome_tests_unit.gypi @@ -730,6 +730,7 @@ 'browser/extensions/page_action_controller_unittest.cc', 'browser/extensions/permissions_updater_unittest.cc', 'browser/extensions/file_reader_unittest.cc', + 'browser/extensions/image_loader_unittest.cc', 'browser/extensions/image_loading_tracker_unittest.cc', 'browser/extensions/key_identifier_conversion_views_unittest.cc', 'browser/extensions/management_policy_unittest.cc', |