diff options
author | finnur@chromium.org <finnur@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-03-24 16:00:34 +0000 |
---|---|---|
committer | finnur@chromium.org <finnur@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-03-24 16:00:34 +0000 |
commit | 699e1cd56df7b6f775601c80e99d26f83162a412 (patch) | |
tree | 22d80a14e0ddd552731daac0be3aa2e6dc8bd9ec | |
parent | bfae5e6625d0b3cd38d1b7481a12f6a177a2fbde (diff) | |
download | chromium_src-699e1cd56df7b6f775601c80e99d26f83162a412.zip chromium_src-699e1cd56df7b6f775601c80e99d26f83162a412.tar.gz chromium_src-699e1cd56df7b6f775601c80e99d26f83162a412.tar.bz2 |
Eliminate all UI thread decoding of extension images.
Except one, that is. We have one location we need to
take a look at (I've added a comment). This changelist
converts UI usage of DecodeImage on the UI thread to
a revamped and simplified ImageLoadingTracker class.
I plan on adding to GetFilePath a DCHECK for the File
thread but decided to do so in another changelist,
since it has a high likelyhood of flushing something
out and be backed out because of that. This started
out as issue 38521 (make infobar use cached icons)
but grew in scope to just eliminate all UI thread
access to DecodeImage and GetFilePath.
BUG=http://crbug.com/38521
TEST=None (extensions should work as before)
Review URL: http://codereview.chromium.org/1075006
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@42471 0039d316-1c4b-4281-b951-d872f2087c98
29 files changed, 398 insertions, 361 deletions
diff --git a/chrome/browser/cocoa/extensions/browser_action_button.mm b/chrome/browser/cocoa/extensions/browser_action_button.mm index dabba77..b9fda32 100644 --- a/chrome/browser/cocoa/extensions/browser_action_button.mm +++ b/chrome/browser/cocoa/extensions/browser_action_button.mm @@ -42,13 +42,12 @@ class ExtensionImageTrackerBridge : public NotificationObserver, public: ExtensionImageTrackerBridge(BrowserActionButton* owner, Extension* extension) : owner_(owner), - tracker_(NULL) { + tracker_(this) { // The Browser Action API does not allow the default icon path to be // changed at runtime, so we can load this now and cache it. std::string path = extension->browser_action()->default_icon_path(); if (!path.empty()) { - tracker_ = new ImageLoadingTracker(this, 1); - tracker_->PostLoadImageTask(extension->GetResource(path), + tracker_.LoadImage(extension->GetResource(path), gfx::Size(Extension::kBrowserActionIconMaxSize, Extension::kBrowserActionIconMaxSize)); } @@ -57,15 +56,12 @@ class ExtensionImageTrackerBridge : public NotificationObserver, } ~ExtensionImageTrackerBridge() { - if (tracker_) - tracker_->StopTrackingImageLoad(); } // ImageLoadingTracker::Observer implementation. - void OnImageLoaded(SkBitmap* image, size_t index) { + void OnImageLoaded(SkBitmap* image, ExtensionResource resource, int index) { if (image) [owner_ setDefaultIcon:gfx::SkBitmapToNSImage(*image)]; - tracker_ = NULL; [owner_ updateState]; } @@ -83,8 +79,8 @@ class ExtensionImageTrackerBridge : public NotificationObserver, // Weak. Owns us. BrowserActionButton* owner_; - // Loads the button's icons for us on the file thread. Weak. - ImageLoadingTracker* tracker_; + // Loads the button's icons for us on the file thread. + ImageLoadingTracker tracker_; // Used for registering to receive notifications and automatic clean up. NotificationRegistrar registrar_; diff --git a/chrome/browser/cocoa/extensions/extension_action_context_menu.mm b/chrome/browser/cocoa/extensions/extension_action_context_menu.mm index 30db253..f055e33 100644 --- a/chrome/browser/cocoa/extensions/extension_action_context_menu.mm +++ b/chrome/browser/cocoa/extensions/extension_action_context_menu.mm @@ -73,8 +73,8 @@ class AsyncUninstaller : public base::RefCountedThreadSafe<AsyncUninstaller>, if (!browser) return; - ExtensionInstallUI client(browser->profile()); - client.ConfirmUninstall(this, extension_, uninstall_icon->get()); + install_ui_.reset(new ExtensionInstallUI(browser->profile())); + install_ui_->ConfirmUninstall(this, extension_); } // The extension that we're loading the icon for. Weak. @@ -83,6 +83,8 @@ class AsyncUninstaller : public base::RefCountedThreadSafe<AsyncUninstaller>, // The uninstall icon shown by the confirmation dialog. scoped_ptr<SkBitmap> uninstall_icon_; + scoped_ptr<ExtensionInstallUI> install_ui_; + DISALLOW_COPY_AND_ASSIGN(AsyncUninstaller); }; diff --git a/chrome/browser/cocoa/location_bar_view_mac.h b/chrome/browser/cocoa/location_bar_view_mac.h index 393acf3..687fb7d 100644 --- a/chrome/browser/cocoa/location_bar_view_mac.h +++ b/chrome/browser/cocoa/location_bar_view_mac.h @@ -241,7 +241,8 @@ class LocationBarViewMac : public AutocompleteEditController, virtual void OnMousePressed(NSRect bounds); // Overridden from ImageLoadingTracker. - virtual void OnImageLoaded(SkBitmap* image, size_t index); + virtual void OnImageLoaded( + SkBitmap* image, ExtensionResource resource, int index); // Called to notify the Page Action that it should determine whether to be // visible or hidden. |contents| is the TabContents that is active, |url| @@ -263,7 +264,7 @@ class LocationBarViewMac : public AutocompleteEditController, PageActionImageView() : owner_(NULL), profile_(NULL), page_action_(NULL), - tracker_(NULL), + tracker_(this), current_tab_id_(-1), preview_enabled_(false) {} @@ -289,7 +290,7 @@ class LocationBarViewMac : public AutocompleteEditController, // The object that is waiting for the image loading to complete // asynchronously. - ImageLoadingTracker* tracker_; + ImageLoadingTracker tracker_; // The tab id we are currently showing the icon for. int current_tab_id_; diff --git a/chrome/browser/cocoa/location_bar_view_mac.mm b/chrome/browser/cocoa/location_bar_view_mac.mm index affcdfab..b2416f3 100644 --- a/chrome/browser/cocoa/location_bar_view_mac.mm +++ b/chrome/browser/cocoa/location_bar_view_mac.mm @@ -618,6 +618,7 @@ LocationBarViewMac::PageActionImageView::PageActionImageView( : owner_(owner), profile_(profile), page_action_(page_action), + tracker_(this), current_tab_id_(-1), preview_enabled_(false) { Extension* extension = profile->GetExtensionsService()->GetExtensionById( @@ -630,12 +631,11 @@ LocationBarViewMac::PageActionImageView::PageActionImageView( if (!page_action_->default_icon_path().empty()) icon_paths.push_back(page_action_->default_icon_path()); - tracker_ = new ImageLoadingTracker(this, icon_paths.size()); for (std::vector<std::string>::iterator iter = icon_paths.begin(); iter != icon_paths.end(); ++iter) { - tracker_->PostLoadImageTask(extension->GetResource(*iter), - gfx::Size(Extension::kPageActionIconMaxSize, - Extension::kPageActionIconMaxSize)); + tracker_.LoadImage(extension->GetResource(*iter), + gfx::Size(Extension::kPageActionIconMaxSize, + Extension::kPageActionIconMaxSize)); } registrar_.Add(this, NotificationType::EXTENSION_HOST_VIEW_SHOULD_CLOSE, @@ -643,8 +643,6 @@ LocationBarViewMac::PageActionImageView::PageActionImageView( } LocationBarViewMac::PageActionImageView::~PageActionImageView() { - if (tracker_) - tracker_->StopTrackingImageLoad(); } NSSize LocationBarViewMac::PageActionImageView::GetPreferredImageSize() { @@ -689,28 +687,24 @@ void LocationBarViewMac::PageActionImageView::OnMousePressed(NSRect bounds) { } } -void LocationBarViewMac::PageActionImageView::OnImageLoaded(SkBitmap* image, - size_t index) { +void LocationBarViewMac::PageActionImageView::OnImageLoaded( + SkBitmap* image, ExtensionResource resource, int index) { // We loaded icons()->size() icons, plus one extra if the Page Action had // a default icon. - int total_icons = page_action_->icon_paths()->size(); + int total_icons = static_cast<int>(page_action_->icon_paths()->size()); if (!page_action_->default_icon_path().empty()) total_icons++; - DCHECK(static_cast<int>(index) < total_icons); + DCHECK(index < total_icons); // Map the index of the loaded image back to its name. If we ever get an // index greater than the number of icons, it must be the default icon. if (image) { - if (index < page_action_->icon_paths()->size()) + if (index < static_cast<int>(page_action_->icon_paths()->size())) page_action_icons_[page_action_->icon_paths()->at(index)] = *image; else page_action_icons_[page_action_->default_icon_path()] = *image; } - // If we are done, release the tracker. - if (static_cast<int>(index) == (total_icons - 1)) - tracker_ = NULL; - owner_->UpdatePageActions(); if (preview_enabled_) diff --git a/chrome/browser/extensions/crx_installer.cc b/chrome/browser/extensions/crx_installer.cc index 93ec0bb..c342a3d 100644 --- a/chrome/browser/extensions/crx_installer.cc +++ b/chrome/browser/extensions/crx_installer.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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. @@ -166,7 +166,7 @@ void CrxInstaller::ConfirmInstall() { if (client_.get()) { AddRef(); // Balanced in Proceed() and Abort(). - client_->ConfirmInstall(this, extension_.get(), install_icon_.get()); + client_->ConfirmInstall(this, extension_.get()); } else { ChromeThread::PostTask( ChromeThread::FILE, FROM_HERE, diff --git a/chrome/browser/extensions/extension_browsertest.cc b/chrome/browser/extensions/extension_browsertest.cc index 5f214fd..90cca84 100644 --- a/chrome/browser/extensions/extension_browsertest.cc +++ b/chrome/browser/extensions/extension_browsertest.cc @@ -87,20 +87,18 @@ class MockAbortExtensionInstallUI : public ExtensionInstallUI { MockAbortExtensionInstallUI() : ExtensionInstallUI(NULL) {} // Simulate a user abort on an extension installation. - void ConfirmInstall( - Delegate* delegate, Extension* extension, SkBitmap* icon) { + virtual void ConfirmInstall(Delegate* delegate, Extension* extension) { delegate->InstallUIAbort(); MessageLoopForUI::current()->Quit(); } - void ConfirmUninstall(Delegate* delegate, Extension* extension, - SkBitmap* icon) {} + virtual void ConfirmUninstall(Delegate* delegate, Extension* extension) {} - void OnInstallSuccess(Extension* extension) {} + virtual void OnInstallSuccess(Extension* extension) {} - void OnInstallFailure(const std::string& error) {} + virtual void OnInstallFailure(const std::string& error) {} - void OnOverinstallAttempted(Extension* extension) {} + virtual void OnOverinstallAttempted(Extension* extension) {} }; bool ExtensionBrowserTest::InstallOrUpdateExtension(const std::string& id, diff --git a/chrome/browser/extensions/extension_context_menu_model.cc b/chrome/browser/extensions/extension_context_menu_model.cc index 2c3be7d..0a0438f 100644 --- a/chrome/browser/extensions/extension_context_menu_model.cc +++ b/chrome/browser/extensions/extension_context_menu_model.cc @@ -109,12 +109,8 @@ void ExtensionContextMenuModel::ExecuteCommand(int command_id) { break; } case UNINSTALL: { - scoped_ptr<SkBitmap> uninstall_icon; - Extension::DecodeIcon(extension_, Extension::EXTENSION_ICON_LARGE, - &uninstall_icon); - - ExtensionInstallUI client(profile_); - client.ConfirmUninstall(this, extension_, uninstall_icon.get()); + install_ui_.reset(new ExtensionInstallUI(profile_)); + install_ui_->ConfirmUninstall(this, extension_); break; } case MANAGE: { diff --git a/chrome/browser/extensions/extension_context_menu_model.h b/chrome/browser/extensions/extension_context_menu_model.h index 21667ef..14b3574 100644 --- a/chrome/browser/extensions/extension_context_menu_model.h +++ b/chrome/browser/extensions/extension_context_menu_model.h @@ -64,6 +64,9 @@ class ExtensionContextMenuModel // The delegate which handles the 'inspect popup' menu command (or NULL). PopupDelegate* delegate_; + // Keeps track of the extension install UI. + scoped_ptr<ExtensionInstallUI> install_ui_; + DISALLOW_COPY_AND_ASSIGN(ExtensionContextMenuModel); }; diff --git a/chrome/browser/extensions/extension_disabled_infobar_delegate.cc b/chrome/browser/extensions/extension_disabled_infobar_delegate.cc index ad82ef35..18812a9 100644 --- a/chrome/browser/extensions/extension_disabled_infobar_delegate.cc +++ b/chrome/browser/extensions/extension_disabled_infobar_delegate.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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. @@ -24,16 +24,11 @@ class ExtensionDisabledDialogDelegate ExtensionDisabledDialogDelegate(Profile* profile, ExtensionsService* service, Extension* extension) - : profile_(profile), service_(service), extension_(extension) { + : service_(service), extension_(extension) { AddRef(); // Balanced in Proceed or Abort. - // Do this now because we can't touch extension on the file loop. - install_icon_resource_ = - extension_->GetIconPath(Extension::EXTENSION_ICON_LARGE); - - ChromeThread::PostTask( - ChromeThread::FILE, FROM_HERE, - NewRunnableMethod(this, &ExtensionDisabledDialogDelegate::Start)); + install_ui_.reset(new ExtensionInstallUI(profile)); + install_ui_->ConfirmInstall(this, extension_); } // ExtensionInstallUI::Delegate @@ -53,29 +48,11 @@ class ExtensionDisabledDialogDelegate virtual ~ExtensionDisabledDialogDelegate() {} - void Start() { - // We start on the file thread so we can decode the install icon. - FilePath install_icon_path = install_icon_resource_.GetFilePath(); - Extension::DecodeIconFromPath( - install_icon_path, Extension::EXTENSION_ICON_LARGE, &install_icon_); - // Then we display the UI on the UI thread. - ChromeThread::PostTask( - ChromeThread::UI, FROM_HERE, - NewRunnableMethod( - this, &ExtensionDisabledDialogDelegate::ConfirmInstall)); - } - - void ConfirmInstall() { - DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI)); - ExtensionInstallUI ui(profile_); - ui.ConfirmInstall(this, extension_, install_icon_.get()); - } + // The UI for showing the install dialog when enabling. + scoped_ptr<ExtensionInstallUI> install_ui_; - Profile* profile_; ExtensionsService* service_; Extension* extension_; - ExtensionResource install_icon_resource_; - scoped_ptr<SkBitmap> install_icon_; }; class ExtensionDisabledInfobarDelegate diff --git a/chrome/browser/extensions/extension_dom_ui.cc b/chrome/browser/extensions/extension_dom_ui.cc index c3663a0..907e6c0 100644 --- a/chrome/browser/extensions/extension_dom_ui.cc +++ b/chrome/browser/extensions/extension_dom_ui.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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. @@ -331,6 +331,8 @@ RefCountedMemory* ExtensionDOMUI::GetFaviconResourceBytes(Profile* profile, if (!extension) return NULL; - return ReadFileData( - extension->GetIconPath(Extension::EXTENSION_ICON_BITTY).GetFilePath()); + // TODO(arv): Move this off of the UI thread and onto the File thread. If + // possible to do this asynchronously, use ImageLoadingTracker. + return ReadFileData(extension->GetIconPath( + Extension::EXTENSION_ICON_BITTY).GetFilePath()); } diff --git a/chrome/browser/extensions/extension_install_ui.cc b/chrome/browser/extensions/extension_install_ui.cc index e7a8406..f36f02b 100644 --- a/chrome/browser/extensions/extension_install_ui.cc +++ b/chrome/browser/extensions/extension_install_ui.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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. @@ -127,16 +127,22 @@ static std::wstring GetInstallWarning(Extension* extension) { } // namespace ExtensionInstallUI::ExtensionInstallUI(Profile* profile) - : profile_(profile), ui_loop_(MessageLoop::current()) + : profile_(profile), + ui_loop_(MessageLoop::current()), + extension_(NULL), + delegate_(NULL), + prompt_type_(NUM_PROMPT_TYPES), + ALLOW_THIS_IN_INITIALIZER_LIST(tracker_(this)) #if defined(TOOLKIT_GTK) - ,previous_use_gtk_theme_(false) + , previous_use_gtk_theme_(false) #endif {} void ExtensionInstallUI::ConfirmInstall(Delegate* delegate, - Extension* extension, - SkBitmap* install_icon) { + Extension* extension) { DCHECK(ui_loop_ == MessageLoop::current()); + extension_ = extension; + delegate_ = delegate; // We special-case themes to not show any confirm UI. Instead they are // immediately installed, and then we show an infobar (see OnInstallSuccess) @@ -148,7 +154,7 @@ void ExtensionInstallUI::ConfirmInstall(Delegate* delegate, previous_theme_id_ = previous_theme->id(); #if defined(TOOLKIT_GTK) - // On linux, we also need to take the user's system settings into account + // On Linux, we also need to take the user's system settings into account // to undo theme installation. previous_use_gtk_theme_ = GtkThemeProvider::GetFrom(profile_)->UseGtkTheme(); @@ -158,53 +164,25 @@ void ExtensionInstallUI::ConfirmInstall(Delegate* delegate, return; } - if (!install_icon) { - install_icon = ResourceBundle::GetSharedInstance().GetBitmapNamed( - IDR_EXTENSION_DEFAULT_ICON); - } - icon_ = *install_icon; - - NotificationService* service = NotificationService::current(); - service->Notify(NotificationType::EXTENSION_WILL_SHOW_CONFIRM_DIALOG, - Source<ExtensionInstallUI>(this), - NotificationService::NoDetails()); - - ShowExtensionInstallUIPromptImpl( - profile_, delegate, extension, &icon_, - WideToUTF16Hack(GetInstallWarning(extension)), INSTALL_PROMPT); + ShowConfirmation(INSTALL_PROMPT); } void ExtensionInstallUI::ConfirmUninstall(Delegate* delegate, - Extension* extension, - SkBitmap* icon) { + Extension* extension) { DCHECK(ui_loop_ == MessageLoop::current()); + extension_ = extension; + delegate_ = delegate; - if (!icon) { - icon = ResourceBundle::GetSharedInstance().GetBitmapNamed( - IDR_EXTENSION_DEFAULT_ICON); - } - - string16 message = - l10n_util::GetStringUTF16(IDS_EXTENSION_UNINSTALL_CONFIRMATION); - ShowExtensionInstallUIPromptImpl(profile_, delegate, extension, icon, - message, UNINSTALL_PROMPT); + ShowConfirmation(UNINSTALL_PROMPT); } void ExtensionInstallUI::ConfirmEnableIncognito(Delegate* delegate, - Extension* extension, - SkBitmap* icon) { + Extension* extension) { DCHECK(ui_loop_ == MessageLoop::current()); + extension_ = extension; + delegate_ = delegate; - if (!icon) { - icon = ResourceBundle::GetSharedInstance().GetBitmapNamed( - IDR_EXTENSION_DEFAULT_ICON); - } - - string16 message = - l10n_util::GetStringFUTF16(IDS_EXTENSION_PROMPT_WARNING_INCOGNITO, - l10n_util::GetStringUTF16(IDS_PRODUCT_NAME)); - ShowExtensionInstallUIPromptImpl(profile_, delegate, extension, icon, - message, ENABLE_INCOGNITO_PROMPT); + ShowConfirmation(ENABLE_INCOGNITO_PROMPT); } void ExtensionInstallUI::OnInstallSuccess(Extension* extension) { @@ -213,7 +191,7 @@ void ExtensionInstallUI::OnInstallSuccess(Extension* extension) { return; } - // GetLastActiveWithProfile will fail on the build bots. This needs to + // GetLastActiveWithProfile will fail on the build bots. This needs to be // implemented differently if any test is created which depends on // ExtensionInstalledBubble showing. #if defined(TOOLKIT_VIEWS) @@ -258,6 +236,50 @@ void ExtensionInstallUI::OnOverinstallAttempted(Extension* extension) { ShowThemeInfoBar(extension); } +void ExtensionInstallUI::OnImageLoaded( + SkBitmap* image, ExtensionResource resource, int index) { + if (image) + icon_ = *image; + else + icon_ = SkBitmap(); + if (icon_.empty()) { + icon_ = *ResourceBundle::GetSharedInstance().GetBitmapNamed( + IDR_EXTENSION_DEFAULT_ICON); + } + + switch (prompt_type_) { + case INSTALL_PROMPT: { + NotificationService* service = NotificationService::current(); + service->Notify(NotificationType::EXTENSION_WILL_SHOW_CONFIRM_DIALOG, + Source<ExtensionInstallUI>(this), + NotificationService::NoDetails()); + + ShowExtensionInstallUIPromptImpl( + profile_, delegate_, extension_, &icon_, + WideToUTF16Hack(GetInstallWarning(extension_)), INSTALL_PROMPT); + break; + } + case UNINSTALL_PROMPT: { + string16 message = + l10n_util::GetStringUTF16(IDS_EXTENSION_UNINSTALL_CONFIRMATION); + ShowExtensionInstallUIPromptImpl(profile_, delegate_, extension_, &icon_, + message, UNINSTALL_PROMPT); + break; + } + case ENABLE_INCOGNITO_PROMPT: { + string16 message = + l10n_util::GetStringFUTF16(IDS_EXTENSION_PROMPT_WARNING_INCOGNITO, + l10n_util::GetStringUTF16(IDS_PRODUCT_NAME)); + ShowExtensionInstallUIPromptImpl(profile_, delegate_, extension_, &icon_, + message, ENABLE_INCOGNITO_PROMPT); + break; + } + default: + NOTREACHED() << "Unknown message"; + break; + } +} + void ExtensionInstallUI::ShowThemeInfoBar(Extension* new_theme) { if (!new_theme->IsTheme()) return; @@ -290,6 +312,16 @@ void ExtensionInstallUI::ShowThemeInfoBar(Extension* new_theme) { tab_contents->AddInfoBar(new_delegate); } +void ExtensionInstallUI::ShowConfirmation(PromptType prompt_type) { + // Load the image asynchronously. For the response, check OnImageLoaded. + prompt_type_ = prompt_type; + ExtensionResource image = + extension_->GetIconPath(Extension::EXTENSION_ICON_LARGE); + tracker_.LoadImage(image, + gfx::Size(Extension::EXTENSION_ICON_LARGE, + Extension::EXTENSION_ICON_LARGE)); +} + #if defined(OS_MACOSX) void ExtensionInstallUI::ShowGenericExtensionInstalledInfoBar( Extension* new_extension) { diff --git a/chrome/browser/extensions/extension_install_ui.h b/chrome/browser/extensions/extension_install_ui.h index 05562e8..14c5f93 100644 --- a/chrome/browser/extensions/extension_install_ui.h +++ b/chrome/browser/extensions/extension_install_ui.h @@ -1,18 +1,19 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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_EXTENSION_INSTALL_UI_H_ #define CHROME_BROWSER_EXTENSIONS_EXTENSION_INSTALL_UI_H_ +#include <string> + #include "base/file_path.h" #include "base/ref_counted.h" #include "base/scoped_ptr.h" +#include "chrome/browser/extensions/image_loading_tracker.h" #include "gfx/native_widget_types.h" #include "third_party/skia/include/core/SkBitmap.h" -#include <string> - class Extension; class ExtensionsService; class MessageLoop; @@ -22,7 +23,7 @@ class SandboxedExtensionUnpacker; class TabContents; // Displays all the UI around extension installation and uninstallation. -class ExtensionInstallUI { +class ExtensionInstallUI : public ImageLoadingTracker::Observer { public: enum PromptType { INSTALL_PROMPT = 0, @@ -56,24 +57,21 @@ class ExtensionInstallUI { // // We *MUST* eventually call either Proceed() or Abort() // on |delegate|. - virtual void ConfirmInstall(Delegate* delegate, Extension* extension, - SkBitmap* icon); + virtual void ConfirmInstall(Delegate* delegate, Extension* extension); // This is called by the extensions management page to verify whether the // uninstallation should proceed. This is declared virtual for testing. // // We *MUST* eventually call either Proceed() or Abort() // on |delegate|. - virtual void ConfirmUninstall(Delegate* delegate, Extension* extension, - SkBitmap* icon); + virtual void ConfirmUninstall(Delegate* delegate, Extension* extension); // This is called by the extensions management page to verify whether the // uninstallation should proceed. This is declared virtual for testing. // // We *MUST* eventually call either Proceed() or Abort() // on |delegate|. - virtual void ConfirmEnableIncognito(Delegate* delegate, Extension* extension, - SkBitmap* icon); + virtual void ConfirmEnableIncognito(Delegate* delegate, Extension* extension); // Installation was successful. This is declared virtual for testing. virtual void OnInstallSuccess(Extension* extension); @@ -85,11 +83,20 @@ class ExtensionInstallUI { // installed. This is declared virtual for testing. virtual void OnOverinstallAttempted(Extension* extension); + // ImageLoadingTracker::Observer overrides. + virtual void OnImageLoaded( + SkBitmap* image, ExtensionResource resource, int index); + private: // When a Theme is downloaded it is applied and an info bar is shown to give // the user a choice to keep it or undo the installation. void ShowThemeInfoBar(Extension* new_theme); + // 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). + void ShowConfirmation(PromptType prompt_type); + #if defined(OS_MACOSX) // When an extension is installed on Mac with neither browser action nor // page action icons, show an infobar instead of a popup bubble. @@ -111,9 +118,17 @@ class ExtensionInstallUI { MessageLoop* ui_loop_; std::string previous_theme_id_; // Used to undo theme installation. SkBitmap icon_; // The extensions installation icon. + Extension* extension_; // The extension we are showing the UI for. + Delegate* delegate_; // The delegate we will call Proceed/Abort on after + // confirmation UI. + PromptType prompt_type_; // The type of prompt we are going to show. + + // Keeps track of extension images being loaded on the File thread for the + // purpose of showing the install UI. + ImageLoadingTracker tracker_; #if defined(TOOLKIT_GTK) - // Also needed to undo theme installation in the linux UI. + // Also needed to undo theme installation in the Linux UI. bool previous_use_gtk_theme_; #endif }; diff --git a/chrome/browser/extensions/extensions_ui.cc b/chrome/browser/extensions/extensions_ui.cc index c43daac..1575577 100644 --- a/chrome/browser/extensions/extensions_ui.cc +++ b/chrome/browser/extensions/extensions_ui.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2006-2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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. @@ -372,6 +372,12 @@ ExtensionResource ExtensionsDOMHandler::PickExtensionIcon( return ExtensionResource(); } +ExtensionInstallUI* ExtensionsDOMHandler::GetExtensionInstallUI() { + if (!install_ui_.get()) + install_ui_.reset(new ExtensionInstallUI(dom_ui_->GetProfile())); + return install_ui_.get(); +} + void ExtensionsDOMHandler::HandleToggleDeveloperMode(const Value* value) { bool developer_mode = dom_ui_->GetProfile()->GetPrefs() ->GetBoolean(prefs::kExtensionsUIDeveloperMode); @@ -448,13 +454,10 @@ void ExtensionsDOMHandler::HandleEnableIncognitoMessage(const Value* value) { return; // only one prompt at a time // Prompt the user first. - scoped_ptr<SkBitmap> icon; - Extension::DecodeIcon(extension, Extension::EXTENSION_ICON_LARGE, &icon); - ui_prompt_type_ = ExtensionInstallUI::ENABLE_INCOGNITO_PROMPT; extension_id_prompting_ = extension_id; - ExtensionInstallUI client(dom_ui_->GetProfile()); - client.ConfirmEnableIncognito(this, extension, icon.get()); + + GetExtensionInstallUI()->ConfirmEnableIncognito(this, extension); } else { extensions_service_->SetIsIncognitoEnabled(extension, false); } @@ -475,14 +478,10 @@ void ExtensionsDOMHandler::HandleUninstallMessage(const Value* value) { if (!extension_id_prompting_.empty()) return; // only one prompt at a time - scoped_ptr<SkBitmap> uninstall_icon; - Extension::DecodeIcon(extension, Extension::EXTENSION_ICON_LARGE, - &uninstall_icon); - ui_prompt_type_ = ExtensionInstallUI::UNINSTALL_PROMPT; extension_id_prompting_ = extension_id; - ExtensionInstallUI client(dom_ui_->GetProfile()); - client.ConfirmUninstall(this, extension, uninstall_icon.get()); + + GetExtensionInstallUI()->ConfirmUninstall(this, extension); } void ExtensionsDOMHandler::InstallUIProceed(bool create_app_shortcut) { diff --git a/chrome/browser/extensions/extensions_ui.h b/chrome/browser/extensions/extensions_ui.h index daf5c2b..8a96c54 100644 --- a/chrome/browser/extensions/extensions_ui.h +++ b/chrome/browser/extensions/extensions_ui.h @@ -1,4 +1,4 @@ -// Copyright (c) 2006-2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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. @@ -56,7 +56,7 @@ class ExtensionsUIHTMLSource : public ChromeURLDataManager::DataSource { DISALLOW_COPY_AND_ASSIGN(ExtensionsUIHTMLSource); }; -// The handler for Javascript messages related to the "extensions" view. +// The handler for JavaScript messages related to the "extensions" view. class ExtensionsDOMHandler : public DOMMessageHandler, public NotificationObserver, @@ -201,6 +201,10 @@ class ExtensionsDOMHandler // Called on the UI thread. void OnIconsLoaded(DictionaryValue* json_data); + // Returns the ExtensionInstallUI object for this class, creating it if + // needed. + ExtensionInstallUI* GetExtensionInstallUI(); + // Our model. scoped_refptr<ExtensionsService> extensions_service_; @@ -213,6 +217,10 @@ class ExtensionsDOMHandler // Used to load icons asynchronously on the file thread. scoped_refptr<IconLoader> icon_loader_; + // Used to show confirmation UI for uninstalling/enabling extensions in + // incognito mode. + scoped_ptr<ExtensionInstallUI> install_ui_; + // We monitor changes to the extension system so that we can reload when // necessary. NotificationRegistrar registrar_; diff --git a/chrome/browser/extensions/image_loading_tracker.cc b/chrome/browser/extensions/image_loading_tracker.cc index a6eafca..a1f4ad2 100644 --- a/chrome/browser/extensions/image_loading_tracker.cc +++ b/chrome/browser/extensions/image_loading_tracker.cc @@ -1,60 +1,55 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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_loading_tracker.h" #include "base/file_util.h" -#include "base/logging.h" -#include "base/message_loop.h" -#include "base/scoped_ptr.h" -#include "base/task.h" -#include "base/thread.h" #include "chrome/browser/chrome_thread.h" #include "chrome/common/extensions/extension_resource.h" -#include "gfx/size.h" #include "skia/ext/image_operations.h" #include "third_party/skia/include/core/SkBitmap.h" #include "webkit/glue/image_decoder.h" //////////////////////////////////////////////////////////////////////////////// -// ImageLoadingTracker::LoadImageTask +// ImageLoadingTracker::ImageLoader -// The LoadImageTask is for asynchronously loading the image on the file thread. -// If the image is successfully loaded and decoded it will report back on the -// calling thread to let the caller know the image is done loading. -class ImageLoadingTracker::LoadImageTask : public Task { +// A RefCounted class for loading images on the File thread and reporting back +// on the UI thread. +class ImageLoadingTracker::ImageLoader + : public base::RefCountedThreadSafe<ImageLoader> { public: - // Constructor for the LoadImageTask class. |tracker| is the object that - // we use to communicate back to the entity that wants the image after we - // decode it. |path| is the path to load the image from. |max_size| is the - // maximum size for the loaded image. It will be resized to fit this if - // larger. |index| is an identifier for the image that we pass back to the - // caller. - LoadImageTask(ImageLoadingTracker* tracker, - const ExtensionResource& resource, - const gfx::Size& max_size, - size_t index) - : tracker_(tracker), - resource_(resource), - max_size_(max_size), - index_(index) { + explicit ImageLoader(ImageLoadingTracker* tracker) + : tracker_(tracker) { CHECK(ChromeThread::GetCurrentThreadIdentifier(&callback_thread_id_)); + DCHECK(!ChromeThread::CurrentlyOn(ChromeThread::FILE)); } - void ReportBack(SkBitmap* image) { + // Lets this class know that the tracker is no longer interested in the + // results. + void StopTracking() { + tracker_ = NULL; + } + + // Instructs the loader to load a task on the File thread. + void LoadImage(const ExtensionResource& resource, + const gfx::Size& max_size) { + DCHECK(!ChromeThread::CurrentlyOn(ChromeThread::FILE)); ChromeThread::PostTask( - callback_thread_id_, FROM_HERE, - NewRunnableMethod( - tracker_, &ImageLoadingTracker::OnImageLoaded, image, index_)); + ChromeThread::FILE, FROM_HERE, + NewRunnableMethod(this, &ImageLoader::LoadOnFileThread, resource, + max_size)); } - virtual void Run() { + void LoadOnFileThread(ExtensionResource resource, + const gfx::Size& max_size) { + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE)); + // Read the file from disk. std::string file_contents; - FilePath path = resource_.GetFilePath(); + FilePath path = resource.GetFilePath(); if (path.empty() || !file_util::ReadFileToString(path, &file_contents)) { - ReportBack(NULL); + ReportBack(NULL, resource); return; } @@ -65,55 +60,83 @@ class ImageLoadingTracker::LoadImageTask : public Task { scoped_ptr<SkBitmap> decoded(new SkBitmap()); *decoded = decoder.Decode(data, file_contents.length()); if (decoded->empty()) { - ReportBack(NULL); + ReportBack(NULL, resource); return; // Unable to decode. } - if (decoded->width() > max_size_.width() || - decoded->height() > max_size_.height()) { + if (decoded->width() > max_size.width() || + decoded->height() > max_size.height()) { // The bitmap is too big, re-sample. *decoded = skia::ImageOperations::Resize( *decoded, skia::ImageOperations::RESIZE_LANCZOS3, - max_size_.width(), max_size_.height()); + max_size.width(), max_size.height()); } - ReportBack(decoded.release()); + ReportBack(decoded.release(), resource); } - private: - // The thread that we need to call back on to report that we are done. - ChromeThread::ID callback_thread_id_; + void ReportBack(SkBitmap* image, const ExtensionResource& resource) { + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE)); - // The object that is waiting for us to respond back. - ImageLoadingTracker* tracker_; + ChromeThread::PostTask( + callback_thread_id_, FROM_HERE, + NewRunnableMethod(this, &ImageLoader::ReportOnUIThread, + image, resource)); + } - // The image resource to load asynchronously. - ExtensionResource resource_; + void ReportOnUIThread(SkBitmap* image, ExtensionResource resource) { + DCHECK(!ChromeThread::CurrentlyOn(ChromeThread::FILE)); - // The max size for the loaded image. - gfx::Size max_size_; + if (tracker_) + tracker_->OnImageLoaded(image, resource); - // The index of the icon being loaded. - size_t index_; + if (image) + delete image; + } + + private: + // The tracker we are loading the image for. If NULL, it means the tracker is + // no longer interested in the reply. + ImageLoadingTracker* tracker_; + + // The thread that we need to call back on to report that we are done. + ChromeThread::ID callback_thread_id_; + + DISALLOW_COPY_AND_ASSIGN(ImageLoader); }; //////////////////////////////////////////////////////////////////////////////// // ImageLoadingTracker -void ImageLoadingTracker::PostLoadImageTask(const ExtensionResource& resource, - const gfx::Size& max_size) { - ChromeThread::PostTask( - ChromeThread::FILE, FROM_HERE, - new LoadImageTask(this, resource, max_size, posted_count_++)); +ImageLoadingTracker::ImageLoadingTracker(Observer* observer) + : observer_(observer), + responses_(0) { } -void ImageLoadingTracker::OnImageLoaded(SkBitmap* image, size_t index) { - if (observer_) - observer_->OnImageLoaded(image, index); +ImageLoadingTracker::~ImageLoadingTracker() { + // The loader is created lazily and is NULL if the tracker is destroyed before + // any valid image load tasks have been posted. + if (loader_) + loader_->StopTracking(); +} - if (image) - delete image; +void ImageLoadingTracker::LoadImage(const ExtensionResource& resource, + gfx::Size max_size) { + // If we don't have a path we don't need to do any further work, just respond + // back. + if (resource.relative_path().empty()) { + OnImageLoaded(NULL, resource); + return; + } + + // Instruct the ImageLoader to load this on the File thread. LoadImage does + // not block. + if (!loader_) + loader_ = new ImageLoader(this); + loader_->LoadImage(resource, max_size); +} - if (--image_count_ == 0) - Release(); // We are no longer needed. +void ImageLoadingTracker::OnImageLoaded( + SkBitmap* image, const ExtensionResource& resource) { + observer_->OnImageLoaded(image, resource, responses_++); } diff --git a/chrome/browser/extensions/image_loading_tracker.h b/chrome/browser/extensions/image_loading_tracker.h index 23f7b71..adcf009 100644 --- a/chrome/browser/extensions/image_loading_tracker.h +++ b/chrome/browser/extensions/image_loading_tracker.h @@ -1,4 +1,4 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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. @@ -6,76 +6,68 @@ #define CHROME_BROWSER_EXTENSIONS_IMAGE_LOADING_TRACKER_H_ #include "base/ref_counted.h" +#include "chrome/common/extensions/extension.h" class ExtensionResource; class SkBitmap; namespace gfx { -class Size; + class Size; } // The views need to load their icons asynchronously but might be deleted before -// the images have loaded. This class stays alive while the request is in -// progress (manages its own lifetime) and keeps track of whether the view still -// cares about the icon loading. -// Consider abstracting out a FilePathProvider (ExtensionResource) and moving -// back to chrome/browser/ if other subsystems want to use it. -class ImageLoadingTracker - : public base::RefCountedThreadSafe<ImageLoadingTracker> { +// the images have loaded. This class encapsulates a loader class that stays +// alive while the request is in progress (manages its own lifetime) and keeps +// track of whether the view still cares about the icon loading. +// +// To use this class, have your class derive from ImageLoadingTracker::Observer, +// and add a member variable ImageLoadingTracker tracker_. Then override +// Observer::OnImageLoaded and call: +// tracker_.LoadImage(resource, max_size); +// ... and wait for OnImageLoaded to be called back on you with a pointer to the +// SkBitmap loaded. +// +class ImageLoadingTracker { public: class Observer { public: // Will be called when the image with the given index has loaded. // The |image| is owned by the tracker, so the observer should make a copy - // if they need to access it after this call. - virtual void OnImageLoaded(SkBitmap* image, size_t index) = 0; + // if they need to access it after this call. |image| can be null if valid + // image was not found or it failed to decode. |resource| is the + // ExtensionResource where the |image| came from and the |index| represents + // the index of the image just loaded (starts at 0 and increments every + // time this function is called). + virtual void OnImageLoaded(SkBitmap* image, ExtensionResource resource, + int index) = 0; }; - ImageLoadingTracker(Observer* observer, size_t image_count) - : observer_(observer), image_count_(image_count), posted_count_(0) { - AddRef(); // We hold on to a reference to ourself to make sure we don't - // get deleted until we get a response from image loading (see - // ImageLoadingDone). - } + explicit ImageLoadingTracker(Observer* observer); + ~ImageLoadingTracker(); - // If there are remaining images to be loaded, the observing object should - // call this method on its destruction, so that the tracker will not attempt - // to make any more callbacks to it. - void StopTrackingImageLoad() { - observer_ = NULL; - } - - // Specify image resource to load. This method must be called a number of - // times equal to the |image_count| arugment to the constructor. Calling it - // any more or less than that is an error. If the loaded image is larger than + // Specify image resource to load. If the loaded image is larger than // |max_size| it will be resized to those dimensions. - void PostLoadImageTask(const ExtensionResource& resource, - const gfx::Size& max_size); + void LoadImage(const ExtensionResource& resource, + gfx::Size max_size); private: - class LoadImageTask; - - friend class base::RefCountedThreadSafe<ImageLoadingTracker>; - - ~ImageLoadingTracker() {} + class ImageLoader; - // When an image has finished loaded and scaled on the file thread, it is - // posted back to this method on the original thread. This method then calls - // the observer's OnImageLoaded and deletes the ImageLoadingTracker if it was - // the last image in the list. + // When an image has finished loaded and been resized on the file thread, it + // is posted back to this method on the original thread. This method then + // calls the observer's OnImageLoaded and deletes the ImageLoadingTracker if + // it was the last image in the list. // |image| may be null if the file failed to decode. - void OnImageLoaded(SkBitmap* image, size_t index); + void OnImageLoaded(SkBitmap* image, const ExtensionResource& resource); // The view that is waiting for the image to load. Observer* observer_; - // The number of images this ImageTracker should keep track of. This is - // decremented as each image finishes loading, and the tracker will delete - // itself when it reaches zero. - size_t image_count_; + // The number of times we've reported back. + int responses_; - // The number of tasks that have been posted so far. - size_t posted_count_; + // The object responsible for loading the image on the File thread. + scoped_refptr<ImageLoader> loader_; DISALLOW_COPY_AND_ASSIGN(ImageLoadingTracker); }; diff --git a/chrome/browser/gtk/browser_actions_toolbar_gtk.cc b/chrome/browser/gtk/browser_actions_toolbar_gtk.cc index b6bb11b..6f608c4 100644 --- a/chrome/browser/gtk/browser_actions_toolbar_gtk.cc +++ b/chrome/browser/gtk/browser_actions_toolbar_gtk.cc @@ -69,7 +69,7 @@ class BrowserActionButton : public NotificationObserver, Extension* extension) : toolbar_(toolbar), extension_(extension), - tracker_(NULL), + tracker_(this), tab_specific_icon_(NULL), default_icon_(NULL) { button_.Own( @@ -85,8 +85,7 @@ class BrowserActionButton : public NotificationObserver, // changed at runtime, so we can load this now and cache it. std::string path = extension_->browser_action()->default_icon_path(); if (!path.empty()) { - tracker_ = new ImageLoadingTracker(this, 1); - tracker_->PostLoadImageTask(extension_->GetResource(path), + tracker_.LoadImage(extension_->GetResource(path), gfx::Size(Extension::kBrowserActionIconMaxSize, Extension::kBrowserActionIconMaxSize)); } @@ -112,9 +111,6 @@ class BrowserActionButton : public NotificationObserver, g_object_unref(default_icon_); button_.Destroy(); - - if (tracker_) - tracker_->StopTrackingImageLoad(); } GtkWidget* widget() { return button_.get(); } @@ -132,12 +128,11 @@ class BrowserActionButton : public NotificationObserver, } // ImageLoadingTracker::Observer implementation. - void OnImageLoaded(SkBitmap* image, size_t index) { + void OnImageLoaded(SkBitmap* image, ExtensionResource resource, int index) { if (image) { default_skbitmap_ = *image; default_icon_ = gfx::GdkPixbufFromSkBitmap(image); } - tracker_ = NULL; // The tracker object will delete itself when we return. UpdateState(); } @@ -274,7 +269,7 @@ class BrowserActionButton : public NotificationObserver, OwnedWidgetGtk button_; // Loads the button's icons for us on the file thread. - ImageLoadingTracker* tracker_; + ImageLoadingTracker tracker_; // If we are displaying a tab-specific icon, it will be here. GdkPixbuf* tab_specific_icon_; @@ -644,8 +639,8 @@ gboolean BrowserActionsToolbarGtk::OnDragFailed(GtkWidget* widget, return TRUE; } -void BrowserActionsToolbarGtk::OnHierarchyChanged(GtkWidget* widget, - GtkWidget* previous_toplevel) { +void BrowserActionsToolbarGtk::OnHierarchyChanged( + GtkWidget* widget, GtkWidget* previous_toplevel) { GtkWidget* toplevel = gtk_widget_get_toplevel(widget); if (!GTK_WIDGET_TOPLEVEL(toplevel)) return; diff --git a/chrome/browser/gtk/location_bar_view_gtk.cc b/chrome/browser/gtk/location_bar_view_gtk.cc index 541d01e..a764b3d 100644 --- a/chrome/browser/gtk/location_bar_view_gtk.cc +++ b/chrome/browser/gtk/location_bar_view_gtk.cc @@ -1057,6 +1057,7 @@ LocationBarViewGtk::PageActionViewGtk::PageActionViewGtk( profile_(profile), page_action_(page_action), last_icon_pixbuf_(NULL), + tracker_(this), preview_enabled_(false) { event_box_.Own(gtk_event_box_new()); gtk_widget_set_size_request(event_box_.get(), @@ -1083,19 +1084,15 @@ LocationBarViewGtk::PageActionViewGtk::PageActionViewGtk( if (!page_action_->default_icon_path().empty()) icon_paths.push_back(page_action_->default_icon_path()); - tracker_ = new ImageLoadingTracker(this, icon_paths.size()); for (std::vector<std::string>::iterator iter = icon_paths.begin(); iter != icon_paths.end(); ++iter) { - tracker_->PostLoadImageTask( - extension->GetResource(*iter), - gfx::Size(Extension::kPageActionIconMaxSize, - Extension::kPageActionIconMaxSize)); + tracker_.LoadImage(extension->GetResource(*iter), + gfx::Size(Extension::kPageActionIconMaxSize, + Extension::kPageActionIconMaxSize)); } } LocationBarViewGtk::PageActionViewGtk::~PageActionViewGtk() { - if (tracker_) - tracker_->StopTrackingImageLoad(); image_.Destroy(); event_box_.Destroy(); for (PixbufMap::iterator iter = pixbufs_.begin(); iter != pixbufs_.end(); @@ -1178,11 +1175,11 @@ void LocationBarViewGtk::PageActionViewGtk::UpdateVisibility( } } -void LocationBarViewGtk::PageActionViewGtk::OnImageLoaded(SkBitmap* image, - size_t index) { +void LocationBarViewGtk::PageActionViewGtk::OnImageLoaded( + SkBitmap* image, ExtensionResource resource, int index) { // We loaded icons()->size() icons, plus one extra if the page action had // a default icon. - size_t total_icons = page_action_->icon_paths()->size(); + int total_icons = static_cast<int>(page_action_->icon_paths()->size()); if (!page_action_->default_icon_path().empty()) total_icons++; DCHECK(index < total_icons); @@ -1191,16 +1188,12 @@ void LocationBarViewGtk::PageActionViewGtk::OnImageLoaded(SkBitmap* image, // index greater than the number of icons, it must be the default icon. if (image) { GdkPixbuf* pixbuf = gfx::GdkPixbufFromSkBitmap(image); - if (index < page_action_->icon_paths()->size()) + if (index < static_cast<int>(page_action_->icon_paths()->size())) pixbufs_[page_action_->icon_paths()->at(index)] = pixbuf; else pixbufs_[page_action_->default_icon_path()] = pixbuf; } - // If we are done, release the tracker. - if (index == (total_icons - 1)) - tracker_ = NULL; - owner_->UpdatePageActions(); } diff --git a/chrome/browser/gtk/location_bar_view_gtk.h b/chrome/browser/gtk/location_bar_view_gtk.h index 2037a2e..a92d894 100644 --- a/chrome/browser/gtk/location_bar_view_gtk.h +++ b/chrome/browser/gtk/location_bar_view_gtk.h @@ -193,7 +193,8 @@ class LocationBarViewGtk : public AutocompleteEditController, void UpdateVisibility(TabContents* contents, GURL url); // A callback from ImageLoadingTracker for when the image has loaded. - virtual void OnImageLoaded(SkBitmap* image, size_t index); + virtual void OnImageLoaded( + SkBitmap* image, ExtensionResource resource, int index); // Simulate left mouse click on the page action button. void TestActivatePageAction(); @@ -237,8 +238,8 @@ class LocationBarViewGtk : public AutocompleteEditController, GdkPixbuf* last_icon_pixbuf_; // The object that is waiting for the image loading to complete - // asynchronously. It will delete itself once it is done. - ImageLoadingTracker* tracker_; + // asynchronously. + ImageLoadingTracker tracker_; // The widgets for this page action. OwnedWidgetGtk event_box_; diff --git a/chrome/browser/views/browser_actions_container.cc b/chrome/browser/views/browser_actions_container.cc index 0f3eff7..088b0ed 100644 --- a/chrome/browser/views/browser_actions_container.cc +++ b/chrome/browser/views/browser_actions_container.cc @@ -98,7 +98,7 @@ BrowserActionButton::BrowserActionButton(Extension* extension, : ALLOW_THIS_IN_INITIALIZER_LIST(MenuButton(this, L"", NULL, false)), browser_action_(extension->browser_action()), extension_(extension), - tracker_(NULL), + ALLOW_THIS_IN_INITIALIZER_LIST(tracker_(this)), showing_context_menu_(false), panel_(panel) { set_alignment(TextButton::ALIGN_CENTER); @@ -120,16 +120,12 @@ BrowserActionButton::BrowserActionButton(Extension* extension, // will crash. But since we know that ImageLoadingTracker is asynchronous, // this should be OK. And doing this in the constructor means that we don't // have to protect against it getting done multiple times. - tracker_ = new ImageLoadingTracker(this, 1); - tracker_->PostLoadImageTask( - extension->GetResource(relative_path), - gfx::Size(Extension::kBrowserActionIconMaxSize, - Extension::kBrowserActionIconMaxSize)); + tracker_.LoadImage(extension->GetResource(relative_path), + gfx::Size(Extension::kBrowserActionIconMaxSize, + Extension::kBrowserActionIconMaxSize)); } BrowserActionButton::~BrowserActionButton() { - if (tracker_) - tracker_->StopTrackingImageLoad(); } gfx::Insets BrowserActionButton::GetInsets() const { @@ -142,12 +138,11 @@ void BrowserActionButton::ButtonPressed(views::Button* sender, panel_->OnBrowserActionExecuted(this, false); // inspect_with_devtools } -void BrowserActionButton::OnImageLoaded(SkBitmap* image, size_t index) { +void BrowserActionButton::OnImageLoaded( + SkBitmap* image, ExtensionResource resource, int index) { if (image) default_icon_ = *image; - tracker_ = NULL; // The tracker object will delete itself when we return. - // Call back to UpdateState() because a more specific icon might have been set // while the load was outstanding. UpdateState(); @@ -207,7 +202,7 @@ GURL BrowserActionButton::GetPopupUrl() { bool BrowserActionButton::Activate() { if (IsPopup()) { - panel_->OnBrowserActionExecuted(this, false); // inspect_with_devtools + panel_->OnBrowserActionExecuted(this, false); // |inspect_with_devtools|. // TODO(erikkay): Run a nested modal loop while the mouse is down to // enable menu-like drag-select behavior. @@ -511,7 +506,7 @@ void BrowserActionsContainer::HidePopup() { void BrowserActionsContainer::TestExecuteBrowserAction(int index) { BrowserActionButton* button = browser_action_views_[index]->button(); - OnBrowserActionExecuted(button, false); // inspect_with_devtools + OnBrowserActionExecuted(button, false); // |inspect_with_devtools|. } void BrowserActionsContainer::TestSetIconVisibilityCount(size_t icons) { @@ -1114,7 +1109,7 @@ void BrowserActionsContainer::NotifyMenuDeleted( void BrowserActionsContainer::InspectPopup( ExtensionAction* action) { OnBrowserActionExecuted(GetBrowserActionView(action)->button(), - true); // inspect_with_devtools + true); // |inspect_with_devtools|. } void BrowserActionsContainer::ExtensionPopupClosed(ExtensionPopup* popup) { diff --git a/chrome/browser/views/browser_actions_container.h b/chrome/browser/views/browser_actions_container.h index 09e1ee2..58ec5ed 100644 --- a/chrome/browser/views/browser_actions_container.h +++ b/chrome/browser/views/browser_actions_container.h @@ -69,7 +69,8 @@ class BrowserActionButton : public views::MenuButton, virtual void ButtonPressed(views::Button* sender, const views::Event& event); // Overridden from ImageLoadingTracker. - virtual void OnImageLoaded(SkBitmap* image, size_t index); + virtual void OnImageLoaded( + SkBitmap* image, ExtensionResource resource, int index); // Overridden from NotificationObserver: virtual void Observe(NotificationType type, @@ -105,9 +106,8 @@ class BrowserActionButton : public views::MenuButton, Extension* extension_; // The object that is waiting for the image loading to complete - // asynchronously. This object can potentially outlive the BrowserActionView, - // and takes care of deleting itself. - ImageLoadingTracker* tracker_; + // asynchronously. + ImageLoadingTracker tracker_; // Whether we are currently showing/just finished showing a context menu. bool showing_context_menu_; @@ -245,8 +245,6 @@ class BrowserActionsContainer public BrowserActionOverflowMenuController::Observer, public ExtensionContextMenuModel::PopupDelegate, public ExtensionPopup::Observer { - - friend class ShowFolderMenuTask; public: BrowserActionsContainer(Browser* browser, views::View* owner_view); virtual ~BrowserActionsContainer(); @@ -368,6 +366,8 @@ class BrowserActionsContainer static bool disable_animations_during_testing_; private: + friend class ShowFolderMenuTask; + typedef std::vector<BrowserActionView*> BrowserActionViews; // ExtensionToolbarModel::Observer implementation. diff --git a/chrome/browser/views/infobars/extension_infobar.cc b/chrome/browser/views/infobars/extension_infobar.cc index ac9e689..2ae4b35 100644 --- a/chrome/browser/views/infobars/extension_infobar.cc +++ b/chrome/browser/views/infobars/extension_infobar.cc @@ -13,7 +13,6 @@ #include "chrome/common/extensions/extension.h" #include "chrome/common/platform_util.h" #include "gfx/canvas.h" -#include "grit/browser_resources.h" #include "grit/theme_resources.h" #include "views/controls/button/menu_button.h" #include "views/controls/menu/menu_2.h" @@ -31,7 +30,8 @@ static const int kDropArrowLeftMargin = 3; ExtensionInfoBar::ExtensionInfoBar(ExtensionInfoBarDelegate* delegate) : InfoBar(delegate), - delegate_(delegate) { + delegate_(delegate), + ALLOW_THIS_IN_INITIALIZER_LIST(tracker_(this)) { ExtensionHost* extension_host = delegate_->extension_host(); // We set the target height for the InfoBar to be the height of the @@ -91,6 +91,36 @@ void ExtensionInfoBar::Layout() { view->SetBounds(x, 0, width() - x - kFarRightMargin - 1, height() - 1); } +void ExtensionInfoBar::OnImageLoaded( + SkBitmap* image, ExtensionResource resource, int index) { + ResourceBundle& rb = ResourceBundle::GetSharedInstance(); + + // We fall back on the default extension icon on failure. + SkBitmap* icon; + if (!image || image->empty()) + icon = rb.GetBitmapNamed(IDR_EXTENSIONS_SECTION); + else + icon = image; + + SkBitmap* drop_image = rb.GetBitmapNamed(IDR_APP_DROPARROW); + + int image_size = Extension::EXTENSION_ICON_BITTY; + scoped_ptr<gfx::Canvas> canvas( + new gfx::Canvas(image_size + kDropArrowLeftMargin + drop_image->width(), + image_size, false)); + canvas->DrawBitmapInt(*icon, + 0, 0, icon->width(), icon->height(), + 0, 0, image_size, image_size, + false); + canvas->DrawBitmapInt(*drop_image, + image_size + kDropArrowLeftMargin, + image_size / 2); + menu_->SetIcon(canvas->ExtractBitmap()); + menu_->SetVisible(true); + + Layout(); +} + void ExtensionInfoBar::RunMenu(View* source, const gfx::Point& pt) { if (!options_menu_contents_.get()) { Browser* browser = BrowserView::GetBrowserViewForNativeWindow( @@ -105,46 +135,20 @@ void ExtensionInfoBar::RunMenu(View* source, const gfx::Point& pt) { } void ExtensionInfoBar::SetupIconAndMenu() { - SkBitmap icon; + menu_ = new views::MenuButton(NULL, std::wstring(), this, false); + menu_->SetVisible(false); + AddChildView(menu_); - // TODO(finnur): http://crbug.com/38521. Use cached version of the Extension - // icon instead of loading it here. ExtensionResource icon_resource; Extension::Icons size = delegate_->extension_host()->extension()-> GetIconPathAllowLargerSize(&icon_resource, Extension::EXTENSION_ICON_BITTY); - if (!icon_resource.GetFilePath().empty()) { - scoped_ptr<SkBitmap> bitmap; - Extension::DecodeIconFromPath(icon_resource.GetFilePath(), size, &bitmap); - if (bitmap.get()) - icon = *bitmap.release(); + if (!icon_resource.relative_path().empty()) { + // Create a tracker to load the image. It will report back on OnImageLoaded. + tracker_.LoadImage(icon_resource, gfx::Size(size, size)); + } else { + OnImageLoaded(NULL, icon_resource, 0); // |image|, |index|. } - - // Failure to get the path or failure to decode it causes fall-back to the - // default extension icon. - if (icon.empty()) { - icon = *ResourceBundle::GetSharedInstance().GetBitmapNamed( - IDR_EXTENSION_DEFAULT_ICON); - } - - ResourceBundle& rb = ResourceBundle::GetSharedInstance(); - SkBitmap* drop_image = rb.GetBitmapNamed(IDR_APP_DROPARROW); - menu_ = new views::MenuButton(NULL, std::wstring(), this, false); - - int image_size = Extension::EXTENSION_ICON_BITTY; - scoped_ptr<gfx::Canvas> canvas( - new gfx::Canvas(image_size + kDropArrowLeftMargin + drop_image->width(), - image_size, false)); - canvas->DrawBitmapInt(icon, - 0, 0, icon.width(), icon.height(), - 0, 0, image_size, image_size, - false); - canvas->DrawBitmapInt(*drop_image, - image_size + kDropArrowLeftMargin, - image_size / 2); - menu_->SetIcon(canvas->ExtractBitmap()); - - AddChildView(menu_); } InfoBar* ExtensionInfoBarDelegate::CreateInfoBar() { diff --git a/chrome/browser/views/infobars/extension_infobar.h b/chrome/browser/views/infobars/extension_infobar.h index ed811d1..7867247 100644 --- a/chrome/browser/views/infobars/extension_infobar.h +++ b/chrome/browser/views/infobars/extension_infobar.h @@ -7,6 +7,7 @@ #include "chrome/browser/views/infobars/infobars.h" +#include "chrome/browser/extensions/image_loading_tracker.h" #include "chrome/browser/views/extensions/extension_view.h" #include "views/controls/menu/view_menu_delegate.h" @@ -21,6 +22,7 @@ namespace views { // This class implements InfoBars for Extensions. class ExtensionInfoBar : public InfoBar, public ExtensionView::Container, + public ImageLoadingTracker::Observer, public views::ViewMenuDelegate { public: explicit ExtensionInfoBar(ExtensionInfoBarDelegate* delegate); @@ -34,6 +36,10 @@ class ExtensionInfoBar : public InfoBar, // Overridden from views::View: virtual void Layout(); + // Overridden from ImageLoadingTracker::Observer: + virtual void OnImageLoaded( + SkBitmap* image, ExtensionResource resource, int index); + // Overridden from views::ViewMenuDelegate: virtual void RunMenu(View* source, const gfx::Point& pt); @@ -51,6 +57,9 @@ class ExtensionInfoBar : public InfoBar, scoped_ptr<views::Menu2> options_menu_menu_; views::MenuButton* menu_; + // Keeps track of images being loaded on the File thread. + ImageLoadingTracker tracker_; + DISALLOW_COPY_AND_ASSIGN(ExtensionInfoBar); }; diff --git a/chrome/browser/views/location_bar_view.cc b/chrome/browser/views/location_bar_view.cc index a7b996a..6aa9795 100644 --- a/chrome/browser/views/location_bar_view.cc +++ b/chrome/browser/views/location_bar_view.cc @@ -1427,7 +1427,7 @@ LocationBarView::PageActionImageView::PageActionImageView( owner_(owner), profile_(profile), page_action_(page_action), - tracker_(NULL), + ALLOW_THIS_IN_INITIALIZER_LIST(tracker_(this)), current_tab_id_(-1), preview_enabled_(false), popup_(NULL) { @@ -1441,20 +1441,15 @@ LocationBarView::PageActionImageView::PageActionImageView( if (!page_action_->default_icon_path().empty()) icon_paths.push_back(page_action_->default_icon_path()); - tracker_ = new ImageLoadingTracker(this, icon_paths.size()); for (std::vector<std::string>::iterator iter = icon_paths.begin(); iter != icon_paths.end(); ++iter) { - tracker_->PostLoadImageTask( - extension->GetResource(*iter), - gfx::Size(Extension::kPageActionIconMaxSize, - Extension::kPageActionIconMaxSize)); + tracker_.LoadImage(extension->GetResource(*iter), + gfx::Size(Extension::kPageActionIconMaxSize, + Extension::kPageActionIconMaxSize)); } } LocationBarView::PageActionImageView::~PageActionImageView() { - if (tracker_) - tracker_->StopTrackingImageLoad(); - if (popup_) HidePopup(); } @@ -1561,11 +1556,11 @@ void LocationBarView::PageActionImageView::ShowInfoBubble() { ShowInfoBubbleImpl(ASCIIToWide(tooltip_), GetColor(false, TEXT)); } -void LocationBarView::PageActionImageView::OnImageLoaded(SkBitmap* image, - size_t index) { +void LocationBarView::PageActionImageView::OnImageLoaded( + SkBitmap* image, ExtensionResource resource, int index) { // We loaded icons()->size() icons, plus one extra if the page action had // a default icon. - size_t total_icons = page_action_->icon_paths()->size(); + int total_icons = static_cast<int>(page_action_->icon_paths()->size()); if (!page_action_->default_icon_path().empty()) total_icons++; DCHECK(index < total_icons); @@ -1573,16 +1568,12 @@ void LocationBarView::PageActionImageView::OnImageLoaded(SkBitmap* image, // Map the index of the loaded image back to its name. If we ever get an // index greater than the number of icons, it must be the default icon. if (image) { - if (index < page_action_->icon_paths()->size()) + if (index < static_cast<int>(page_action_->icon_paths()->size())) page_action_icons_[page_action_->icon_paths()->at(index)] = *image; else page_action_icons_[page_action_->default_icon_path()] = *image; } - // If we are done, release the tracker. - if (index == total_icons - 1) - tracker_ = NULL; - owner_->UpdatePageActions(); } diff --git a/chrome/browser/views/location_bar_view.h b/chrome/browser/views/location_bar_view.h index ce41ec7..084006d 100644 --- a/chrome/browser/views/location_bar_view.h +++ b/chrome/browser/views/location_bar_view.h @@ -439,12 +439,13 @@ class LocationBarView : public LocationBar, virtual void ShowInfoBubble(); // Overridden from ImageLoadingTracker. - virtual void OnImageLoaded(SkBitmap* image, size_t index); + virtual void OnImageLoaded( + SkBitmap* image, ExtensionResource resource, int index); // Overridden from ExtensionContextMenuModelModel::Delegate virtual void InspectPopup(ExtensionAction* action); - // Overriden from ExtensionPopup::Observer + // Overridden from ExtensionPopup::Observer virtual void ExtensionPopupClosed(ExtensionPopup* popup); // Called to notify the PageAction that it should determine whether to be @@ -479,7 +480,7 @@ class LocationBarView : public LocationBar, // The object that is waiting for the image loading to complete // asynchronously. - ImageLoadingTracker* tracker_; + ImageLoadingTracker tracker_; // The tab id we are currently showing the icon for. int current_tab_id_; diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc index eb2ffe2..98a2f84 100644 --- a/chrome/common/extensions/extension.cc +++ b/chrome/common/extensions/extension.cc @@ -1484,7 +1484,7 @@ ExtensionResource Extension::GetIconPath(Icons icon) { Extension::Icons Extension::GetIconPathAllowLargerSize( ExtensionResource* resource, Icons icon) { *resource = GetIconPath(icon); - if (!resource->GetFilePath().empty()) + if (!resource->relative_path().empty()) return icon; if (icon == EXTENSION_ICON_BITTY) return GetIconPathAllowLargerSize(resource, EXTENSION_ICON_SMALL); diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h index dbfbee0..05c1522 100644 --- a/chrome/common/extensions/extension.h +++ b/chrome/common/extensions/extension.h @@ -185,14 +185,20 @@ class Extension { static bool IsPrivilegeIncrease(Extension* old_extension, Extension* new_extension); + // *** HITS THE FILESYSTEM. Do not call on UI thread! *** // Given an extension and icon size, read it if present and decode it into // result. + // NOTE: If you need the icon on the UI thread, please use the + // ImageLoadingTracker, which uses the File thread internally to decode. static void DecodeIcon(Extension* extension, Icons icon_size, scoped_ptr<SkBitmap>* result); + // *** HITS THE FILESYSTEM. Do not call on UI thread! *** // Given an icon_path and icon size, read it if present and decode it into // result. + // NOTE: If you need the icon on the UI thread, please use the + // ImageLoadingTracker, which uses the File thread internally to decode. static void DecodeIconFromPath(const FilePath& icon_path, Icons icon_size, scoped_ptr<SkBitmap>* result); diff --git a/chrome/common/extensions/extension_resource.cc b/chrome/common/extensions/extension_resource.cc index 195b3e8..8eed2e9 100644 --- a/chrome/common/extensions/extension_resource.cc +++ b/chrome/common/extensions/extension_resource.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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. @@ -57,7 +57,7 @@ FilePath ExtensionResource::GetFilePath(const FilePath& extension_root, return FilePath(); } -// Unittesting helpers. +// Unit-testing helpers. FilePath::StringType ExtensionResource::NormalizeSeperators( FilePath::StringType path) const { #if defined(FILE_PATH_USES_WIN_SEPARATORS) diff --git a/chrome/common/extensions/extension_resource.h b/chrome/common/extensions/extension_resource.h index 0356bec..e87787d 100644 --- a/chrome/common/extensions/extension_resource.h +++ b/chrome/common/extensions/extension_resource.h @@ -1,4 +1,4 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2010 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. @@ -20,11 +20,15 @@ class ExtensionResource { // Returns actual path to the resource (default or locale specific). // *** MIGHT HIT FILESYSTEM. Do not call on UI thread! *** + // NOTE: If you need to access ExtensionResources, such as bitmaps, on the UI + // thread, please use the ImageLoadingTracker, which uses the File thread. const FilePath& GetFilePath() const; // Static version to avoid creating an instance of ExtensionResource // when all we want is the localization code. // *** MIGHT HIT FILESYSTEM. Do not call on UI thread! *** + // NOTE: If you need to access ExtensionResources, such as bitmaps, on the UI + // thread, please use the ImageLoadingTracker, which uses the File thread. static FilePath GetFilePath(const FilePath& extension_root, const FilePath& relative_path); @@ -32,7 +36,7 @@ class ExtensionResource { const FilePath& extension_root() const { return extension_root_; } const FilePath& relative_path() const { return relative_path_; } - // Unittest helpers. + // Unit test helpers. FilePath::StringType NormalizeSeperators(FilePath::StringType path) const; bool ComparePathWithDefault(const FilePath& path) const; |