diff --git a/chrome/browser/task_management/providers/web_contents/extension_tag_browsertest.cc b/chrome/browser/task_management/providers/web_contents/extension_tag_browsertest.cc index 969349b931190..6b38a76c42e52 100644 --- a/chrome/browser/task_management/providers/web_contents/extension_tag_browsertest.cc +++ b/chrome/browser/task_management/providers/web_contents/extension_tag_browsertest.cc @@ -6,6 +6,10 @@ #include "chrome/browser/extensions/extension_browsertest.h" #include "chrome/browser/task_management/task_management_browsertest_util.h" #include "chrome/common/chrome_switches.h" +#include "extensions/browser/test_image_loader.h" +#include "extensions/common/constants.h" +#include "ui/gfx/image/image.h" +#include "ui/gfx/skia_util.h" namespace task_management { @@ -56,7 +60,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionTagsTest, Basic) { } #if defined(OS_WIN) -// Test disabled due to flakiness on Windows. +// Test disabled due to flakiness on Windows XP. // See bug: http://crbug.com/519333 #define MAYBE_PreAndPostExistingTaskProviding \ DISABLED_PreAndPostExistingTaskProviding @@ -80,16 +84,30 @@ IN_PROC_BROWSER_TEST_F(ExtensionTagsTest, EXPECT_EQ(2U, tracked_tags().size()); EXPECT_TRUE(task_manager.tasks().empty()); + base::RunLoop run_loop; + run_loop.RunUntilIdle(); + // Start observing, pre-existing tasks will be provided. task_manager.StartObserving(); - EXPECT_EQ(2U, task_manager.tasks().size()); - EXPECT_EQ(Task::EXTENSION, task_manager.tasks().back()->GetType()); + ASSERT_EQ(2U, task_manager.tasks().size()); + const Task* extension_task = task_manager.tasks().back(); + EXPECT_EQ(Task::EXTENSION, extension_task->GetType()); + + SkBitmap expected_bitmap = + extensions::TestImageLoader::LoadAndGetExtensionBitmap( + extension, + "icon_128.png", + extension_misc::EXTENSION_ICON_SMALL); + ASSERT_FALSE(expected_bitmap.empty()); + + EXPECT_TRUE(gfx::BitmapsAreEqual(*extension_task->icon().bitmap(), + expected_bitmap)); // Unload the extension and expect that the task manager now shows only the // about:blank tab. UnloadExtension(extension->id()); EXPECT_EQ(1U, tracked_tags().size()); - EXPECT_EQ(1U, task_manager.tasks().size()); + ASSERT_EQ(1U, task_manager.tasks().size()); const Task* about_blank_task = task_manager.tasks().back(); EXPECT_EQ(Task::RENDERER, about_blank_task->GetType()); EXPECT_EQ(base::UTF8ToUTF16("Tab: about:blank"), about_blank_task->title()); diff --git a/chrome/browser/task_management/providers/web_contents/extension_task.cc b/chrome/browser/task_management/providers/web_contents/extension_task.cc index a63781a20de87..9df1366560d86 100644 --- a/chrome/browser/task_management/providers/web_contents/extension_task.cc +++ b/chrome/browser/task_management/providers/web_contents/extension_task.cc @@ -8,7 +8,9 @@ #include "content/public/browser/browser_context.h" #include "content/public/browser/web_contents.h" #include "extensions/browser/view_type_utils.h" +#include "extensions/common/constants.h" #include "extensions/common/extension.h" +#include "extensions/common/manifest_handlers/icons_handler.h" #include "extensions/common/view_type.h" #include "grit/theme_resources.h" #include "ui/base/resource/resource_bundle.h" @@ -40,6 +42,7 @@ ExtensionTask::ExtensionTask(content::WebContents* web_contents, GetDefaultIcon(), web_contents, web_contents->GetRenderProcessHost()) { + LoadExtensionIcon(extension); } ExtensionTask::~ExtensionTask() { @@ -51,15 +54,21 @@ void ExtensionTask::OnTitleChanged(content::NavigationEntry* entry) { } void ExtensionTask::OnFaviconChanged() { - // For now we never change the favicon of the extension, we always use the - // default one. - // TODO(afakhry): In the future use the extensions' favicons. + // We don't care about the favicon of the WebContents but rather of the + // extension. } Task::Type ExtensionTask::GetType() const { return Task::EXTENSION; } +void ExtensionTask::OnExtensionIconImageChanged(extensions::IconImage* image) { + DCHECK_EQ(extension_icon_.get(), image); + + if (!image->image_skia().isNull()) + set_icon(image->image_skia()); +} + base::string16 ExtensionTask::GetExtensionTitle( content::WebContents* web_contents, const extensions::Extension* extension, @@ -75,10 +84,27 @@ base::string16 ExtensionTask::GetExtensionTitle( return RendererTask::PrefixRendererTitle( title, - extension->is_app(), + extension && extension->is_app(), true, // is_extension web_contents->GetBrowserContext()->IsOffTheRecord(), is_background); } +void ExtensionTask::LoadExtensionIcon(const extensions::Extension* extension) { + if (!extension) + return; + + extension_icon_.reset( + new extensions::IconImage(web_contents()->GetBrowserContext(), + extension, + extensions::IconsInfo::GetIcons(extension), + extension_misc::EXTENSION_ICON_SMALL, + icon(), + this)); + + // Triggers actual image loading with 1x resources. + extension_icon_->image_skia().GetRepresentation(1.0f); + set_icon(extension_icon_->image_skia()); +} + } // namespace task_management diff --git a/chrome/browser/task_management/providers/web_contents/extension_task.h b/chrome/browser/task_management/providers/web_contents/extension_task.h index 9ab3ecc0abd16..83c2d2b82b15e 100644 --- a/chrome/browser/task_management/providers/web_contents/extension_task.h +++ b/chrome/browser/task_management/providers/web_contents/extension_task.h @@ -6,6 +6,7 @@ #define CHROME_BROWSER_TASK_MANAGEMENT_PROVIDERS_WEB_CONTENTS_EXTENSION_TASK_H_ #include "chrome/browser/task_management/providers/web_contents/renderer_task.h" +#include "extensions/browser/extension_icon_image.h" #include "extensions/common/view_type.h" namespace extensions { @@ -15,7 +16,9 @@ class Extension; namespace task_management { // Defines a task manager representation for extensions. -class ExtensionTask : public RendererTask { +class ExtensionTask + : public RendererTask, + public extensions::IconImage::Observer { public: ExtensionTask(content::WebContents* web_contents, const extensions::Extension* extension, @@ -27,6 +30,9 @@ class ExtensionTask : public RendererTask { void OnFaviconChanged() override; Type GetType() const override; + // extensions::IconImage::Observer + void OnExtensionIconImageChanged(extensions::IconImage* image) override; + private: // If |extension| is nullptr, this method will get the title from // the |web_contents|. @@ -35,6 +41,13 @@ class ExtensionTask : public RendererTask { const extensions::Extension* extension, extensions::ViewType view_type) const; + // This is called upon the creation of this task to load the extension icon + // for the first time if any. + void LoadExtensionIcon(const extensions::Extension* extension); + + // The favicon of the extension represented by this task. + scoped_ptr extension_icon_; + DISALLOW_COPY_AND_ASSIGN(ExtensionTask); }; diff --git a/extensions/browser/extension_icon_image_unittest.cc b/extensions/browser/extension_icon_image_unittest.cc index 3fb4e691b711b..29bb1ff207c59 100644 --- a/extensions/browser/extension_icon_image_unittest.cc +++ b/extensions/browser/extension_icon_image_unittest.cc @@ -14,6 +14,7 @@ #include "content/public/test/test_browser_thread.h" #include "extensions/browser/extensions_test.h" #include "extensions/browser/image_loader.h" +#include "extensions/browser/test_image_loader.h" #include "extensions/common/extension.h" #include "extensions/common/extension_paths.h" #include "extensions/common/manifest.h" @@ -65,55 +66,6 @@ class MockImageSkiaSource : public gfx::ImageSkiaSource { gfx::ImageSkia image_; }; -// Helper class for synchronously loading extension image resource. -class TestImageLoader { - public: - explicit TestImageLoader(const Extension* extension) - : extension_(extension), - waiting_(false), - image_loaded_(false) { - } - virtual ~TestImageLoader() {} - - void OnImageLoaded(const gfx::Image& image) { - image_ = image; - image_loaded_ = true; - if (waiting_) - base::MessageLoop::current()->Quit(); - } - - SkBitmap LoadBitmap(const std::string& path, - int size) { - image_loaded_ = false; - - 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. - if (!image_loaded_) { - waiting_ = true; - base::MessageLoop::current()->Run(); - waiting_ = false; - } - - EXPECT_TRUE(image_loaded_); - - return image_.IsEmpty() ? SkBitmap() : *image_.ToSkBitmap(); - } - - private: - const Extension* extension_; - bool waiting_; - bool image_loaded_; - gfx::Image image_; - ImageLoader image_loader_; - - DISALLOW_COPY_AND_ASSIGN(TestImageLoader); -}; - class ExtensionIconImageTest : public ExtensionsTest, public IconImage::Observer { public: @@ -184,15 +136,6 @@ class ExtensionIconImageTest : public ExtensionsTest, return gfx::ImageSkia(gfx::ImageSkiaRep(gfx::Size(16, 16), 1.0f)); } - // Loads an image to be used in test from the extension. - // The image will be loaded from the relative path |path|. - SkBitmap GetTestBitmap(const Extension* extension, - const std::string& path, - int size) { - TestImageLoader image_loader(extension); - return image_loader.LoadBitmap(path, size); - } - private: int image_loaded_count_; bool quit_in_image_loaded_; @@ -220,13 +163,14 @@ 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.get(), "16.png", 16); + SkBitmap bitmap_16 = + TestImageLoader::LoadAndGetExtensionBitmap(extension.get(), "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.get(), "48.png", 32); + TestImageLoader::LoadAndGetExtensionBitmap(extension.get(), "48.png", 32); ASSERT_FALSE(bitmap_48_resized_to_32.empty()); IconImage image(browser_context(), @@ -293,7 +237,8 @@ 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.get(), "48.png", 48); + SkBitmap bitmap_48 = + TestImageLoader::LoadAndGetExtensionBitmap(extension.get(), "48.png", 48); ASSERT_FALSE(bitmap_48.empty()); IconImage image(browser_context(), @@ -330,7 +275,8 @@ TEST_F(ExtensionIconImageTest, FallbackToBigger) { // Load images we expect to find as representations in icon_image, so we // can later use them to validate icon_image. - SkBitmap bitmap_24 = GetTestBitmap(extension.get(), "24.png", 24); + SkBitmap bitmap_24 = + TestImageLoader::LoadAndGetExtensionBitmap(extension.get(), "24.png", 24); ASSERT_FALSE(bitmap_24.empty()); IconImage image(browser_context(), @@ -522,7 +468,8 @@ 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.get(), "16.png", 16); + SkBitmap bitmap_16 = + TestImageLoader::LoadAndGetExtensionBitmap(extension.get(), "16.png", 16); ASSERT_FALSE(bitmap_16.empty()); scoped_ptr image( diff --git a/extensions/browser/test_image_loader.cc b/extensions/browser/test_image_loader.cc new file mode 100644 index 0000000000000..af6ce25cfb15a --- /dev/null +++ b/extensions/browser/test_image_loader.cc @@ -0,0 +1,58 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "extensions/browser/test_image_loader.h" + +#include "base/bind.h" +#include "extensions/browser/image_loader.h" +#include "extensions/common/extension.h" + +namespace extensions { + +TestImageLoader::TestImageLoader() : waiting_(false), image_loaded_(false) {} + +TestImageLoader::~TestImageLoader() {} + +// static +SkBitmap TestImageLoader::LoadAndGetExtensionBitmap( + const Extension* extension, + const std::string& image_path, + int size) { + TestImageLoader image_loader; + return image_loader.LoadAndGetBitmap(extension, image_path, size); +} + +void TestImageLoader::OnImageLoaded(const gfx::Image& image) { + image_ = image; + image_loaded_ = true; + if (waiting_) + loader_message_loop_quit_.Run(); +} + +SkBitmap TestImageLoader::LoadAndGetBitmap(const Extension* extension, + const std::string& path, + int size) { + image_loaded_ = false; + + ImageLoader image_loader; + 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. + if (!image_loaded_) { + waiting_ = true; + base::RunLoop run_loop; + loader_message_loop_quit_ = run_loop.QuitClosure(); + run_loop.Run(); + waiting_ = false; + } + + DCHECK(image_loaded_); + + return image_.IsEmpty() ? SkBitmap() : *image_.ToSkBitmap(); +} + +} // namespace extensions diff --git a/extensions/browser/test_image_loader.h b/extensions/browser/test_image_loader.h new file mode 100644 index 0000000000000..89003286514ed --- /dev/null +++ b/extensions/browser/test_image_loader.h @@ -0,0 +1,44 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef EXTENSIONS_BROWSER_TEST_IMAGE_LOADER_H_ +#define EXTENSIONS_BROWSER_TEST_IMAGE_LOADER_H_ + +#include "base/run_loop.h" +#include "ui/gfx/image/image.h" + +namespace extensions { + +class Extension; + +// Helper class for synchronously loading an extension image resource. +class TestImageLoader { + public: + TestImageLoader(); + ~TestImageLoader(); + + // Loads an image to be used in test from |extension|. + // The image will be loaded from the relative path |image_path|. + static SkBitmap LoadAndGetExtensionBitmap(const Extension* extension, + const std::string& image_path, + int size); + + private: + void OnImageLoaded(const gfx::Image& image); + + SkBitmap LoadAndGetBitmap(const Extension* extension, + const std::string& path, + int size); + + gfx::Image image_; + base::Closure loader_message_loop_quit_; + bool waiting_; + bool image_loaded_; + + DISALLOW_COPY_AND_ASSIGN(TestImageLoader); +}; + +} // namespace extensions + +#endif // EXTENSIONS_BROWSER_TEST_IMAGE_LOADER_H_ diff --git a/extensions/extensions.gypi b/extensions/extensions.gypi index 46e920f4d81e9..5cd17b0b9da71 100644 --- a/extensions/extensions.gypi +++ b/extensions/extensions.gypi @@ -1045,6 +1045,8 @@ 'browser/test_extension_registry_observer.h', 'browser/test_extensions_browser_client.cc', 'browser/test_extensions_browser_client.h', + 'browser/test_image_loader.cc', + 'browser/test_image_loader.h', 'browser/test_management_policy.cc', 'browser/test_management_policy.h', 'browser/test_runtime_api_delegate.cc',