diff options
Diffstat (limited to 'chrome/browser/extensions')
7 files changed, 99 insertions, 36 deletions
diff --git a/chrome/browser/extensions/api/extension_action/extension_action_api.cc b/chrome/browser/extensions/api/extension_action/extension_action_api.cc index 291aac2..cf7db41 100644 --- a/chrome/browser/extensions/api/extension_action/extension_action_api.cc +++ b/chrome/browser/extensions/api/extension_action/extension_action_api.cc @@ -29,6 +29,7 @@ #include "extensions/browser/image_util.h" #include "extensions/browser/notification_types.h" #include "extensions/common/error_utils.h" +#include "extensions/common/feature_switch.h" #include "ui/gfx/image/image.h" #include "ui/gfx/image/image_skia.h" @@ -48,7 +49,6 @@ const char kNoTabError[] = "No tab with id: *."; const char kOpenPopupError[] = "Failed to show popup either because there is an existing popup or another " "error occurred."; -const char kInternalError[] = "Internal error."; } // namespace @@ -171,13 +171,9 @@ ExtensionAction::ShowAction ExtensionActionAPI::ExecuteExtensionAction( int tab_id = SessionTabHelper::IdForTab(web_contents); - ExtensionActionManager* action_manager = - ExtensionActionManager::Get(static_cast<Profile*>(browser_context_)); - // TODO(devlin): Make a ExtensionActionManager::GetExtensionAction method. ExtensionAction* extension_action = - action_manager->GetBrowserAction(*extension); - if (!extension_action) - extension_action = action_manager->GetPageAction(*extension); + ExtensionActionManager::Get(browser_context_)->GetExtensionAction( + *extension); // Anything that calls this should have a page or browser action. DCHECK(extension_action); @@ -205,6 +201,29 @@ ExtensionAction::ShowAction ExtensionActionAPI::ExecuteExtensionAction( return ExtensionAction::ACTION_NONE; } +bool ExtensionActionAPI::ShowExtensionActionPopup( + const Extension* extension, + Browser* browser, + bool grant_active_tab_permissions) { + ExtensionAction* extension_action = + ExtensionActionManager::Get(browser_context_)->GetExtensionAction( + *extension); + if (!extension_action) + return false; + + if (extension_action->action_type() == ActionInfo::TYPE_PAGE && + !FeatureSwitch::extension_action_redesign()->IsEnabled()) { + // We show page actions in the location bar unless the new toolbar is + // enabled. + return browser->window()->GetLocationBar()->ShowPageActionPopup( + extension, grant_active_tab_permissions); + } else { + return ExtensionToolbarModel::Get(browser->profile())-> + ShowExtensionActionPopup( + extension, browser, grant_active_tab_permissions); + } +} + void ExtensionActionAPI::NotifyChange(ExtensionAction* extension_action, content::WebContents* web_contents, content::BrowserContext* context) { @@ -563,13 +582,19 @@ BrowserActionOpenPopupFunction::BrowserActionOpenPopupFunction() } bool BrowserActionOpenPopupFunction::RunAsync() { - ExtensionToolbarModel* model = ExtensionToolbarModel::Get(GetProfile()); - if (!model) { - error_ = kInternalError; - return false; - } - - if (!model->ShowBrowserActionPopup(extension_.get())) { + // We only allow the popup in the active window. + Browser* browser = chrome::FindLastActiveWithProfile( + GetProfile(), chrome::GetActiveDesktop()); + + // If there's no active browser, or the Toolbar isn't visible, abort. + // Otherwise, try to open a popup in the active browser. + // TODO(justinlin): Remove toolbar check when http://crbug.com/308645 is + // fixed. + if (!browser || + !browser->window()->IsActive() || + !browser->window()->IsToolbarVisible() || + !ExtensionActionAPI::Get(GetProfile())->ShowExtensionActionPopup( + extension_.get(), browser, false)) { error_ = kOpenPopupError; return false; } diff --git a/chrome/browser/extensions/api/extension_action/extension_action_api.h b/chrome/browser/extensions/api/extension_action/extension_action_api.h index 3830096..7d76b2c 100644 --- a/chrome/browser/extensions/api/extension_action/extension_action_api.h +++ b/chrome/browser/extensions/api/extension_action/extension_action_api.h @@ -83,6 +83,14 @@ class ExtensionActionAPI : public BrowserContextKeyedAPI { Browser* browser, bool grant_active_tab_permissions); + // Opens the popup for the given |extension| in the given |browser|'s window. + // If |grant_active_tab_permissions| is true, this grants the extension + // activeTab (so this should only be done if this is through a direct user + // action). + bool ShowExtensionActionPopup(const Extension* extension, + Browser* browser, + bool grant_active_tab_permissions); + // Notifies that there has been a change in the given |extension_action|. void NotifyChange(ExtensionAction* extension_action, content::WebContents* web_contents, diff --git a/chrome/browser/extensions/extension_action_manager.cc b/chrome/browser/extensions/extension_action_manager.cc index 125021a..14ab141 100644 --- a/chrome/browser/extensions/extension_action_manager.cc +++ b/chrome/browser/extensions/extension_action_manager.cc @@ -97,8 +97,7 @@ void PopulateMissingValues(const Extension& extension, if (action->default_icon()) *default_icon = *action->default_icon(); - const ExtensionIconSet& extension_icons = - extensions::IconsInfo::GetIcons(&extension); + const ExtensionIconSet& extension_icons = IconsInfo::GetIcons(&extension); std::string largest_icon = extension_icons.Get( extension_misc::EXTENSION_ICON_GIGANTOR, ExtensionIconSet::MATCH_SMALLER); @@ -193,7 +192,7 @@ ExtensionAction* ExtensionActionManager::GetSystemIndicator( // given profile. This could return NULL if the system indicator area is // unavailable on the current system. If so, return NULL to signal that // the system indicator area is unusable. - if (!extensions::SystemIndicatorManagerFactory::GetForProfile(profile_)) + if (!SystemIndicatorManagerFactory::GetForProfile(profile_)) return NULL; return GetOrCreateOrNull(&system_indicators_, extension, @@ -202,4 +201,10 @@ ExtensionAction* ExtensionActionManager::GetSystemIndicator( profile_); } +ExtensionAction* ExtensionActionManager::GetExtensionAction( + const Extension& extension) const { + ExtensionAction* action = GetBrowserAction(extension); + return action ? action : GetPageAction(extension); +} + } // namespace extensions diff --git a/chrome/browser/extensions/extension_action_manager.h b/chrome/browser/extensions/extension_action_manager.h index 8973397..0d78277 100644 --- a/chrome/browser/extensions/extension_action_manager.h +++ b/chrome/browser/extensions/extension_action_manager.h @@ -33,21 +33,26 @@ class ExtensionActionManager : public KeyedService, // shared between a profile and its incognito version. static ExtensionActionManager* Get(content::BrowserContext* browser_context); - // Retrieves the page action, or browser action for |extension|. + // Retrieves the page action, browser action, or system indicator for + // |extension|. // If the result is not NULL, it remains valid until the extension is // unloaded. - ExtensionAction* GetPageAction(const extensions::Extension& extension) const; - ExtensionAction* GetBrowserAction( - const extensions::Extension& extension) const; - ExtensionAction* GetSystemIndicator( - const extensions::Extension& extension) const; + ExtensionAction* GetPageAction(const Extension& extension) const; + ExtensionAction* GetBrowserAction(const Extension& extension) const; + ExtensionAction* GetSystemIndicator(const Extension& extension) const; + + // Returns either the PageAction or BrowserAction for |extension|, or NULL if + // none exists. Since an extension can only declare one of Browser|PageAction, + // this is okay to use anywhere you need a generic "ExtensionAction". + // Since SystemIndicators are used differently and don't follow this + // rule of mutual exclusion, they are not checked or returned. + ExtensionAction* GetExtensionAction(const Extension& extension) const; // Gets the best fit ExtensionAction for the given |extension|. This takes // into account |extension|'s browser or page actions, if any, along with its // name and any declared icons. scoped_ptr<ExtensionAction> GetBestFitAction( - const extensions::Extension& extension, - extensions::ActionInfo::Type type) const; + const Extension& extension, ActionInfo::Type type) const; private: // Implement ExtensionRegistryObserver. diff --git a/chrome/browser/extensions/extension_toolbar_model.cc b/chrome/browser/extensions/extension_toolbar_model.cc index 35e51b2..1858603 100644 --- a/chrome/browser/extensions/extension_toolbar_model.cc +++ b/chrome/browser/extensions/extension_toolbar_model.cc @@ -569,13 +569,18 @@ void ExtensionToolbarModel::OnExtensionToolbarPrefChange() { } } -bool ExtensionToolbarModel::ShowBrowserActionPopup(const Extension* extension) { +bool ExtensionToolbarModel::ShowExtensionActionPopup( + const Extension* extension, + Browser* browser, + bool grant_active_tab) { ObserverListBase<Observer>::Iterator it(observers_); Observer* obs = NULL; + // Look for the Observer associated with the browser. + // This would be cleaner if we had an abstract class for the Toolbar UI + // (like we do for LocationBar), but sadly, we don't. while ((obs = it.GetNext()) != NULL) { - // Stop after first popup since it should only show in the active window. - if (obs->ShowExtensionActionPopup(extension)) - return true; + if (obs->GetBrowser() == browser) + return obs->ShowExtensionActionPopup(extension, grant_active_tab); } return false; } diff --git a/chrome/browser/extensions/extension_toolbar_model.h b/chrome/browser/extensions/extension_toolbar_model.h index c0d35d6..29e83b3 100644 --- a/chrome/browser/extensions/extension_toolbar_model.h +++ b/chrome/browser/extensions/extension_toolbar_model.h @@ -37,8 +37,8 @@ class ExtensionToolbarModel : public content::NotificationObserver, // A class which is informed of changes to the model; represents the view of // MVC. Also used for signaling view changes such as showing extension popups. - // TODO(devlin): Should this really be an observer? There should probably be - // only one (aka a Delegate)... + // TODO(devlin): Should this really be an observer? It acts more like a + // delegate. class Observer { public: // An extension has been added to the toolbar and should go at |index|. @@ -59,8 +59,11 @@ class ExtensionToolbarModel : public content::NotificationObserver, virtual void ToolbarExtensionUpdated(const Extension* extension) = 0; // Signal the |extension| to show the popup now in the active window. + // If |grant_active_tab| is true, then active tab permissions should be + // given to the extension (only do this if this is through a user action). // Returns true if a popup was slated to be shown. - virtual bool ShowExtensionActionPopup(const Extension* extension) = 0; + virtual bool ShowExtensionActionPopup(const Extension* extension, + bool grant_active_tab) = 0; // Signal when the container needs to be redrawn because of a size change, // and when the model has finished loading. @@ -75,6 +78,9 @@ class ExtensionToolbarModel : public content::NotificationObserver, // with the new set (and just assume the new set is different). virtual void ToolbarHighlightModeChanged(bool is_highlighting) = 0; + // Returns the browser associated with the Observer. + virtual Browser* GetBrowser() = 0; + protected: virtual ~Observer() {} }; @@ -112,9 +118,13 @@ class ExtensionToolbarModel : public content::NotificationObserver, void OnExtensionToolbarPrefChange(); - // Tells observers to display a popup without granting tab permissions and - // returns whether the popup was slated to be shown. - bool ShowBrowserActionPopup(const Extension* extension); + // Finds the Observer associated with |browser| and tells it to display a + // popup for the given |extension|. If |grant_active_tab| is true, this + // grants active tab permissions to the |extension|; only do this because of + // a direct user action. + bool ShowExtensionActionPopup(const Extension* extension, + Browser* browser, + bool grant_active_tab); // Ensures that the extensions in the |extension_ids| list are visible on the // toolbar. This might mean they need to be moved to the front (if they are in diff --git a/chrome/browser/extensions/extension_toolbar_model_unittest.cc b/chrome/browser/extensions/extension_toolbar_model_unittest.cc index e2631c2..9b90bb5 100644 --- a/chrome/browser/extensions/extension_toolbar_model_unittest.cc +++ b/chrome/browser/extensions/extension_toolbar_model_unittest.cc @@ -75,7 +75,8 @@ class ExtensionToolbarModelTestObserver virtual void ToolbarExtensionUpdated(const Extension* extension) OVERRIDE { } - virtual bool ShowExtensionActionPopup(const Extension* extension) OVERRIDE { + virtual bool ShowExtensionActionPopup(const Extension* extension, + bool grant_active_tab) OVERRIDE { return false; } @@ -87,6 +88,10 @@ class ExtensionToolbarModelTestObserver highlight_mode_count_ += is_highlighting ? 1 : -1; } + virtual Browser* GetBrowser() OVERRIDE { + return NULL; + } + ExtensionToolbarModel* model_; size_t inserted_count_; |