diff options
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; |